Skip to content

An abandoned request still crashes the server: #2317 did not cover every WorkerDbAbortError path #2483

Description

@ateles-agent

Summary

The hosted instance crash-looped on 2026-09-23 with the same unhandled WorkerDbAbortError that #2316 describes, on a build that contains #2317, the fix for #2316. So #2317 closed one path and at least one more remains.

Evidence

[neotoma] exit code=1 uptime=21.6s
file:///app/dist/repositories/worker/worker_file_database.js:545
    this.abandon(new WorkerDbAbortError("DB request aborted by caller"));
WorkerDbAbortError: DB request aborted by caller
    at EventTarget.onAbort (worker_file_database.js:545:34)
    at AbortController.abort (node:internal/abort_controller:392:5)
    at ServerResponse.onClose (middleware/db_abort_context.js:32:28)
    at Socket.onServerResponseClose (node:_http_server:277:5)
Node.js v20.20.2
  • Immediately before each exit the log shows DB reader pool recovered after 1714ms of saturation, and requests taking ~2.8s.
  • Fly event log: repeated exit_code=134 roughly 20–30s after each start, until Fly stopped restarting. Subsequent manual starts were refused with resource_exhausted: rate limit exceeded.

Mechanism (as far as the logs show)

Load saturates the reader pool; callers time out and close their connections; db_abort_context aborts the in-flight DB request; the abort surfaces as an exception nothing catches, and the process exits. On restart the same backlog of retrying clients arrives at once, so the loop reproduces within seconds. A client disconnect is an ordinary event and must not be able to end the process.

What to check

  • Every path by which abandon(new WorkerDbAbortError(...)) can reject a promise that no caller awaits — the fix(db): an abandoned request must not kill the server (#2316) #2317 fix covered one; the stack above is the onAbort listener path in worker_file_database.
  • Whether the rejection escapes because the request that owned it has already finished or detached, leaving the promise orphaned.
  • A regression test that reproduces a mid-query client disconnect under reader-pool saturation, and fails on a80340c66. A test that passes on the crashing build is not coverage for this.

Related: #2316 (same symptom, closed), #2317 (its fix), #2322 (reader-pool exhaustion), #2339 (interrupting abandoned reads).

Swarm specification

This section is maintained by the Ateles swarm. Each lens agent owns exactly one subsection below; the human-written description above these markers is never modified.

Product / Scope (PM)

The plan_contribution entity stored cleanly with unknown_fields_count: 0. Since there's no issue entity for neotoma#2483 in Neotoma yet (only derived task entities), I can't correct() a gate_status.pm field on an issue entity that doesn't exist here — that gate state lives on the GitHub issue itself, which the dispatcher/Anthus manages outside this MCP's entity graph. The plan_contribution sign-off record is the artifact I can write; I'll note this in my verdict rather than fabricate a correct() call against a nonexistent entity.

[pavo] acceptance_criteria: PM scoping signed off. Problem: #2317 closed one WorkerDbAbortError/abandon() escape path; this issue proves at least one more exists (2026-09-23 hosted crash-loop on a build containing #2317, exact stack in worker_file_database.js:545 onAbort). Scope: full audit of abandon(new WorkerDbAbortError(...)) call sites, root-cause fix for the orphaned-promise rejection, regression test reproducing a mid-query disconnect under reader-pool saturation that is red on a80340c66. Out of scope: reader-pool saturation itself (#2322), read cancellation (#2339); containment PR #2484 explicitly does not satisfy closure here. Acceptance checklist and priority/sequencing posted in the spec section. Localized bug fix, no interface-surface change — bug fast path to Eng/impl applies, no forced arch gate. plan_contribution sign-off stored as ent_b8d16387086f36d3c907eeb3.

Engineering

Prior art check: Containment PR #2484 already exists against this issue (qa: signed_off, pr_review: changes_requested per gate_status) but is explicitly recorded as not satisfying closure — do not treat it as the fix. Its diff should be read as a first data point (what it changed and why reviewers requested changes) but this plan targets the root cause, not the containment patch.

Root-cause hypothesis. #2317 added guardAbandonedRead() — a synchronous no-op .catch() — at the outermost public boundary (WorkerStatement.get/.all in src/repositories/worker/worker_file_database.ts). That should mark the entire derived promise chain as handled, including the promise WorkerConnection.request() constructs and rejects via onAbort (~line 617 TS / dist line 545). Since the crash still reproduces on a build containing that fix, the guard is not covering this call. Two concrete candidates, in priority order:

  1. PR Rust panic in neon binding aborts the process when a client disconnects mid-statement (#2316 fix incomplete) #2324 ("retire an abandoned reader instead of terminating it mid-native-call", merged 2026-09-08, one day after fix(db): an abandoned request must not kill the server (#2316) #2317) reworked reader retirement/orphan-draining. This is the most likely place a second, unguarded promise was introduced — e.g. a retirement/drain path that re-issues or re-wraps the read outside the WorkerStatement.get/.all boundary, or a case where failInFlight/abandon now rejects an entry that isn't the one guardAbandonedRead attached to.
  2. A caller that bypasses WorkerStatement entirely — some other code path obtaining a prepared statement or connection handle and calling into routeRead/dispatchRead/WorkerConnection.request directly, skipping the guarded boundary.

Files/modules to touch:

  • src/repositories/worker/worker_file_database.ts — primary fix location. Specifically: WorkerConnection.request() (the onAbort and timeout branches, ~line 600-626), dispatchRead/routeRead (async boundary layers), WorkerStatement.get/.all (existing guardAbandonedRead call site), and whatever retirement/drain logic PR Rust panic in neon binding aborts the process when a client disconnects mid-statement (#2316 fix incomplete) #2324 added (grep for retire, drain, orphan in this file).
  • src/middleware/db_abort_context.ts — read-only reference during investigation (unchanged since Two slow reads exhaust the 2-worker reader pool and hang every query; no statement timeout reclaims a reader #2217, not expected to need changes, but confirm the AbortController it creates isn't being reused/shared across requests in a way that causes cross-request rejection).
  • Test file(s) alongside worker_file_database.ts (repo's existing convention, e.g. worker_file_database.test.ts or equivalent under a __tests__/test dir — confirm exact path in step 1 of build steps below) — new regression test.
  • No schema, API, or entity-contract changes. This is a pure internal promise-lifecycle bug fix; no data/contract changes.

The concrete change:

Build-step checklist:

  1. grep -rn "WorkerDbAbortError\|\.abandon(\|new WorkerDbTimeoutError" src/ repo-wide (the fix(db): an abandoned request must not kill the server (#2316) #2317 investigation only checked within worker_file_database.ts) — enumerate every construction and rejection site, not just the one in the reported stack.
  2. Fetch and read PR Rust panic in neon binding aborts the process when a client disconnects mid-statement (#2316 fix incomplete) #2324's diff in full (gh pr diff 2324 or equivalent) against worker_file_database.ts — confirm or rule out whether its retirement/drain rework introduced a second unguarded promise or bypassed the guardAbandonedRead boundary.
  3. Also fetch containment PR Contain abandoned-request aborts and capture diagnostics (#2483) #2484's diff — read what it changed and why Vanellus requested changes, so the root-cause fix here doesn't collide with or duplicate an in-flight patch on the same file.
  4. Confirm every caller of WorkerFileDatabase.prepare(...).get(/.all( across the repo goes through WorkerStatement (candidate Feature unit FU-113 execution #2 above) — grep for direct WorkerConnection/routeRead/dispatchRead usage outside this file.
  5. Write a regression test that: opens a read under simulated reader-pool saturation (reuse or extend existing saturation-simulation helpers used by Reader-pool exhaustion takes the instance down: auth consumes a pooled reader before it can reject an invalid token, and /ready failing under load kills a merely-slow machine #2322's tests if present), triggers a mid-query client disconnect (fire the same AbortController.abort() path db_abort_context.ts uses on res 'close'), and asserts the process does NOT crash / no unhandled rejection is emitted (e.g. via a process.on('unhandledRejection') listener in the test harness, or Node's --unhandled-rejections=throw semantics reproduced in-test). Confirm this test is RED on a80340c66 before writing the fix — a test that passes on the crashing build is not coverage (explicit PM/issue requirement).
  6. Apply the guard(s) at the construction site(s) identified in steps 1-4. Re-run the new test — confirm GREEN.
  7. Run the full existing worker_file_database test suite plus any Reader-pool exhaustion takes the instance down: auth consumes a pooled reader before it can reject an invalid token, and /ready failing under load kills a merely-slow machine #2322/Interrupt an abandoned libsql read instead of waiting it out (follow-up to #2324) #2339-related suites to confirm no regression in reader-pool or read-cancellation behavior (both explicitly out of scope for this fix — do not modify that logic, only confirm no interaction regression).
  8. Regenerate any generated/derived files whose source was touched (test catalog via npm run generate:test-catalog if a new test file was added or moved) as the last step before opening the PR.
  9. PR description states the design basis (no docs/foundation/ document governs internal worker promise lifecycle; state "Design basis: no design applies — internal bug fix, no interface/contract change") and explicitly notes the relationship to containment PR Contain abandoned-request aborts and capture diagnostics (#2483) #2484 (does this fix supersede it, or apply on top of it — resolve by reading Contain abandoned-request aborts and capture diagnostics (#2483) #2484's diff in step 3 before writing this line).

QA / Test Plan

Prior-art check (performed, not assumed): Searched gh issue/pr list for WorkerDbAbortError/abandon/saturation coverage before writing anything.

Regression test — the core deliverable, must be RED on a80340c66

New file: tests/integration/db_abort_reader_pool_saturation_survival.test.ts (new file, sibling to and explicitly modeled on tests/integration/db_abort_client_disconnect_survival.test.ts from #2317 — same child-process-liveness pattern, for the identical reason #2317's own PR description gives: vitest installs its own unhandledRejection handler, so no in-process assertion can distinguish "crashed" from "survived" — only a child-process exit-code check can. Do not attempt an in-process version of this test; it will pass on the crashing build and give false coverage.)

Saturation helper (new, since none exists — corrects Eng build-step 5's assumption): a small in-file helper that opens N concurrent slow reads (N ≥ configured reader-pool size) to hold every reader busy, confirmed via the DB reader pool recovered after Nms of saturation log line (or the underlying readerPoolStats()/saturation signal worker_file_database.ts already emits — reuse that signal rather than inventing a new one) before firing the disconnect under test. This is the one precondition #2317's own tests never construct.

Cases (fail-then-pass table, same convention #2317 used):

Case on a80340c66 (must be ✗) with fix (must be ✓)
Mid-query client disconnect while the reader pool is saturated (N concurrent slow reads holding every reader, then one caller's res emits 'close' mid-read)
Disconnect landing during a dispatchRead retry attempt (2nd–4th), i.e. abort races a WorkerDbRetiredError-triggered re-dispatch
Disconnect landing on the 4th (final, non-retried) dispatchRead attempt, where lastError is thrown directly instead of retried
Reconnect loop: 5× abandoned SSE streams under sustained saturation (mirrors #2317's own reconnect-loop case, which is what reproduced the loop in production, not just a single crash)
Control: mid-query disconnect without saturation (single reader, no contention) — must already pass on a80340c66 per #2317's existing suite ✓ (already covered)

Each case asserts, per #2317's established assertion shape: (1) child process is still alive after the disconnect, (2) child still serves a fresh DB read afterward, (3) no unhandled-rejection crash trace in child stderr. Do not assert on #2484's containment log line ("unhandled WorkerDbAbortError contained") as a substitute pass condition — if the root-cause fix lands, that log line should stop appearing for this scenario entirely (the rejection becomes handled at its construction site, never reaching the process-wide listener); a test that passes because #2484's global handler caught it is testing containment, not closure, and must not be conflated with this suite's purpose.

Edge cases for the fix's new branches (once the construction site is found and guarded)

  • Guard applied to the dispatchRead retry loop's per-attempt promise must not swallow a genuine WorkerDbRetiredError on attempts 1-3 (retry must still occur) — only the caller-abandonment case is guarded, not the retry's own control flow.
  • A write reaching a retired connection (if dispatchRead's pattern is ever mirrored for writes) must NOT be guarded — fix(db): an abandoned request must not kill the server (#2316) #2317 and this plan both keep the write path loud deliberately; add an explicit negative-path test asserting a write-side unhandled rejection still crashes the process, so a future change to dispatchRead-adjacent write logic can't silently widen the guard's scope.
  • If the fix touches retire()'s Promise.reject(new WorkerDbRetiredError(...)) (line ~327 of the fix(db): retire an abandoned reader instead of terminating it mid-native-call (#2324) #2338 diff) directly: confirm a caller still actively awaiting (not abandoned) receives the real WorkerDbRetiredError/WorkerDbAbortError and can still map it to a response — same "callers that await are unaffected" property fix(db): an abandoned request must not kill the server (#2316) #2317 verified explicitly.
  • Exhausting all 4 dispatchRead retry attempts under sustained saturation (pool never recovers within the retry budget) — assert the final thrown lastError is still guarded if the caller has abandoned, and still surfaces normally if the caller is still listening.

No new endpoints / no contract surface

This is a pure internal promise-lifecycle fix (per Eng scope) — no MCP tool, recipe, schema, or CLI surface changes. No agentic_eval fixture or contract test applies; the regression test above (Vitest integration, child-process harness) is the correct and sufficient artifact per this repo's own established convention for this exact bug class (#2317's precedent), not the Neotoma agentic-eval substrate, which covers agent-facing MCP/store-retrieve behavior rather than internal worker-process crash survival.

Definition-of-done checklist

QA gate status: Cannot sign off yet — no eval/test artifact is committed against this repo checkout in this session (spec-authoring turn only, no code changes made). This test plan is the committed deliverable for the QA lens at this stage of the ORDERED ADDITIVE SPEC pipeline; the actual fixture/spec file lands when Eng implements against this plan, and QA re-verifies red→green at that point before signing the qa gate.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingenhancementNew feature or requestlanius-triageApplied by Lanius triage workflow

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions