Repository navigation
Conversation
Since brianc#3574 the bind catch block sends Close + Sync and then reports the error right away. The query is still the active one, so the ReadyForQuery that answers the Sync runs the success path too: the callback fires a second time with no rows, and a Query object emits 'end' after 'error'. Hand the error to the ReadyForQuery handler through _canceledDueToError instead, the way a failing type parser already does, so the query settles exactly once. Fixes brianc#3820
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
1 participant
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 #3820.
When a parameter fails to serialize (a
toPostgres()that throws, a BigInt in a JSON value), the catch block inQuery.preparesends Close + Sync (added in #3574 so the connection does not wedge) and then callshandleErrorimmediately. The query is still the client's active query, so the ReadyForQuery that answers the Sync also runshandleReadyForQuery: the callback fires a second time with(null, result)and no rows, and aQueryobject emitsendaftererror.This hands the error to the ReadyForQuery handler through
_canceledDueToError, the same path a throwing type parser uses, so the query settles once.Tests:
throw-in-bind-tests.js: the stubbedsyncnow answers withreadyForQuerylike the server does, plus three new cases (callback runs once, noendaftererror, the next query runs). The "runs once" cases fail on master.bind-error-settles-once-tests.jsintegration test against a real server, pipeline on and off.Ran
make test-unit(290 pass) and the integration suite locally on PostgreSQL 17.5; onlyssl-tests.jsfails here because the local server has no SSL.No behaviour change for the promise API. For a
Queryobject with noerrorlistener the throw now surfaces from the ReadyForQuery handler instead of synchronously out ofclient.query(), which is where a type-parser failure already surfaces it.