Move Content Sanitizer Lambda RequestId out of the aggregatable error message - #88
Merged
Merged
Conversation
invocationId is unique per Lambda invocation, so including it on error and critical log entries defeated log-aggregation/error-tracking tools that group errors by identical fields. info/warn/track logs are unaffected and still carry it for correlation.
invocationId was never embedded in the log message/title text, so omitting it from error/critical entries wasn't necessary to keep aggregation working — it's a separate structured field. Revert to always including it for request correlation.
…ssages Sandbox.Timedout (and other sandbox) errors embed a fresh "RequestId: <uuid>" in errorMessage on every invocation. That was flowing straight into the DbError message text logged by handler.ts, so every timeout produced a differently-worded log line and defeated aggregation by message.
Instead of dropping the sandbox error's RequestId entirely, pull it into a new DbError.lambdaRequestId field. It still reaches the logs (handler.ts already logs the whole error object as structured context), it just no longer varies the message text used for aggregation.
wparad
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Sandbox.Timedout) embed a freshRequestId: <uuid>inerrorMessageon every Lambda invocation.LambdaContentSanitizer.extractInvocationErrorDetail()(src/processor/content-sanitizer-client.ts) was folding that text straight into theDbErrormessage, whichhandler.tsthen logs verbatim as the error's title — so every timeout produced a differently-worded log line (e.g.RequestId: 0fdb18b6-f166-4949-b101-e5e2e495fe54 Error: Task timed out after 10.00 seconds), defeating log aggregation by message.DbError.lambdaRequestIdstructured field.dbError()(src/errors.ts) now takes an optional{ lambdaRequestId }to set it.handler.tsalready logs the whole error object (error: result.error) as structured JSON context alongside the message title, solambdaRequestIdstill reaches the logs — it just lives in its own field instead of being baked into the message text, so the title stays stable across invocations and errors aggregate correctly (e.g.Content Sanitizer Lambda error: Unhandled — Sandbox.Timedout: Error: Task timed out after 10.00 seconds).Test plan
npx eslint src/npx tsc --noEmit -p tsconfig.check.jsonnpx vitest run tests/processor/content-sanitizer-client.spec.ts— covers the RequestId being moved intolambdaRequestIdand stripped from the message, and a pass-through case with no embedded RequestIdnpm test(lint + typecheck + full vitest suite) — all 2846 tests pass🤖 Generated with Claude Code