From 80f46c418fa2ef2854902240f8048f616a1268c4 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 6 Aug 2026 11:45:40 -0700 Subject: [PATCH 1/4] fix(logger): never let structured serialization throw into the caller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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`. --- packages/logger/src/index.test.ts | 39 +++++++++++++++++ packages/logger/src/index.ts | 71 ++++++++++++++++++++++--------- 2 files changed, 91 insertions(+), 19 deletions(-) diff --git a/packages/logger/src/index.test.ts b/packages/logger/src/index.test.ts index 076fd74c293..8c774f010dd 100644 --- a/packages/logger/src/index.test.ts +++ b/packages/logger/src/index.test.ts @@ -216,4 +216,43 @@ describe('Logger', () => { ) }) }) + + describe('structured serialization safety', () => { + const createEnabledLogger = () => + new Logger('Test', { enabled: true, colorize: false, logLevel: LogLevel.DEBUG }) + + test('should emit a line instead of throwing on cyclic metadata', () => { + const cyclic: Record = { id: 'x' } + cyclic.self = cyclic + + expect(() => createEnabledLogger().info('hello', cyclic)).not.toThrow() + expect(consoleLogSpy).toHaveBeenCalledTimes(1) + const parsed = JSON.parse(consoleLogSpy.mock.calls[0][0] as string) + expect(parsed.message).toBe('hello') + expect(parsed.id).toBe('x') + expect(parsed.self.self).toBe('[Circular]') + }) + + test('should emit a line instead of throwing on BigInt metadata', () => { + expect(() => createEnabledLogger().error('boom', { size: 10n })).not.toThrow() + expect(consoleErrorSpy).toHaveBeenCalledTimes(1) + const parsed = JSON.parse(consoleErrorSpy.mock.calls[0][0] as string) + expect(parsed.message).toBe('boom') + expect(parsed.size).toBe('10') + }) + + test('should fall back to a minimal entry when a value cannot be serialized at all', () => { + const hostile = { + get boom() { + throw new Error('getter exploded') + }, + } + + expect(() => createEnabledLogger().info('hello', hostile)).not.toThrow() + const parsed = JSON.parse(consoleLogSpy.mock.calls[0][0] as string) + expect(parsed.message).toBe('hello') + expect(parsed.module).toBe('Test') + expect(parsed.serializationError).toBe(true) + }) + }) }) diff --git a/packages/logger/src/index.ts b/packages/logger/src/index.ts index 25e6e3f9ae3..0de51e32656 100644 --- a/packages/logger/src/index.ts +++ b/packages/logger/src/index.ts @@ -139,6 +139,55 @@ const formatObject = (obj: unknown, isDev: boolean): string => { } } +/** Merges caller-supplied log arguments into the structured entry. */ +const mergeArgs = (entry: Record, args: unknown[]): Record => { + for (const arg of args) { + if (arg === null || arg === undefined) continue + if (arg instanceof Error) { + entry.error = arg.message + entry.stack = arg.stack + } else if (typeof arg === 'object') { + Object.assign(entry, arg) + } else { + entry.extra = arg + } + } + return entry +} + +/** JSON replacer that tolerates cyclic references and BigInt values. */ +const tolerantReplacer = () => { + const seen = new WeakSet() + return (_key: string, value: unknown): unknown => { + if (typeof value === 'bigint') return value.toString() + if (value !== null && typeof value === 'object') { + if (seen.has(value)) return '[Circular]' + seen.add(value) + } + return value + } +} + +/** + * Builds and serializes a production log entry without ever throwing. + * + * Caller-supplied arguments are merged in verbatim, so a cyclic reference, a + * BigInt, or a throwing getter would otherwise raise inside the caller's code + * path — losing the line and aborting whatever was being logged about. A + * logger must never be able to break its caller. + */ +const serializeEntry = (base: Record, args: unknown[]): string => { + try { + return JSON.stringify(mergeArgs({ ...base }, args)) + } catch {} + + try { + return JSON.stringify(mergeArgs({ ...base }, args), tolerantReplacer()) + } catch {} + + return JSON.stringify({ ...base, serializationError: true }, tolerantReplacer()) +} + /** * Logger class for standardized console logging * @@ -280,33 +329,17 @@ export class Logger { } } else { // Structured JSON for production — CloudWatch Log Insights auto-parses JSON lines - const entry: Record = { + const base: Record = { timestamp, level, module: this.module, message, } for (const [k, v] of metadataEntries) { - entry[k] = v - } - // Merge extra args into the entry - for (const arg of args) { - if ( - arg !== null && - arg !== undefined && - typeof arg === 'object' && - !(arg instanceof Error) - ) { - Object.assign(entry, arg) - } else if (arg instanceof Error) { - entry.error = arg.message - entry.stack = arg.stack - } else if (arg !== null && arg !== undefined) { - entry.extra = arg - } + base[k] = v } - const line = JSON.stringify(entry) + const line = serializeEntry(base, args) if (level === LogLevel.ERROR) { console.error(line) } else { From 3b3e5a1685e699fc2d8337b33758d61fa63cefff Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 6 Aug 2026 11:57:56 -0700 Subject: [PATCH 2/4] fix(logger): keep hostile child metadata from throwing into the caller --- packages/logger/src/index.test.ts | 41 +++++++++++++++++++++++++++++++ packages/logger/src/index.ts | 30 +++++++++++++++++++++- 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/packages/logger/src/index.test.ts b/packages/logger/src/index.test.ts index 8c774f010dd..1b8febd123b 100644 --- a/packages/logger/src/index.test.ts +++ b/packages/logger/src/index.test.ts @@ -254,5 +254,46 @@ describe('Logger', () => { expect(parsed.module).toBe('Test') expect(parsed.serializationError).toBe(true) }) + + test('should not throw when withMetadata receives a throwing getter', () => { + const hostile = { + safe: 'kept', + get boom() { + throw new Error('getter exploded') + }, + } as unknown as Parameters[0] + + let child: Logger | undefined + expect(() => { + child = createEnabledLogger().withMetadata(hostile) + }).not.toThrow() + + expect(() => child?.info('hello')).not.toThrow() + const parsed = JSON.parse(consoleLogSpy.mock.calls[0][0] as string) + expect(parsed.message).toBe('hello') + expect(parsed.safe).toBe('kept') + expect(parsed.boom).toBe('[Unreadable]') + }) + + test('should not throw when withMetadata receives a hostile proxy', () => { + const hostile = new Proxy( + {}, + { + ownKeys() { + throw new Error('ownKeys exploded') + }, + } + ) as Parameters[0] + + let child: Logger | undefined + expect(() => { + child = createEnabledLogger().withMetadata(hostile) + }).not.toThrow() + + expect(() => child?.info('hello')).not.toThrow() + const parsed = JSON.parse(consoleLogSpy.mock.calls[0][0] as string) + expect(parsed.message).toBe('hello') + expect(parsed.metadataError).toBe(true) + }) }) }) diff --git a/packages/logger/src/index.ts b/packages/logger/src/index.ts index 0de51e32656..98b15bd8146 100644 --- a/packages/logger/src/index.ts +++ b/packages/logger/src/index.ts @@ -188,6 +188,34 @@ const serializeEntry = (base: Record, args: unknown[]): string return JSON.stringify({ ...base, serializationError: true }, tolerantReplacer()) } +/** + * Copies caller-supplied metadata into a plain object without ever throwing. + * + * `LoggerMetadata` is structurally typed, so nothing stops a caller from handing + * over an object carrying a throwing getter or a hostile proxy. A spread invokes + * those traps, so the copy degrades key-by-key and finally to a marker rather + * than raising inside the caller's code path. + */ +const materializeMetadata = (metadata: LoggerMetadata): LoggerMetadata => { + try { + return { ...metadata } + } catch {} + + const safe: LoggerMetadata = {} + try { + for (const key of Object.keys(metadata)) { + try { + safe[key] = metadata[key] + } catch { + safe[key] = '[Unreadable]' + } + } + return safe + } catch {} + + return { metadataError: true } +} + /** * Logger class for standardized console logging * @@ -240,7 +268,7 @@ export class Logger { child.module = this.module child.config = this.config child.isDev = this.isDev - child.metadata = { ...this.metadata, ...metadata } + child.metadata = { ...this.metadata, ...materializeMetadata(metadata) } return child } From aea66d0432dbee9d1835998579c97607971585e2 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 6 Aug 2026 12:27:50 -0700 Subject: [PATCH 3/4] fix(logger): keep a throwing toJSON from escaping the final fallback --- packages/logger/src/index.test.ts | 19 +++++++++++++++++++ packages/logger/src/index.ts | 22 +++++++++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/packages/logger/src/index.test.ts b/packages/logger/src/index.test.ts index 1b8febd123b..8a4696d7e02 100644 --- a/packages/logger/src/index.test.ts +++ b/packages/logger/src/index.test.ts @@ -275,6 +275,25 @@ describe('Logger', () => { expect(parsed.boom).toBe('[Unreadable]') }) + test('should not throw when retained metadata has a throwing toJSON', () => { + const hostile = { + evil: { + toJSON() { + throw new Error('toJSON exploded') + }, + }, + } as unknown as Parameters[0] + + const child = createEnabledLogger().withMetadata(hostile) + + expect(() => child.info('hello')).not.toThrow() + const parsed = JSON.parse(consoleLogSpy.mock.calls[0][0] as string) + expect(parsed.message).toBe('hello') + expect(parsed.module).toBe('Test') + expect(parsed.serializationError).toBe(true) + expect(parsed.evil).toBeUndefined() + }) + test('should not throw when withMetadata receives a hostile proxy', () => { const hostile = new Proxy( {}, diff --git a/packages/logger/src/index.ts b/packages/logger/src/index.ts index 98b15bd8146..9003ade34fd 100644 --- a/packages/logger/src/index.ts +++ b/packages/logger/src/index.ts @@ -185,7 +185,27 @@ const serializeEntry = (base: Record, args: unknown[]): string return JSON.stringify(mergeArgs({ ...base }, args), tolerantReplacer()) } catch {} - return JSON.stringify({ ...base, serializationError: true }, tolerantReplacer()) + return minimalEntry(base) +} + +/** + * Last-resort entry built only from fields this module controls. + * + * A replacer cannot rescue a throwing `toJSON`, because `JSON.stringify` invokes + * it before the replacer ever sees the value. So the final fallback drops every + * caller-supplied value instead of re-serializing it, and passes strings through + * only when they are already strings — coercing would re-enter hostile + * `toString`. What remains cannot throw. + */ +const minimalEntry = (base: Record): string => { + const asString = (value: unknown) => (typeof value === 'string' ? value : '[Unserializable]') + return JSON.stringify({ + timestamp: asString(base.timestamp), + level: asString(base.level), + module: asString(base.module), + message: asString(base.message), + serializationError: true, + }) } /** From 8d5863e70cd639e1a1d84c22e5d21d0aacc1ac11 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 6 Aug 2026 13:26:30 -0700 Subject: [PATCH 4/4] fix(logger): keep repeated references out of the circular-reference fallback --- packages/logger/src/index.test.ts | 28 ++++++++++++++++++++++++++++ packages/logger/src/index.ts | 19 +++++++++++++------ 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/packages/logger/src/index.test.ts b/packages/logger/src/index.test.ts index 8a4696d7e02..78d88313267 100644 --- a/packages/logger/src/index.test.ts +++ b/packages/logger/src/index.test.ts @@ -275,6 +275,34 @@ describe('Logger', () => { expect(parsed.boom).toBe('[Unreadable]') }) + test('should keep repeated references that are not cycles', () => { + const shared = { s: 'REAL_DATA', n: 42 } + const payload = { p: shared, q: shared, arr: [shared, shared], big: 1n } as unknown as object + + createEnabledLogger().info('hello', payload) + + const parsed = JSON.parse(consoleLogSpy.mock.calls[0][0] as string) + expect(parsed.p).toEqual({ s: 'REAL_DATA', n: 42 }) + expect(parsed.q).toEqual({ s: 'REAL_DATA', n: 42 }) + expect(parsed.arr).toEqual([ + { s: 'REAL_DATA', n: 42 }, + { s: 'REAL_DATA', n: 42 }, + ]) + expect(parsed.big).toBe('1') + }) + + test('should still mark a genuine cycle as circular', () => { + const cyclic: Record = { name: 'root' } + cyclic.self = cyclic + const payload = { cyclic, big: 1n } as unknown as object + + createEnabledLogger().info('hello', payload) + + const parsed = JSON.parse(consoleLogSpy.mock.calls[0][0] as string) + expect(parsed.cyclic.name).toBe('root') + expect(parsed.cyclic.self).toBe('[Circular]') + }) + test('should not throw when retained metadata has a throwing toJSON', () => { const hostile = { evil: { diff --git a/packages/logger/src/index.ts b/packages/logger/src/index.ts index 9003ade34fd..47da5e03f0c 100644 --- a/packages/logger/src/index.ts +++ b/packages/logger/src/index.ts @@ -157,13 +157,20 @@ const mergeArgs = (entry: Record, args: unknown[]): Record { - const seen = new WeakSet() - return (_key: string, value: unknown): unknown => { + const ancestors: object[] = [] + return function (this: unknown, _key: string, value: unknown): unknown { if (typeof value === 'bigint') return value.toString() - if (value !== null && typeof value === 'object') { - if (seen.has(value)) return '[Circular]' - seen.add(value) - } + if (value === null || typeof value !== 'object') return value + /** + * Track the ancestor path, not every object ever visited. `this` is the + * object holding the current key, so unwinding to it drops the siblings we + * have finished descending. A set of everything seen would label the second + * appearance of a merely repeated reference `[Circular]` and discard real + * data, since a payload that references one object twice has no cycle. + */ + while (ancestors.length > 0 && ancestors[ancestors.length - 1] !== this) ancestors.pop() + if (ancestors.includes(value)) return '[Circular]' + ancestors.push(value) return value } }