Skip to content

Fix TypeError when a report has no stored timezone - #1

Open
ZIJ wants to merge 1 commit into
mainfrom
fix/oncall-api-69dcce6c653e
Open

ZIJ wants to merge 1 commit into
mainfrom
fix/oncall-api-69dcce6c653e

Conversation

@ZIJ

@ZIJ ZIJ commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Cause: handleRequest in app/api.mjs called report.timezone.trim() unconditionally. The captured incident's "report-legacy" record has timezone: null, so .trim() threw TypeError: Cannot read properties of null (reading 'trim'), returning a 500 for GET /reports/report-legacy.

Fix: default to "UTC" when timezone is null/undefined before trimming: (report.timezone ?? "UTC").trim(). Reports with a configured timezone (e.g. "Europe/London") are unaffected.

Regression test added at app/test/api.regression.test.mjs, using synthetic state derived from the incident snapshot (report-legacy with timezone: null, report-current with timezone: "Europe/London"). It asserts the legacy report now returns 200 with timezone "UTC" and the correct generatedAt, and that the healthy report's response is unchanged. This test fails with the original TypeError before the fix and passes after.

Sentry incident: event. This is the deliberately faulty on-call example app.

Tested against commit 04b1e334a737824d719341f674d7048afa69401c. The same regression command failed before the fix and passed afterward:

node --test --test-reporter=tap --test-concurrency=1 app/test/api.regression.test.mjs app/test/api.test.mjs app/test/worker.test.mjs
Exit 1
TAP version 13
# Subtest: GET on a report with no stored timezone falls back to UTC instead of throwing
not ok 1 - GET on a report with no stored timezone falls back to UTC instead of throwing
  ---
  duration_ms: 59.060364
  type: 'test'
  location: '<verification>/before/app/test/api.regression.test.mjs:24:1'
  failureType: 'testCodeFailure'
  error: "Cannot read properties of null (reading 'trim')"
  code: 'ERR_TEST_FAILURE'
  name: 'TypeError'
  stack: |-
    handleRequest (file://<verification>/before/app/api.mjs:9:36)
    TestContext.<anonymous> (file://<verification>/before/app/test/api.regression.test.mjs:25:26)
    Test.runInAsyncScope (node:async_hooks:214:14)
    Test.run (node:internal/test_runner/test:1047:25)
    Test.start (node:internal/test_runner/test:944:17)
    startSubtestAfterBootstrap (node:internal/test_runner/harness:296:17)
  ...
# Subtest: GET on a report with a configured timezone is unaffected by the fallback
ok 2 - GET on a report with a configured timezone is unaffected by the fallback
  ---
  duration_ms: 26.512245
  type: 'test'
  ...
# Subtest: GET returns a report with its timestamp in the configured timezone
ok 3 - GET returns a report with its timestamp in the configured timezone
  ---
  duration_ms: 83.485214
  type: 'test'
  ...
# Subtest: unknown reports and unmatched routes return 404
ok 4 - unknown reports and unmatched routes return 404
  ---
  duration_ms: 1.403182
  type: 'test'
  ...
# Subtest: report writes are rejected
ok 5 - report writes are rejected
  ---
  duration_ms: 0.965535
  type: 'test'
  ...
# Subtest: a bounded batch processes healthy jobs in creation order
ok 6 - a bounded batch processes healthy jobs in creation order
  ---
  duration_ms: 16.231367
  type: 'test'
  ...
# Subtest: CSV output preserves commas, quotes, and embedded newlines
ok 7 - CSV output preserves commas, quotes, and embedded newlines
  ---
  duration_ms: 0.334221
  type: 'test'
  ...
# Subtest: completed jobs are not repeated when another batch runs
ok 8 - completed jobs are not repeated when another batch runs
  ---
  duration_ms: 0.508818
  type: 'test'
  ...
1..8
# tests 8
# suites 0
# pass 7
# fail 1
# cancelled 0
# skipped 0
# todo 0
# duration_ms 703.683624

node --test --test-reporter=tap --test-concurrency=1 app/test/api.regression.test.mjs app/test/api.test.mjs app/test/worker.test.mjs
Exit 0
TAP version 13
# Subtest: GET on a report with no stored timezone falls back to UTC instead of throwing
ok 1 - GET on a report with no stored timezone falls back to UTC instead of throwing
  ---
  duration_ms: 83.657105
  type: 'test'
  ...
# Subtest: GET on a report with a configured timezone is unaffected by the fallback
ok 2 - GET on a report with a configured timezone is unaffected by the fallback
  ---
  duration_ms: 1.028324
  type: 'test'
  ...
# Subtest: GET returns a report with its timestamp in the configured timezone
ok 3 - GET returns a report with its timestamp in the configured timezone
  ---
  duration_ms: 83.676543
  type: 'test'
  ...
# Subtest: unknown reports and unmatched routes return 404
ok 4 - unknown reports and unmatched routes return 404
  ---
  duration_ms: 1.409147
  type: 'test'
  ...
# Subtest: report writes are rejected
ok 5 - report writes are rejected
  ---
  duration_ms: 0.947061
  type: 'test'
  ...
# Subtest: a bounded batch processes healthy jobs in creation order
ok 6 - a bounded batch processes healthy jobs in creation order
  ---
  duration_ms: 16.103704
  type: 'test'
  ...
# Subtest: CSV output preserves commas, quotes, and embedded newlines
ok 7 - CSV output preserves commas, quotes, and embedded newlines
  ---
  duration_ms: 0.339642
  type: 'test'
  ...
# Subtest: completed jobs are not repeated when another batch runs
ok 8 - completed jobs are not repeated when another batch runs
  ---
  duration_ms: 0.52389
  type: 'test'
  ...
1..8
# tests 8
# suites 0
# pass 8
# fail 0
# cancelled 0
# skipped 0
# todo 0
# duration_ms 701.650732

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant