Skip to content

Commit 356f2b2

Browse files
committed
fix(logger): never let structured serialization throw into the caller
In production the JSON branch merged caller-supplied arguments into the log entry and stringified it with no error handling. A cyclic reference, a BigInt, or a throwing getter in that metadata raised a TypeError out of `logger.info` and friends: the line was lost and the caller's code path aborted. Dev was unaffected — the colorized branch already routes objects through `formatObject`, which catches — so this class of bug is invisible locally and only surfaces in production, where it reads as structured logs disappearing while raw stack traces keep shipping. Build and serialize through `serializeEntry`, which falls back to a cycle/BigInt-tolerant replacer and then to a minimal entry flagged with `serializationError`.
1 parent 64b3472 commit 356f2b2

2 files changed

Lines changed: 91 additions & 19 deletions

File tree

packages/logger/src/index.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,4 +216,43 @@ describe('Logger', () => {
216216
)
217217
})
218218
})
219+
220+
describe('structured serialization safety', () => {
221+
const createEnabledLogger = () =>
222+
new Logger('Test', { enabled: true, colorize: false, logLevel: LogLevel.DEBUG })
223+
224+
test('should emit a line instead of throwing on cyclic metadata', () => {
225+
const cyclic: Record<string, unknown> = { id: 'x' }
226+
cyclic.self = cyclic
227+
228+
expect(() => createEnabledLogger().info('hello', cyclic)).not.toThrow()
229+
expect(consoleLogSpy).toHaveBeenCalledTimes(1)
230+
const parsed = JSON.parse(consoleLogSpy.mock.calls[0][0] as string)
231+
expect(parsed.message).toBe('hello')
232+
expect(parsed.id).toBe('x')
233+
expect(parsed.self.self).toBe('[Circular]')
234+
})
235+
236+
test('should emit a line instead of throwing on BigInt metadata', () => {
237+
expect(() => createEnabledLogger().error('boom', { size: 10n })).not.toThrow()
238+
expect(consoleErrorSpy).toHaveBeenCalledTimes(1)
239+
const parsed = JSON.parse(consoleErrorSpy.mock.calls[0][0] as string)
240+
expect(parsed.message).toBe('boom')
241+
expect(parsed.size).toBe('10')
242+
})
243+
244+
test('should fall back to a minimal entry when a value cannot be serialized at all', () => {
245+
const hostile = {
246+
get boom() {
247+
throw new Error('getter exploded')
248+
},
249+
}
250+
251+
expect(() => createEnabledLogger().info('hello', hostile)).not.toThrow()
252+
const parsed = JSON.parse(consoleLogSpy.mock.calls[0][0] as string)
253+
expect(parsed.message).toBe('hello')
254+
expect(parsed.module).toBe('Test')
255+
expect(parsed.serializationError).toBe(true)
256+
})
257+
})
219258
})

packages/logger/src/index.ts

Lines changed: 52 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,55 @@ const formatObject = (obj: unknown, isDev: boolean): string => {
139139
}
140140
}
141141

142+
/** Merges caller-supplied log arguments into the structured entry. */
143+
const mergeArgs = (entry: Record<string, unknown>, args: unknown[]): Record<string, unknown> => {
144+
for (const arg of args) {
145+
if (arg === null || arg === undefined) continue
146+
if (arg instanceof Error) {
147+
entry.error = arg.message
148+
entry.stack = arg.stack
149+
} else if (typeof arg === 'object') {
150+
Object.assign(entry, arg)
151+
} else {
152+
entry.extra = arg
153+
}
154+
}
155+
return entry
156+
}
157+
158+
/** JSON replacer that tolerates cyclic references and BigInt values. */
159+
const tolerantReplacer = () => {
160+
const seen = new WeakSet<object>()
161+
return (_key: string, value: unknown): unknown => {
162+
if (typeof value === 'bigint') return value.toString()
163+
if (value !== null && typeof value === 'object') {
164+
if (seen.has(value)) return '[Circular]'
165+
seen.add(value)
166+
}
167+
return value
168+
}
169+
}
170+
171+
/**
172+
* Builds and serializes a production log entry without ever throwing.
173+
*
174+
* Caller-supplied arguments are merged in verbatim, so a cyclic reference, a
175+
* BigInt, or a throwing getter would otherwise raise inside the caller's code
176+
* path — losing the line and aborting whatever was being logged about. A
177+
* logger must never be able to break its caller.
178+
*/
179+
const serializeEntry = (base: Record<string, unknown>, args: unknown[]): string => {
180+
try {
181+
return JSON.stringify(mergeArgs({ ...base }, args))
182+
} catch {}
183+
184+
try {
185+
return JSON.stringify(mergeArgs({ ...base }, args), tolerantReplacer())
186+
} catch {}
187+
188+
return JSON.stringify({ ...base, serializationError: true }, tolerantReplacer())
189+
}
190+
142191
/**
143192
* Logger class for standardized console logging
144193
*
@@ -280,33 +329,17 @@ export class Logger {
280329
}
281330
} else {
282331
// Structured JSON for production — CloudWatch Log Insights auto-parses JSON lines
283-
const entry: Record<string, unknown> = {
332+
const base: Record<string, unknown> = {
284333
timestamp,
285334
level,
286335
module: this.module,
287336
message,
288337
}
289338
for (const [k, v] of metadataEntries) {
290-
entry[k] = v
291-
}
292-
// Merge extra args into the entry
293-
for (const arg of args) {
294-
if (
295-
arg !== null &&
296-
arg !== undefined &&
297-
typeof arg === 'object' &&
298-
!(arg instanceof Error)
299-
) {
300-
Object.assign(entry, arg)
301-
} else if (arg instanceof Error) {
302-
entry.error = arg.message
303-
entry.stack = arg.stack
304-
} else if (arg !== null && arg !== undefined) {
305-
entry.extra = arg
306-
}
339+
base[k] = v
307340
}
308341

309-
const line = JSON.stringify(entry)
342+
const line = serializeEntry(base, args)
310343
if (level === LogLevel.ERROR) {
311344
console.error(line)
312345
} else {

0 commit comments

Comments
 (0)