Conversation
The local Hyperdrive proxy pipes the client socket and the database socket together. Only the database side had an `error` listener, so an error on the client socket had no listener and was thrown as an uncaught exception, taking down the whole Node process. Both sockets now go through a single helper that pipes them together and tears down each side when either one errors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: e398757 The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
workers-devprod
requested review from
a team and
NuroDev
and removed request for
a team
September 22, 2026 10:05
Contributor
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
@cloudflare/autoconfig
@cloudflare/build-output-utils
@cloudflare/codemods
@cloudflare/config
@cloudflare/containers-shared
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-functions
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-plugin
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
On the TLS paths, #handleConnection writes to the database, reads the reply and runs a TLS handshake before it reaches a pipe site, and the client socket has no error listener for that whole window. A client that resets mid-negotiation still took the Node process down. The listener is now attached as soon as the connection is accepted, alongside the per-pipe teardown that knows which peer to destroy. setupTLSConnection had become a pass-through to pipeSockets, and createPlainTCPConnection no longer needs a handler of its own now that its callers pipe the socket it returns on the next line; both are gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
On the fallback paths the database socket is opened after the negotiation, so it can be created when the client has already errored. Its `error` and `close` have fired by then, a listener attached at the pipe would never run, and `pipe()` only ends the destination for a source that ended cleanly — leaving the connection to the database open with nothing to serve. No test: the window is a client error landing inside the TCP connect of the replacement socket, which I could not make deterministic. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pipeSockets had the longest doc comment in the file on its smallest function, most of it restating the two pipes and the two handlers below it. Only the pipe() re-emit is worth writing down. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The entry described the internals on both sides of the fix, so it never said what a user would have seen or whether they were affected. It now names the symptom, the surfaces it killed, and the mode that skips the proxy. It also covers the fallback connection left open, which the entry did not mention. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
2 participants
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 #15768.
The Hyperdrive proxy pipes the client socket and the database socket together. The database side has an
errorlistener, the client side has none, so a client socket error has nothing listening and takes the Node process down. There are two windows where that happens.After the pipes are set up.
pipe()does not cover this. It attaches anonerrorhandler to the destination, but that handler removes itself and re-emits once no other listener is left. The four places that pipe the two sockets now go through one helper,pipeSockets, which attaches a teardown handler to both ends first.Before they exist. On the TLS paths,
#handleConnectionwrites to the database, reads the reply and runs a TLS handshake before it reaches a pipe site, and the client socket has no listener for that whole window — the case raised in review on the first revision. A listener is now attached as soon as the connection is accepted. Reaching this one takes a client that resets mid-negotiation: a Worker that half-closes, aborts its writer or throws leaves the proxy intact, and so does disposing of Miniflare mid-negotiation.The two are complementary, not alternatives. The entry listener only has the
dbSocketcreated at the top of#handleConnectionin scope, and on the TLS paths the socket that ends up piped is a laternewDbSocketortlsSocket, so the pipe sites are where the right peer can be torn down.One more teardown gap, also raised in review: on the fallback paths the database socket is opened after the negotiation, so it can be created once the client has already errored. Its
errorandclosehave fired by then, a listener attached at the pipe would never run, andpipe()only ends the destination for a source that ended cleanly — so the connection to the database stayed open with nothing to serve.pipeSocketsnow destroys the peer straight away when the client is already gone. There is no test for this one: the window is a client error landing inside the TCP connect of the replacement socket, and I could not make that deterministic.Two things fell out of the helper.
setupTLSConnectionhad become a pass-through topipeSockets, so it is gone.createPlainTCPConnectionno longer installs a handler of its own — its callers pipe the socket it returns on the next line — and its now-unusedclientSocketparameter went with it.packages/miniflare/test/plugins/hyperdrive/proxy.spec.tscovers both windows: a mock Postgres that upgrades to TLS and then writes continuously while the client is killed mid-stream, and a mock Postgres that sits on the SSL negotiation while the client resets. Both fail onmainand pass here.Note
This is a contribution from an AI agent: Claude Code, Claude Opus 5. A human reviewed the diff and ran the reproduction, but wrote neither the patch nor this description.