fix(server): abort the request when the client disconnects on the Node adapter - #355
Open
Vito-168 wants to merge 1 commit into
Open
fix(server): abort the request when the client disconnects on the Node adapter#355Vito-168 wants to merge 1 commit into
Vito-168 wants to merge 1 commit into
Conversation
…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
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
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.
Fixes #354.
The problem
toWebRequestbuilds itsRequestwithout asignal, sorequest.signalis anAbortSignalthat can never fire — however long ago the client hung up. AndwriteWebResponseends withreader.releaseLock(), which detaches the reader but leaves theunderlying 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/:streamalready forwards
ctx.request.signalintoruntime.events(...)(
routes/deepseek-harness.ts:121), which hands it tofetch(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
AbortSignalfrom the Node socket and pass it into theRequest; cancel the bodyreader instead of releasing it so cancellation propagates upstream.
The
writableFinishedguard is the subtle part:closefires 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:writableFinishedguard)bun test src/serve-node.test.ts→ 7 pass / 0 fail.tsc --noEmitclean.Scope
Node path only — Bun's
serve()already provides a workingrequest.signal. So this affectsIPOLLOWORK_RUNTIME=nodeand the packaged desktop server, not Bun deployments.No API change, no new dependencies (
node:httpandnode:stream, both already imported).