fix(runtime-node): report flush failures correctly - #18
Autter review gate blocked
Autter blocked this PR after its agentic checks (build/test/deep scans) completed: 3 confirmed correctness/runtime finding(s). See the findings below and the full PR review for details.
Details
Findings
10 finding(s) — 6 high · 4 medium · 0 low.
🔴 High (6)
- Unbounded redaction recursion can throw into capture callers —
packages/runtime-node/src/redact.ts:149· risk 79 (ai)
The changed redactValue recursively descends every nested array/object with no depth or work bound (line 149). Consequently a sufficiently deeply nested but acyclic caller-supplied Attributes value throws RangeError before redaction returns. initAutterServer installs the compiled redactor (line 719), and both its captureException and captureMessage callers spread activeRedactor(attributes) without a guard (lines 824 and 868); the captured error/message is therefore not emitted and the synchronous capture API can unexpectedly throw into application or global-error call chains. The previous contract bounded traversal at four levels. - Unbounded recursive redaction can overflow on caller-supplied attributes —
packages/runtime-node/src/redact.ts:149· risk 78 (ai)
The new deep traversal recurses once per array/object nesting level with no depth or work bound.redactAttributesand the compiledactiveRedactorboth invoke this path for caller-provided attributes; notablyinitAutterServerconfigures it at server startup andcaptureExceptionpasses its attributes into it before creating the span. An acyclic object (or array) nested beyond the JavaScript stack limit therefore throwsRangeErrorat this recursive call instead of returning safe attributes, causing the telemetry capture path itself to fail. The WeakMap only terminates cycles and does not protect this reachable acyclic input. - Unbounded recursive redaction can throw during telemetry capture —
packages/runtime-node/src/redact.ts:149· risk 78 (ai)
The new deep traversal callsredactValuerecursively for every nested array element and object property with no depth or work limit. A caller can supply an acyclic attribute nested beyond the JavaScript stack limit; that producesRangeErrorbeforeredactAttributesreturns. The active capture path spreadsactiveRedactor(attributes)while creating error spans (server.ts:819-825), so this failure escapescaptureExceptionand prevents the exception from being recorded. WeakMap only breaks cycles and cannot protect an arbitrarily deep acyclic value. - Silent exception swallowing —
packages/runtime-node/src/lifecycle.ts:209· risk 76
installAutterAutoFlushstill turns eachFlushTarget.forceFlushinto a promise and only inspects settled statuses. If a target implementation internally catches or absorbs exporter failures, this aggregate will treat the flush as successful and calltelemetryStats.markAllFlushedforactiveFlushTargetseven though data was lost. Blast radius — if this failure path is hit it cascades to the downstream usage that depends on this file: functionsinstallAutterAutoFlush,unregisterFlushTargets,debugLog,CountingExporter.forceFlush,TelemetryStats.markCaptured,isDebugEnabled,redactAttributes,makeRedactor; scopes@autter/runtime-node; dependent files@opentelemetry/core,@opentelemetry/sdk-trace-base. - Unbounded recursive redaction can overflow the stack on deep acyclic values —
packages/runtime-node/src/redact.ts:142· risk 75 (ai)
Cycle detection handles circular references, but removingmaxDepthmeansredactValuenow recursively walks every level of an acyclic object or array. A deeply nested runtime value passed to the exported redactor can exhaust the JavaScript call stack and throw into the host application. Retain a traversal-depth/node budget or use an iterative traversal while preserving cycle handling. - PR mixes refactor and behavior change —
packages/runtime-node/src/server.ts:907· risk 74
This PR mixes a behavior fix for auto-flush failure reporting with a separate behavior change in redaction logic.packages/runtime-node/src/server.tsandpackages/runtime-node/src/lifecycle.tsare the files contributing most to the mix. Split into one PR for the runtime-node auto-flush failure propagation inpackages/runtime-node/src/server.tsandpackages/runtime-node/src/lifecycle.tspluspackages/runtime-node/test/lifecycle.test.mjs, and a second PR for the recursive/circular attribute redaction changes inpackages/runtime-node/src/redact.tswithpackages/runtime-node/test/redact.test.mjs. The server flush path is higher risk becausepackages/runtime-node/src/server.tshas blastRadiusScore 45 and fanIn 1, so it deserves isolated review. Blast radius — if the refactor introduces a regression the behavior change masks it on: functionsinitAutterServer,recordLlmCall,captureException,ErrorTraceRetentionProcessor.forceFlush,ErrorTraceRetentionProcessor.flush,runWithSpan,installAutterAutoFlush,redactAttributes; scopes@autter/runtime-node; dependent files./lifecycle.js,./redact.js,@opentelemetry/api,@opentelemetry/core,@opentelemetry/exporter-metrics-otlp-http,@opentelemetry/exporter-trace-otlp-http,@opentelemetry/instrumentation-http,@opentelemetry/resources.
🟠 Medium (4)
packages/runtime-node/README.mdusually changes with this file —packages/runtime-node/src/server.ts:910· risk 60 (deterministic)
packages/runtime-node/src/server.tsandpackages/runtime-node/README.mdchanged together in 6 of the last 6 commits that touched either (100%), butpackages/runtime-node/README.mdis not in this PR. This is history, not a rule — if the coupling no longer applies, ignore it. It most often means a matching change was missed (a caller, a type, a fixture, a migration's rollback).- Missing CODEOWNERS reviewer approval —
packages/runtime-node/src/redact.ts:134· risk 49
redactAttributes/makeRedactorchanged inpackages/runtime-node/src/redact.ts, but there is no approving reviewer from the code owner. This affects the exported redaction contract used bycaptureExceptionand any downstream sinks that rely on the sanitized attribute shape. Blast radius — skipping this guardrail cascades to the downstream usage that depends on this file: functionsredactAttributes,makeRedactor,redactString,redactValue,isSensitiveKey,redactWith,installAutterAutoFlush,initAutterServer; scopes@autter/runtime-node; dependent files@opentelemetry/api. - Missing CODEOWNERS reviewer approval —
packages/runtime-node/test/lifecycle.test.mjs:5· risk 48
The new regression coverage forinstallAutterAutoFlush.flushand the built-in-style flush target was added without any approving reviewer from the relevant owner set. Because this test guards the public shutdown behavior, it should be reviewed alongside the implementation change it protects. - Missing CODEOWNERS reviewer approval —
packages/runtime-node/test/redact.test.mjs:128· risk 42
The addedredactAttributesregression tests cover nested and circular attribute handling, but there is no approving reviewer from the file owner. These tests anchor the exported redaction behavior consumed bycaptureExceptionand server-side attribute processing. Blast radius — skipping this guardrail cascades to the downstream usage that depends on this file: scopes@autter/runtime-node; dependent files../dist/index.js,node:assert/strict,node:test.
Annotations
Check failure on line 209 in packages/runtime-node/src/lifecycle.ts
autter-dev / autter/review-gate
🔴 High · Silent exception swallowing
`installAutterAutoFlush` still turns each `FlushTarget.forceFlush` into a promise and only inspects settled statuses. If a target implementation internally catches or absorbs exporter failures, this aggregate will treat the flush as successful and call `telemetryStats.markAllFlushed` for `activeFlushTargets` even though data was lost. Blast radius — if this failure path is hit it cascades to the downstream usage that depends on this file: functions `installAutterAutoFlush`, `unregisterFlushTargets`, `debugLog`, `CountingExporter.forceFlush`, `TelemetryStats.markCaptured`, `isDebugEnabled`, `redactAttributes`, `makeRedactor`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/core`, `@opentelemetry/sdk-trace-base`.
Suggested fix:
Require each flush target to preserve failure information from its internal exporter calls, or have this aggregate reject/return false whenever any target reports a hidden exporter failure instead of only checking the outer promise state. Blast radius — if this failure path is hit it cascades to the downstream usage that depends on this file: functions `installAutterAutoFlush`, `unregisterFlushTargets`, `debugLog`, `CountingExporter.forceFlush`, `TelemetryStats.markCaptured`, `isDebugEnabled`, `redactAttributes`, `makeRedactor`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/core`, `@opentelemetry/sdk-trace-base`.
Check failure on line 142 in packages/runtime-node/src/redact.ts
autter-dev / autter/review-gate
🔴 High · Unbounded recursive redaction can overflow the stack on deep acyclic values
Cycle detection handles circular references, but removing `maxDepth` means `redactValue` now recursively walks every level of an acyclic object or array. A deeply nested runtime value passed to the exported redactor can exhaust the JavaScript call stack and throw into the host application. Retain a traversal-depth/node budget or use an iterative traversal while preserving cycle handling.
Suggested fix:
Preserve the previous depth limit while adding cycle detection: thread a `depth` parameter through `redactValue`, stop recursing when depth reaches 0, and only use the `WeakMap` to break cycles. Keep the existing whitelist/sensitive-key masking behavior unchanged. Blast radius — if this defect reaches production it can fail the downstream usage that depends on this file: functions `redactAttributes`, `makeRedactor`, `redactString`, `redactValue`, `isSensitiveKey`, `redactWith`, `installAutterAutoFlush`, `initAutterServer`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/api`.
Check failure on line 907 in packages/runtime-node/src/server.ts
autter-dev / autter/review-gate
🔴 High · PR mixes refactor and behavior change
This PR mixes a behavior fix for auto-flush failure reporting with a separate behavior change in redaction logic. `packages/runtime-node/src/server.ts` and `packages/runtime-node/src/lifecycle.ts` are the files contributing most to the mix. Split into one PR for the runtime-node auto-flush failure propagation in `packages/runtime-node/src/server.ts` and `packages/runtime-node/src/lifecycle.ts` plus `packages/runtime-node/test/lifecycle.test.mjs`, and a second PR for the recursive/circular attribute redaction changes in `packages/runtime-node/src/redact.ts` with `packages/runtime-node/test/redact.test.mjs`. The server flush path is higher risk because `packages/runtime-node/src/server.ts` has blastRadiusScore 45 and fanIn 1, so it deserves isolated review. Blast radius — if the refactor introduces a regression the behavior change masks it on: functions `initAutterServer`, `recordLlmCall`, `captureException`, `ErrorTraceRetentionProcessor.forceFlush`, `ErrorTraceRetentionProcessor.flush`, `runWithSpan`, `installAutterAutoFlush`, `redactAttributes`; scopes `@autter/runtime-node`; dependent files `./lifecycle.js`, `./redact.js`, `@opentelemetry/api`, `@opentelemetry/core`, `@opentelemetry/exporter-metrics-otlp-http`, `@opentelemetry/exporter-trace-otlp-http`, `@opentelemetry/instrumentation-http`, `@opentelemetry/resources`.
Suggested fix:
Split this PR so refactors land separately from behavior changes.
Pure-refactor PRs should preserve behavior (no test changes beyond renames). Behavior-change PRs should focus on a single new capability.
Start by extracting `packages/runtime-node/src/server.ts`'s refactor portion (or its behavior portion, whichever is smaller) into its own PR.
Check warning on line 910 in packages/runtime-node/src/server.ts
autter-dev / autter/review-gate
🟠 Medium · `packages/runtime-node/README.md` usually changes with this file
`packages/runtime-node/src/server.ts` and `packages/runtime-node/README.md` changed together in 6 of the last 6 commits that touched either (100%), but `packages/runtime-node/README.md` is not in this PR.
This is history, not a rule — if the coupling no longer applies, ignore it. It most often means a matching change was missed (a caller, a type, a fixture, a migration's rollback).
Suggested fix:
In this repository, `packages/runtime-node/src/server.ts` and `packages/runtime-node/README.md` have historically changed together (6 shared commits, 100% co-change rate). The current change modifies `packages/runtime-node/src/server.ts` only. Open `packages/runtime-node/README.md` and determine whether it needs a corresponding change. If it does, make it. If it genuinely does not, explain why the coupling no longer holds.
Check warning on line 134 in packages/runtime-node/src/redact.ts
autter-dev / autter/review-gate
🟠 Medium · Missing CODEOWNERS reviewer approval
`redactAttributes`/`makeRedactor` changed in `packages/runtime-node/src/redact.ts`, but there is no approving reviewer from the code owner. This affects the exported redaction contract used by `captureException` and any downstream sinks that rely on the sanitized attribute shape. Blast radius — skipping this guardrail cascades to the downstream usage that depends on this file: functions `redactAttributes`, `makeRedactor`, `redactString`, `redactValue`, `isSensitiveKey`, `redactWith`, `installAutterAutoFlush`, `initAutterServer`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/api`.
Suggested fix:
Request an approval from the CODEOWNERS owner for `packages/runtime-node/src/redact.ts` and keep that approval current on the latest revision. Blast radius — skipping this guardrail cascades to the downstream usage that depends on this file: functions `redactAttributes`, `makeRedactor`, `redactString`, `redactValue`, `isSensitiveKey`, `redactWith`, `installAutterAutoFlush`, `initAutterServer`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/api`.
Check warning on line 5 in packages/runtime-node/test/lifecycle.test.mjs
autter-dev / autter/review-gate
🟠 Medium · Missing CODEOWNERS reviewer approval
The new regression coverage for `installAutterAutoFlush.flush` and the built-in-style flush target was added without any approving reviewer from the relevant owner set. Because this test guards the public shutdown behavior, it should be reviewed alongside the implementation change it protects.
Suggested fix:
Obtain an approving review from the owner responsible for `packages/runtime-node` changes before merging.
Check warning on line 128 in packages/runtime-node/test/redact.test.mjs
autter-dev / autter/review-gate
🟠 Medium · Missing CODEOWNERS reviewer approval
The added `redactAttributes` regression tests cover nested and circular attribute handling, but there is no approving reviewer from the file owner. These tests anchor the exported redaction behavior consumed by `captureException` and server-side attribute processing. Blast radius — skipping this guardrail cascades to the downstream usage that depends on this file: scopes `@autter/runtime-node`; dependent files `../dist/index.js`, `node:assert/strict`, `node:test`.
Suggested fix:
Request approval from the CODEOWNERS owner covering `packages/runtime-node/test/redact.test.mjs`. Blast radius — skipping this guardrail cascades to the downstream usage that depends on this file: scopes `@autter/runtime-node`; dependent files `../dist/index.js`, `node:assert/strict`, `node:test`.
Check failure on line 149 in packages/runtime-node/src/redact.ts
autter-dev / autter/review-gate
🔴 High · Unbounded redaction recursion can throw into capture callers
The changed redactValue recursively descends every nested array/object with no depth or work bound (line 149). Consequently a sufficiently deeply nested but acyclic caller-supplied Attributes value throws RangeError before redaction returns. initAutterServer installs the compiled redactor (line 719), and both its captureException and captureMessage callers spread activeRedactor(attributes) without a guard (lines 824 and 868); the captured error/message is therefore not emitted and the synchronous capture API can unexpectedly throw into application or global-error call chains. The previous contract bounded traversal at four levels.
Suggested fix:
Use an iterative traversal with an explicit depth/work limit, or restore a safe depth bound while preserving cycle handling. Ensure values beyond the limit are returned or replaced safely so activeRedactor never throws for arbitrary Attributes.
Check failure on line 149 in packages/runtime-node/src/redact.ts
autter-dev / autter/review-gate
🔴 High · Unbounded recursive redaction can overflow on caller-supplied attributes
The new deep traversal recurses once per array/object nesting level with no depth or work bound. `redactAttributes` and the compiled `activeRedactor` both invoke this path for caller-provided attributes; notably `initAutterServer` configures it at server startup and `captureException` passes its attributes into it before creating the span. An acyclic object (or array) nested beyond the JavaScript stack limit therefore throws `RangeError` at this recursive call instead of returning safe attributes, causing the telemetry capture path itself to fail. The WeakMap only terminates cycles and does not protect this reachable acyclic input.
Suggested fix:
Bound redaction traversal with a maximum depth/work budget, or rewrite it iteratively. Preserve cycle handling and ensure values beyond the budget are returned or replaced safely without throwing.
Check failure on line 149 in packages/runtime-node/src/redact.ts
autter-dev / autter/review-gate
🔴 High · Unbounded recursive redaction can throw during telemetry capture
The new deep traversal calls `redactValue` recursively for every nested array element and object property with no depth or work limit. A caller can supply an acyclic attribute nested beyond the JavaScript stack limit; that produces `RangeError` before `redactAttributes` returns. The active capture path spreads `activeRedactor(attributes)` while creating error spans (server.ts:819-825), so this failure escapes `captureException` and prevents the exception from being recorded. WeakMap only breaks cycles and cannot protect an arbitrarily deep acyclic value.
Suggested fix:
Replace recursive traversal with an iterative walk bounded by an explicit depth/work budget, or restore a safe maximum depth. Preserve the copy/no-mutation and cycle behavior, and make inputs beyond the budget resolve to a safe non-throwing value that cannot leak nested secrets.
Check failure on line 209 in packages/runtime-node/src/lifecycle.ts
autter-dev / autter/review-gate
🔴 High · Silent exception swallowing
`installAutterAutoFlush` still turns each `FlushTarget.forceFlush` into a promise and only inspects settled statuses. If a target implementation internally catches or absorbs exporter failures, this aggregate will treat the flush as successful and call `telemetryStats.markAllFlushed` for `activeFlushTargets` even though data was lost. Blast radius — if this failure path is hit it cascades to the downstream usage that depends on this file: functions `installAutterAutoFlush`, `unregisterFlushTargets`, `debugLog`, `CountingExporter.forceFlush`, `TelemetryStats.markCaptured`, `isDebugEnabled`, `redactAttributes`, `makeRedactor`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/core`, `@opentelemetry/sdk-trace-base`.
Suggested fix:
Require each flush target to preserve failure information from its internal exporter calls, or have this aggregate reject/return false whenever any target reports a hidden exporter failure instead of only checking the outer promise state. Blast radius — if this failure path is hit it cascades to the downstream usage that depends on this file: functions `installAutterAutoFlush`, `unregisterFlushTargets`, `debugLog`, `CountingExporter.forceFlush`, `TelemetryStats.markCaptured`, `isDebugEnabled`, `redactAttributes`, `makeRedactor`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/core`, `@opentelemetry/sdk-trace-base`.
Check failure on line 142 in packages/runtime-node/src/redact.ts
autter-dev / autter/review-gate
🔴 High · Unbounded recursive redaction can overflow the stack on deep acyclic values
Cycle detection handles circular references, but removing `maxDepth` means `redactValue` now recursively walks every level of an acyclic object or array. A deeply nested runtime value passed to the exported redactor can exhaust the JavaScript call stack and throw into the host application. Retain a traversal-depth/node budget or use an iterative traversal while preserving cycle handling.
Suggested fix:
Preserve the previous depth limit while adding cycle detection: thread a `depth` parameter through `redactValue`, stop recursing when depth reaches 0, and only use the `WeakMap` to break cycles. Keep the existing whitelist/sensitive-key masking behavior unchanged. Blast radius — if this defect reaches production it can fail the downstream usage that depends on this file: functions `redactAttributes`, `makeRedactor`, `redactString`, `redactValue`, `isSensitiveKey`, `redactWith`, `installAutterAutoFlush`, `initAutterServer`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/api`.
Check failure on line 907 in packages/runtime-node/src/server.ts
autter-dev / autter/review-gate
🔴 High · PR mixes refactor and behavior change
This PR mixes a behavior fix for auto-flush failure reporting with a separate behavior change in redaction logic. `packages/runtime-node/src/server.ts` and `packages/runtime-node/src/lifecycle.ts` are the files contributing most to the mix. Split into one PR for the runtime-node auto-flush failure propagation in `packages/runtime-node/src/server.ts` and `packages/runtime-node/src/lifecycle.ts` plus `packages/runtime-node/test/lifecycle.test.mjs`, and a second PR for the recursive/circular attribute redaction changes in `packages/runtime-node/src/redact.ts` with `packages/runtime-node/test/redact.test.mjs`. The server flush path is higher risk because `packages/runtime-node/src/server.ts` has blastRadiusScore 45 and fanIn 1, so it deserves isolated review. Blast radius — if the refactor introduces a regression the behavior change masks it on: functions `initAutterServer`, `recordLlmCall`, `captureException`, `ErrorTraceRetentionProcessor.forceFlush`, `ErrorTraceRetentionProcessor.flush`, `runWithSpan`, `installAutterAutoFlush`, `redactAttributes`; scopes `@autter/runtime-node`; dependent files `./lifecycle.js`, `./redact.js`, `@opentelemetry/api`, `@opentelemetry/core`, `@opentelemetry/exporter-metrics-otlp-http`, `@opentelemetry/exporter-trace-otlp-http`, `@opentelemetry/instrumentation-http`, `@opentelemetry/resources`.
Suggested fix:
Split this PR so refactors land separately from behavior changes.
Pure-refactor PRs should preserve behavior (no test changes beyond renames). Behavior-change PRs should focus on a single new capability.
Start by extracting `packages/runtime-node/src/server.ts`'s refactor portion (or its behavior portion, whichever is smaller) into its own PR.
Check warning on line 910 in packages/runtime-node/src/server.ts
autter-dev / autter/review-gate
🟠 Medium · `packages/runtime-node/README.md` usually changes with this file
`packages/runtime-node/src/server.ts` and `packages/runtime-node/README.md` changed together in 6 of the last 6 commits that touched either (100%), but `packages/runtime-node/README.md` is not in this PR.
This is history, not a rule — if the coupling no longer applies, ignore it. It most often means a matching change was missed (a caller, a type, a fixture, a migration's rollback).
Suggested fix:
In this repository, `packages/runtime-node/src/server.ts` and `packages/runtime-node/README.md` have historically changed together (6 shared commits, 100% co-change rate). The current change modifies `packages/runtime-node/src/server.ts` only. Open `packages/runtime-node/README.md` and determine whether it needs a corresponding change. If it does, make it. If it genuinely does not, explain why the coupling no longer holds.
Check warning on line 134 in packages/runtime-node/src/redact.ts
autter-dev / autter/review-gate
🟠 Medium · Missing CODEOWNERS reviewer approval
`redactAttributes`/`makeRedactor` changed in `packages/runtime-node/src/redact.ts`, but there is no approving reviewer from the code owner. This affects the exported redaction contract used by `captureException` and any downstream sinks that rely on the sanitized attribute shape. Blast radius — skipping this guardrail cascades to the downstream usage that depends on this file: functions `redactAttributes`, `makeRedactor`, `redactString`, `redactValue`, `isSensitiveKey`, `redactWith`, `installAutterAutoFlush`, `initAutterServer`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/api`.
Suggested fix:
Request an approval from the CODEOWNERS owner for `packages/runtime-node/src/redact.ts` and keep that approval current on the latest revision. Blast radius — skipping this guardrail cascades to the downstream usage that depends on this file: functions `redactAttributes`, `makeRedactor`, `redactString`, `redactValue`, `isSensitiveKey`, `redactWith`, `installAutterAutoFlush`, `initAutterServer`; scopes `@autter/runtime-node`; dependent files `@opentelemetry/api`.
Check warning on line 5 in packages/runtime-node/test/lifecycle.test.mjs
autter-dev / autter/review-gate
🟠 Medium · Missing CODEOWNERS reviewer approval
The new regression coverage for `installAutterAutoFlush.flush` and the built-in-style flush target was added without any approving reviewer from the relevant owner set. Because this test guards the public shutdown behavior, it should be reviewed alongside the implementation change it protects.
Suggested fix:
Obtain an approving review from the owner responsible for `packages/runtime-node` changes before merging.
Check warning on line 128 in packages/runtime-node/test/redact.test.mjs
autter-dev / autter/review-gate
🟠 Medium · Missing CODEOWNERS reviewer approval
The added `redactAttributes` regression tests cover nested and circular attribute handling, but there is no approving reviewer from the file owner. These tests anchor the exported redaction behavior consumed by `captureException` and server-side attribute processing. Blast radius — skipping this guardrail cascades to the downstream usage that depends on this file: scopes `@autter/runtime-node`; dependent files `../dist/index.js`, `node:assert/strict`, `node:test`.
Suggested fix:
Request approval from the CODEOWNERS owner covering `packages/runtime-node/test/redact.test.mjs`. Blast radius — skipping this guardrail cascades to the downstream usage that depends on this file: scopes `@autter/runtime-node`; dependent files `../dist/index.js`, `node:assert/strict`, `node:test`.