Skip to content

Commit 3c12ff2

Browse files
committed
Bugfix: Accept null for optional crash report fields
Old crash files read by a new app version deserialize `build_mode` and `short_id` as `Option::None`, and the Rust client serializes those as JSON `null` (not omitted — specta's unified mode rejects `skip_serializing_if`). The server validator was treating `null` as invalid and 400-ing the upload, losing exactly the upgrade-window reports we want to keep. Treat `null` the same as `undefined` ("field absent"), and add a regression test.
1 parent e89a63a commit 3c12ff2

2 files changed

Lines changed: 27 additions & 2 deletions

File tree

apps/api-server/src/crash-report.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,23 @@ describe('POST /crash-report', () => {
100100
expect(bindArgs[8]).toBe('CRASH-A2345')
101101
})
102102

103+
it('accepts explicit nulls for buildMode and shortId (upgrade-window compat)', async () => {
104+
// Rust clients serialize `Option::None` as `null` because specta's unified mode
105+
// rejects `skip_serializing_if`. Old crash files read by a new client surface as
106+
// `None` and would hit this path. Reject would lose the report.
107+
const { db, bindMock } = createMockD1()
108+
const bindings = createBindings({ TELEMETRY_DB: db })
109+
110+
const report = { ...validCrashReport, buildMode: null, shortId: null }
111+
112+
const res = await postCrashReport(report, bindings)
113+
expect(res.status).toBe(204)
114+
115+
const bindArgs = bindMock.mock.calls[0]
116+
expect(bindArgs[7]).toBeNull()
117+
expect(bindArgs[8]).toBeNull()
118+
})
119+
103120
it('returns 400 for invalid buildMode', async () => {
104121
const bindings = createBindings()
105122
const report = { ...validCrashReport, buildMode: 'staging' }

apps/api-server/src/telemetry.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,20 @@ function validateCrashReportShape(report: Record<string, unknown>): string | nul
4848
return `Missing required field: ${field}`
4949
}
5050
}
51+
// `null` and `undefined` both mean "field absent" — old clients omit the keys,
52+
// newer Rust clients serialize `Option::None` as `null` (specta's unified mode
53+
// rejects `skip_serializing_if`). Treat them the same so an upgrade-window send
54+
// of a pre-fix-* crash file with `None` defaults isn't rejected.
5155
const buildMode = report.buildMode
52-
if (buildMode !== undefined && buildMode !== 'release' && buildMode !== 'debug') {
56+
if (buildMode !== undefined && buildMode !== null && buildMode !== 'release' && buildMode !== 'debug') {
5357
return 'Invalid buildMode'
5458
}
5559
const shortId = report.shortId
56-
if (shortId !== undefined && (typeof shortId !== 'string' || !crashShortIdPattern.test(shortId))) {
60+
if (
61+
shortId !== undefined &&
62+
shortId !== null &&
63+
(typeof shortId !== 'string' || !crashShortIdPattern.test(shortId))
64+
) {
5765
return 'Invalid shortId'
5866
}
5967
return null

0 commit comments

Comments
 (0)