Skip to content

Fall back to UTC when a legacy report has no timezone - #5

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

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

Conversation

@ZIJ

@ZIJ ZIJ commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Cause: GET /reports/:id in app/api.mjs called report.timezone.trim() unconditionally. Legacy report records (e.g. report-legacy) have timezone: null, so this threw "TypeError: Cannot read properties of null (reading 'trim')" and produced a 500, matching the captured Sentry event and the failing replay of report-legacy.

Fix: default the timezone to "UTC" before trimming: (report.timezone ?? "UTC").trim(). This is the smallest change that keeps existing healthy-timezone behavior (e.g. report-current / Europe/London) unaffected while making legacy null-timezone reports resolve successfully with a UTC-formatted timestamp.

Regression test: app/test/api.regression.test.mjs derives its state directly from the captured incident snapshot (report-legacy with timezone: null, report-current with 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 it.

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

Tested against commit c0051fb2cd810c65e2c2f12a43b85912623d7c35. 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 falls back to UTC for a legacy report with no timezone
not ok 1 - GET falls back to UTC for a legacy report with no timezone
  ---
  duration_ms: 60.382156
  type: 'test'
  location: '<verification>/before/app/test/api.regression.test.mjs:26: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:27: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 still honors an explicit timezone for a healthy report
ok 2 - GET still honors an explicit timezone for a healthy report
  ---
  duration_ms: 26.694357
  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.915413
  type: 'test'
  ...
# Subtest: unknown reports and unmatched routes return 404
ok 4 - unknown reports and unmatched routes return 404
  ---
  duration_ms: 1.400009
  type: 'test'
  ...
# Subtest: report writes are rejected
ok 5 - report writes are rejected
  ---
  duration_ms: 0.940651
  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.11543
  type: 'test'
  ...
# Subtest: CSV output preserves commas, quotes, and embedded newlines
ok 7 - CSV output preserves commas, quotes, and embedded newlines
  ---
  duration_ms: 0.325516
  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.497309
  type: 'test'
  ...
1..8
# tests 8
# suites 0
# pass 7
# fail 1
# cancelled 0
# skipped 0
# todo 0
# duration_ms 705.920824

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 falls back to UTC for a legacy report with no timezone
ok 1 - GET falls back to UTC for a legacy report with no timezone
  ---
  duration_ms: 85.748722
  type: 'test'
  ...
# Subtest: GET still honors an explicit timezone for a healthy report
ok 2 - GET still honors an explicit timezone for a healthy report
  ---
  duration_ms: 1.089644
  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: 84.035898
  type: 'test'
  ...
# Subtest: unknown reports and unmatched routes return 404
ok 4 - unknown reports and unmatched routes return 404
  ---
  duration_ms: 1.390168
  type: 'test'
  ...
# Subtest: report writes are rejected
ok 5 - report writes are rejected
  ---
  duration_ms: 0.950661
  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.116085
  type: 'test'
  ...
# Subtest: CSV output preserves commas, quotes, and embedded newlines
ok 7 - CSV output preserves commas, quotes, and embedded newlines
  ---
  duration_ms: 0.323815
  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.501537
  type: 'test'
  ...
1..8
# tests 8
# suites 0
# pass 8
# fail 0
# cancelled 0
# skipped 0
# todo 0
# duration_ms 706.896808

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