fix(logger): never let structured serialization throw into the caller - #6331
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview
Regression tests cover cycles, BigInt, throwing getters, repeated non-cyclic refs, hostile Reviewed by Cursor Bugbot for commit 1a4b662. Configure here. |
Greptile SummaryThe PR makes production structured logging degrade safely when caller-supplied metadata cannot be serialized.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/logger/src/index.ts | Adds layered, non-throwing metadata materialization and structured-entry serialization fallbacks; the previously reported escape paths are addressed. |
| packages/logger/src/index.test.ts | Adds focused regression tests covering hostile metadata and serialization edge cases. |
Reviews (5): Last reviewed commit: "fix(logger): keep repeated references ou..." | Re-trigger Greptile
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 2463ff6. Configure here.
2463ff6 to
22e6b9b
Compare
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 22e6b9b. Configure here.
|
@cursor review |
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`.
1a4b662 to
8d5863e
Compare
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 1a4b662. Configure here.
Context
Production structured logging went fully silent for hours across all five
sim-production-us-east-1-appECS tasks —INFO/WARN/ERRORall dropped to zero per 30 min, and a raw substring search for\"module\":returned nothing. Confirmed as genuine loss viaAWS/Logs IncomingLogEvents(~25k → ~2-6k per 30 min), not a query artifact. Non-JSON output (Next.js internals, Better Auth, raw stack traces) kept shipping, and thesocketservice on the same cluster and sameawslogsdriver was unaffected — so it was localized to the app containers' own output, not the driver or the account.This PR fixes one concrete, reproducible defect on that path. It is not claimed to be the whole root cause.
The bug
In the production JSON branch,
Logger.logmerged caller-supplied arguments into the entry withObject.assignand then calledJSON.stringify, with no error handling on either step. If that metadata contained a cyclic reference, aBigInt, or a throwing getter, the call raised aTypeErrorout oflogger.info(...)into the caller — the line was lost and whatever was being logged about was aborted.Verified against the current
staginglogger underNODE_ENV=production:Cyclic metadata is easy to hit unintentionally — errors carrying
request/response/causechains, ORM rows with back-references, socket handles.The development path was never affected: the colorized branch routes objects through
formatObject, which already has atry/catch. So this class of bug is invisible locally and only appears in production, where its signature is exactly structured logs vanish while raw stack traces keep shipping.The fix
Entry construction and serialization now go through
serializeEntry, which degrades instead of throwing:JSON.stringify[Circular]) andBigInt(stringified)timestamp/level/module/message, flaggedserializationError: trueA logger must never be able to break its caller.
Tests
Three regression tests in
packages/logger/src/index.test.tscovering cyclic,BigInt, and throwing-getter metadata. All three were confirmed to fail against the unfixed source (git stashonindex.ts→3 failed | 25 passed) and pass with it (28 passed).bun run lintandtsc --noEmitare clean.Follow-ups (not in this repo)
Alarms and ECS task definitions live in the private sibling CDK repo, not here. Two gaps found there and reported separately:
AWS/Logs IncomingLogEventsfor the app log group. Every metric filter over that group sees zero matches during a blackout and sits inOK— a silent alarm indistinguishable from healthy. A log-absence alarm withtreatMissingData: BREACHINGis specified in the report.LogDrivers.awsLogs({ ... })calls omitmode, so both default to blocking.