Skip to content

fix(server): abort the request when the client disconnects on the Node adapter - #355

Open
Vito-168 wants to merge 1 commit into
Devin-AXIS:mainfrom
Vito-168:fix/serve-node-disconnect-signal
Open

fix(server): abort the request when the client disconnects on the Node adapter#355
Vito-168 wants to merge 1 commit into
Devin-AXIS:mainfrom
Vito-168:fix/serve-node-disconnect-signal

Conversation

@Vito-168

Copy link
Copy Markdown

Fixes #354.

The problem

toWebRequest builds its Request without a signal, so request.signal is an
AbortSignal that can never fire — however long ago the client hung up. And
writeWebResponse ends with reader.releaseLock(), which detaches the reader but leaves the
underlying stream unclosed, so the producer is never told to stop.

Together: a streaming response keeps its producer running after the client goes away, for the
life of the process.

This is live on a shipped path. GET /workspace/:id/engine/deepseek-harness/events/:stream
already forwards ctx.request.signal into runtime.events(...)
(routes/deepseek-harness.ts:121), which hands it to fetch (deepseek-harness-runtime.ts:289).
Both ends were wired — only the signal was missing. Every closed Harness event stream leaves
its upstream subscription open, and they accumulate.

The fix

Derive an AbortSignal from the Node socket and pass it into the Request; cancel the body
reader instead of releasing it so cancellation propagates upstream.

The writableFinished guard is the subtle part: close fires on every normal response too,
so without it every completed request would abort its own signal. The test suite pins that
behaviour rather than leaving it to a comment.

Tests

Three added to the existing serve-node.test.ts:

  • a normally completed request does not abort its signal (the writableFinished guard)
  • a client disconnecting mid-response aborts the handler's signal
  • a streaming body is cancelled, not just released, when the client goes away

bun test src/serve-node.test.ts → 7 pass / 0 fail. tsc --noEmit clean.

Scope

Node path only — Bun's serve() already provides a working request.signal. So this affects
IPOLLOWORK_RUNTIME=node and the packaged desktop server, not Bun deployments.

No API change, no new dependencies (node:http and node:stream, both already imported).

…e adapter

`toWebRequest` built its `Request` without a `signal`, so `request.signal`
was an AbortSignal that could never fire, and `writeWebResponse` released
the body reader instead of cancelling it. Together that meant a streaming
response kept its producer alive after the client went away, for the life
of the process.

Both ends of this were already wired: the Harness events route forwards
`ctx.request.signal` into `runtime.events()`, which hands it to `fetch`.
Only the signal itself was missing, so every closed
`/workspace/:id/engine/deepseek-harness/events/:stream` stream left its
upstream subscription open.

Derive the signal from the Node socket, guarding on `writableFinished` so
a normally completed response is not mistaken for a disconnect, and cancel
the reader so cancellation propagates upstream.

Bun's `serve()` already provides a working `request.signal`; this affects
only the Node path.

Refs Devin-AXIS#354
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.

serve-node: client disconnect never aborts the request, leaking streaming producers

1 participant