Skip to content

pg-cursor: report an error that arrives after CommandComplete - #3860

Open
ritojoao wants to merge 1 commit into
brianc:masterfrom
ritojoao:fix-cursor-error-after-complete
Open

ritojoao wants to merge 1 commit into
brianc:masterfrom
ritojoao:fix-cursor-error-after-complete

Conversation

@ritojoao

Copy link
Copy Markdown

Related to #3768

What happens

If the server sends an ErrorResponse after CommandComplete, the cursor is already in the done state, so handleError returns early and the error is lost:

  • A read that is still waiting is answered only on ReadyForQuery, and the client does not forward ReadyForQuery to the cursor after an error. So that read never settles. Reads waiting in the cursor's queue never settle either.
  • Any later read() sees the done state 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 a DEFERRABLE INITIALLY DEFERRED foreign key and a function that inserts a bad child row for each selected row.

The change

  • In handleError, when the state is done, the cursor stores the error, calls the pending read (if any) and every queued read with it, and emits error if there is a listener, as the normal error path does.
  • In read(), when the state is done and 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:

  • "rejects a pending read when the error arrives after CommandComplete": the pending read rejects, a second read rejects with the same error, and the client can still run a query after close().
  • "rejects queued reads when the error arrives after CommandComplete": two reads queued while another query is running both reject.

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:16 and node:20 Docker 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".

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant