sockets: close() with a pipe close in flight logged an uncaught exception - #7313
Merged
Conversation
…tion Socket::close() aborted its streams with no reason, which abort() turns into undefined, so anything rejected by that abort rejected with a bare undefined. When a pipe into the socket had just completed, its close of the writable was still queued; the forced abort drained it, and the write loop's pipe-completion path returned that close's promise, so the rejection failed the loop's task and surfaced as "Uncaught (in promise) undefined" plus an internal-error log, though nothing was actually unhandled. Under a peer disconnect the same path reported "Network connection lost". close() now aborts with a real TypeError, and the pipe-completion path no longer couples the write-loop task to the close's outcome, which belongs to the close request alone. A genuine handler throw is still reported. The test spawns workerd and asserts on its log output, since the test framework does not observe these.
jasnell
reviewed
Sep 11, 2026
jasnell
reviewed
Sep 11, 2026
jasnell
reviewed
Sep 11, 2026
jasnell
reviewed
Sep 11, 2026
Contributor
|
LGTM |
jasnell
reviewed
Sep 11, 2026
jasnell
reviewed
Sep 11, 2026
jasnell
reviewed
Sep 11, 2026
The pipe-completion paths no longer return the close's promise at all: the close is its own request with its own write loop, so nothing waits on it. Drop the redundant JsValue wrap. The scenario also runs in the streams sockets suite under both implementations, asserting the abort reason a later writer observes is a real error and never undefined; the spawn-based test remains the only observer of the uncaught-exception log.
The closed promises are awaited rather than swallowed, pipe and close results are asserted, and the echo reads with for await. The client half-closes and drains without calling close(): at the oldest compat date a client close() after EOF leaves the server side's close() pending, which is unrelated to this change.
jasnell
approved these changes
Sep 11, 2026
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.
A
connect()handler that pipes an inbound socket to another and then closes both concurrently loggedUncaught (in promise): undefinedand an internal-error line (jsg.Error: undefined) on every connection, orNetwork connection lostunder a peer disconnect, with nothing actually unhandled and the handler completing normally.Two causes:
Socket::close()aborted its streams with no reason, whichWritableStreamInternalController::abort()turns intoundefined(as the spec requires for the user-facing call), so everything that abort rejected rejected with a bareundefined. It now aborts with a realTypeError.awaitJstask and was reported as uncaught. The close's outcome belongs to the close request alone, so the loop no longer propagates it.A genuine throw from a
connect()handler is still reported as uncaught.Tests:
closeWithPipeCloseInFlightin the streams sockets suite runs the scenario under both stream implementations and asserts the observable half, that the abort reason a later writer sees is a realTypeErrorand neverundefined; it fails on main under C++ at the current compat date, and passes where the race cannot occur (TypeScript resolvespipeToonly after the destination close completes; beforeinternal_writable_stream_abort_clears_queuethe abort defers until the queued close finishes). The logged line itself isawaitJs'sINTERNAL_ASYNCreport of a rejected promise handed to C++, not a V8 unhandled rejection (the promise did have a handler), sounhandledrejectionnever fires and nothing else is JS-observable;server/tests/socket-closespawnsworkerd testand asserts the log is clean, and fails on main with the exact reported lines. Streams, sockets, connect and net suites pass.