Skip to content

fix(telemetry): preserve fatal process semantics - #3589

Open
GautamSharma99 wants to merge 1 commit into
letta-ai:mainfrom
GautamSharma99:fix/telemetry-fatal-termination
Open

fix(telemetry): preserve fatal process semantics#3589
GautamSharma99 wants to merge 1 commit into
letta-ai:mainfrom
GautamSharma99:fix/telemetry-fatal-termination

Conversation

@GautamSharma99

Copy link
Copy Markdown

Summary

Fixes #3579.

Telemetry previously registered uncaughtException and unhandledRejection listeners that recorded the failure and drained events, but never restored the runtime fatal outcome. In Bun, the presence of those listeners suppresses default termination, so a long-running letta-code process could continue after an uncaught failure with partially mutated state or broken invariants.

This PR makes fatal telemetry best-effort without making fatal errors recoverable.

What changed

  • Extracted process-fatal handling into a focused installFatalErrorHandlers helper.
  • Sets process.exitCode = 1 synchronously as soon as either fatal event arrives.
  • Uses a shared one-shot guard across uncaught exceptions and unhandled rejections so recursive failures do not enqueue duplicate fatal events or start competing drains.
  • Attempts to record the first actionable fatal event, then races telemetry draining against a hard 3-second timeout.
  • Calls process.exit(1) after the drain settles, fails, or times out, ensuring active sockets and timers cannot keep the process alive.
  • Keeps existing noise filtering for broken-pipe errors and rate-limit rejections, but separates reporting policy from termination policy: filtered uncaught failures still terminate non-zero.
  • Stores and removes SIGINT and fatal listener references during telemetry.cleanup(), preventing duplicate process listeners across cleanup/reinitialization cycles.
  • Updated the stale crash-telemetry comment to reflect the new bounded-flush behavior and the remaining persistence limitation.

Why explicit termination

Setting only process.exitCode is insufficient for listener/headless processes because active handles can keep the event loop alive indefinitely. Removing the listener and rethrowing also creates recursive/error-ordering hazards while an asynchronous flush is running. A bounded flush followed by explicit non-zero termination gives telemetry a short delivery window while preserving deterministic fatal semantics.

The exit code is set before tracking or draining so failures inside telemetry itself cannot accidentally produce a successful exit. Tracking and drain errors are deliberately contained because neither should interfere with termination.

Tests

Added real child-Bun-process coverage rather than mocking process.exit:

  • uncaught exceptions attempt a drain and exit with code 1;
  • unhandled rejections attempt a drain and exit with code 1;
  • a drain promise that never settles is cut off by the hard timeout;
  • a recursive fatal event records only the first failure;
  • filtered fatal reasons are not reported but still exit non-zero;
  • cleanup removes both installed fatal listeners.

Validation completed:

  • bun test src/telemetry/fatal-error-handler.test.ts — 6 passed
  • focused telemetry suites — 20 passed before the final additional filtered-error case
  • bun run check — all 12 repository checks passed

Scope

Normal SIGINT shutdown retains its existing successful exit behavior. This change only affects uncaught exceptions and unhandled rejections, which now reliably remain fatal after telemetry is initialized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Telemetry fatal-error handlers allow the process to continue after uncaught errors

1 participant