Repository navigation
fix(cloudflare): emit close when the socket closed promise rejects - #3834
Open
princejain756 wants to merge 1 commit into
Open
princejain756 wants to merge 1 commit into
princejain756 wants to merge 1 commit into
Conversation
workerd rejects the socket closed promise with undefined when close() runs in the same turn as the read that reaches a server-closed stream end (cloudflare/workerd#6479). The handler forwarded that as error and never emitted close, so Connection's end handler never fired and client.end() hung; pg also crashed reading error.code off the undefined rejection. Match net.Socket instead: a rejected closed promise still emits close, and the error is skipped when the rejection follows end() since the socket was being closed anyway. A failed close() call now surfaces close the same way. Fixes brianc#3822
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.
workerd rejects a socket's
closedpromise withundefinedwhensocket.close()runs in the same turn as the read that reaches the end of a stream the server closed (cloudflare/workerd#6479).pghits this afterpg_terminate_backend()during a query: the query rejects,client.end()runsclose()right after_listen()reaches the stream end,closedrejects, and the handler emitserrorbut neverclose.Connectionwaits on the stream'scloseto emitend, soclient.end()never resolves. On top of that, theerrorcarriesundefined, which crashesreportStreamErrorreadingerror.code.Two changes to match
net.Socket, which always emitscloseaftererror:closedpromise now emitsclose(aftererror, same order as net.Socket)erroris skipped when the rejection followsend(), since the socket was being closed anyway; a rejectedclose()emitsclosethe same wayReproduced with stubbed
closedpromises inpg-esm-test/pg-cloudflare.test.js: a mid-session rejection still emitserrorthenclosein that order, andend()followed by a rejection emitsclosewith no spuriouserror. 6/6 tests pass and both new cases fail on the old build.Fixes #3822