Skip to content

Fix crash on reports with a null timezone in GET /reports/:id - #3

Open
ZIJ wants to merge 1 commit into
mainfrom
fix/oncall-api-534c46058a10
Open

ZIJ wants to merge 1 commit into
mainfrom
fix/oncall-api-534c46058a10

Conversation

@ZIJ

@ZIJ ZIJ commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Cause: handleRequest in app/api.mjs called report.timezone.trim() unconditionally. Legacy report records (created before timezone tracking existed) have timezone: null, so this threw "TypeError: Cannot read properties of null (reading 'trim')" and the request returned 500, matching the captured Sentry event for GET /reports/report-legacy.

Fix: default a missing/null timezone to "UTC" before trimming: (report.timezone ?? "UTC").trim(). Reports with a valid timezone string are unaffected; reports with a null timezone now render using UTC instead of crashing.

Regression test: app/test/api.regression.test.mjs exercises handleRequest with a synthetic report derived from the incident's captured state (id "report-legacy", timezone: null) and asserts a 200 response with a non-empty fallback timezone and generatedAt string, instead of the request throwing.

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

Tested against commit b664e075ca7c0173b06f344d47a8cbcfa6e426fa. 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 report with a null timezone falls back to a default instead of crashing
not ok 1 - GET report with a null timezone falls back to a default instead of crashing
  ---
  duration_ms: 58.641637
  type: 'test'
  location: '<verification>/before/app/test/api.regression.test.mjs:19: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:20: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 returns a report with its timestamp in the configured timezone
ok 2 - GET returns a report with its timestamp in the configured timezone
  ---
  duration_ms: 84.714596
  type: 'test'
  ...
# Subtest: unknown reports and unmatched routes return 404
ok 3 - unknown reports and unmatched routes return 404
  ---
  duration_ms: 1.425469
  type: 'test'
  ...
# Subtest: report writes are rejected
ok 4 - report writes are rejected
  ---
  duration_ms: 0.95472
  type: 'test'
  ...
# Subtest: a bounded batch processes healthy jobs in creation order
ok 5 - a bounded batch processes healthy jobs in creation order
  ---
  duration_ms: 16.077736
  type: 'test'
  ...
# Subtest: CSV output preserves commas, quotes, and embedded newlines
ok 6 - CSV output preserves commas, quotes, and embedded newlines
  ---
  duration_ms: 0.327159
  type: 'test'
  ...
# Subtest: completed jobs are not repeated when another batch runs
ok 7 - completed jobs are not repeated when another batch runs
  ---
  duration_ms: 0.509935
  type: 'test'
  ...
1..7
# tests 7
# suites 0
# pass 6
# fail 1
# cancelled 0
# skipped 0
# todo 0
# duration_ms 679.448554

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 report with a null timezone falls back to a default instead of crashing
ok 1 - GET report with a null timezone falls back to a default instead of crashing
  ---
  duration_ms: 83.293418
  type: 'test'
  ...
# Subtest: GET returns a report with its timestamp in the configured timezone
ok 2 - GET returns a report with its timestamp in the configured timezone
  ---
  duration_ms: 83.624762
  type: 'test'
  ...
# Subtest: unknown reports and unmatched routes return 404
ok 3 - unknown reports and unmatched routes return 404
  ---
  duration_ms: 1.402416
  type: 'test'
  ...
# Subtest: report writes are rejected
ok 4 - report writes are rejected
  ---
  duration_ms: 0.932459
  type: 'test'
  ...
# Subtest: a bounded batch processes healthy jobs in creation order
ok 5 - a bounded batch processes healthy jobs in creation order
  ---
  duration_ms: 16.196487
  type: 'test'
  ...
# Subtest: CSV output preserves commas, quotes, and embedded newlines
ok 6 - CSV output preserves commas, quotes, and embedded newlines
  ---
  duration_ms: 0.332082
  type: 'test'
  ...
# Subtest: completed jobs are not repeated when another batch runs
ok 7 - completed jobs are not repeated when another batch runs
  ---
  duration_ms: 0.515313
  type: 'test'
  ...
1..7
# tests 7
# suites 0
# pass 7
# fail 0
# cancelled 0
# skipped 0
# todo 0
# duration_ms 700.819616

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