Repository navigation
Conversation
An ErrorResponse can arrive after CommandComplete, for example when a deferred constraint fails as the implicit transaction commits on Sync. The cursor was already done, so the error was dropped: a pending read never settled and later reads resolved with an empty result. Settle the pending read and any queued reads with the error, and make later reads return it.
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.
Related to #3768
What happens
If the server sends an
ErrorResponseafterCommandComplete, the cursor is already in thedonestate, sohandleErrorreturns early and the error is lost:ReadyForQuery, and the client does not forwardReadyForQueryto the cursor after an error. So that read never settles. Reads waiting in the cursor's queue never settle either.read()sees thedonestate and resolves with[], as if there were simply no more rows.One way to get this: a deferred constraint that fails when the implicit transaction commits on
Sync. The new tests use aDEFERRABLE INITIALLY DEFERREDforeign key and a function that inserts a bad child row for each selected row.The change
handleError, when the state isdone, the cursor stores the error, calls the pending read (if any) and every queued read with it, and emitserrorif there is a listener, as the normal error path does.read(), when the state isdoneand an error was stored, the read gets that error instead of[].Rows that were already received in that batch are dropped and the read gets the error, since the transaction was rolled back. Nothing changes when no error arrives.
Tests
Two new tests in
packages/pg-cursor/test/error-handling.js:close().Without the fix both fail ("read never settled" / "queued reads never settled"): 43 passing, 2 failing. With the fix all 45 pg-cursor tests pass (3 runs,
postgres:16andnode:20Docker images). Prettier and eslint are clean on the changed files.What I am not sure about
The report in #3768 describes the read resolving with
[]. With this change a read after the error no longer resolves with[], but I reproduced it only with a deferred constraint. I did not reproduce the connection-closed or deadlock cases from the issue, so this change covers only the error-after-CommandComplete case. That is why I wrote "Related to" and not "Fixes".