Repository navigation
Close the connection when fetching array types fails - fixes #1192 - #1265
Open
asharrahman02 wants to merge 2 commits into
Open
asharrahman02 wants to merge 2 commits into
asharrahman02 wants to merge 2 commits into
Conversation
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 #1192, fixes #1205 (also the original #279)
ReadyForQuerydrops the promise returned byfetchArrayTypes(), so when the internalpg_typequery fails (statement timeout, cancellation, a pooler killing a fresh backend,sql.end({ timeout })during startup) the rejection is unhandled and Node exits — even though the user's query is rejected and caught normally. Catching it alone isn't enough: the connection would then be handed to the pool without array parsers, and arrays come back as strings like'{1,2}'.This change:
.catch(), so its rejection is always handled.reject(the same wayfetchState()setsresolve) to setneedsTypesagain synchronously.ReadyForQuerythen closes the connection instead of opening it, beforeonopen()can give it to queued work. The next connection fetches types again.sql.reserve()with the server's error.ReadyForQueryremoves the reserve frominitialbefore the fetch, soerrored()never reaches it, and before this change the reserve was resolved with a connection that had no array types.The user's query still gets the original server error, and the success path is unchanged.
selectonpg_catalog.pg_typefor the test database (restored in each test): a first query rejects with42501and a later array query on the same client parses{1,2}as an array; two queued queries onmax: 1both reject; a waitingreserve()rejects. Without the fix, the suite crashes on the unhandled42501. With--unhandled-rejections=warn, the first test fails with'42501,true' != '42501,false'(the array comes back as a string).src/only;cjs/,cf/anddeno/are left for the build.This is the same approach as #1231, which was closed by its author, but it doesn't depend on #1230. We run this change as a patch on 3.4.9 in production.