Repository navigation
Conversation
This was referenced Oct 25, 2026
When the server reported an error after CopyDone instead of CommandComplete (from an AFTER trigger such as a foreign key check, from a deferred constraint at commit, or from a cancellation while these run), the raw COPY stream threw it, but it read CommandComplete and ReadyForQuery outside the cleanup that a failed read of a CopyData message gets. The stream stayed ready, although the connector had already read up to the ReadyForQuery, so its dispose, which closing the connection runs too, waited for another ReadyForQuery, and a later read for another message: until the command timeout broke the connection (which also cleared the pool, unless Cancellation Timeout is -1), or forever without one. An error from a second statement in the command, instead of ReadyForQuery, did the same. BeginTextExport reads through the raw stream, so its reader had the same problem. These reads now end the export as a failed read of a CopyData message does: the stream releases the connection at once, so its dispose neither skips nor waits, and the COPY activity records the error instead of the timeout. After an error ended an export at a read (Cancel makes the server send one too), later calls until the dispose throw InvalidOperationException with the error as the inner exception instead of ObjectDisposedException, and the stream no longer logs that the COPY operation completed. Nor does it when BeginRawBinaryCopy, BeginTextExport or BeginTextImport fails before the COPY starts, e.g. when the server answers the command with an error for a missing table. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brar
force-pushed
the
core/raw-copy-error-dispose
branch
from
November 1, 2026 13:38
9d0d6cc to
3c65a6c
Compare
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the COPY and text-export error-handling changes.
No open issues 🎉
What changed in this PR
Fixes raw COPY and text export error handling so failures after CopyDone release the connection promptly instead of causing later reads or disposal to wait and potentially break it.
Changes:
- Handles completion-message errors through the existing cleanup path and preserves the original error for subsequent calls until disposal.
- Suppresses completion logs when COPY fails during initialization or an export read.
- Adds regression tests for error timing, subsequent reads, disposal, connection reuse, logging, and tracing.
| File | Description |
|---|---|
test/Npgsql.Tests/TracingTests.cs |
Verifies tracing records the original export error after CopyDone. |
test/Npgsql.Tests/CopyTests.cs |
Adds raw and text COPY error cases and successful controls. |
src/Npgsql/NpgsqlRawCopyStream.cs |
Extends error cleanup and preserves failure details until disposal. |
Review effort: Balanced
This branch has not been deployed
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.
When the server reported an error after CopyDone instead of CommandComplete (from an AFTER trigger such as a foreign key check, from a deferred constraint at commit, or from a cancellation while these run), the raw COPY stream threw it, but it read CommandComplete and ReadyForQuery outside the cleanup that a failed read of a CopyData message gets. The stream stayed ready, although the connector had already read up to the ReadyForQuery, so its dispose, which closing the connection runs too, waited for another ReadyForQuery, and so did a later read: until the command timeout broke the connection (which also cleared the data source's pool, unless Cancellation Timeout is -1), or forever without one. An error from a second statement in the command, instead of ReadyForQuery, did the same.
BeginTextExportreads through the raw stream, so its reader had the same problem.These reads now end the export as a failed read of a CopyData message does: the stream releases the connection at once, so its dispose neither skips nor waits, and the COPY activity records the error instead of the timeout. After an error ended an export at a read (
Cancelmakes the server send one too), later calls until the dispose throw InvalidOperationException "The COPY operation ended with an error." with the error as the inner exception: on the raw streamRead,ReadAsync,ReadByte,Write,FlushandCancel, on the text reader its reads andCancel. Before, they threw ObjectDisposedException without the error, or waited as above. After the dispose they throw ObjectDisposedException, as before. The stream no longer logs that the COPY operation completed after such an error, nor whenBeginRawBinaryCopy,BeginTextExportorBeginTextImportfails before the COPY starts, e.g. when the server answers the command with an error for a missing table.How a real server gets there
PostgreSQL sends CopyDone before
EndCopyTorunsExecutorFinish, which fires the queued AFTER triggers (copyto.c:1118and:1015,copy.c:358-359,execMain.c:443), and it commits before it reports command completion (postgres.c:1284-1298and:1337; all REL_18_STABLE).Live on PostgreSQL 18.6, Debug and Release, sync and async, Command Timeout 5 s:
statement_timeoutduring a statement-level AFTER trigger and a failing second statement, forBeginRawBinaryCopyandBeginTextExportalike.Cancelduring a long export. The COPY activity records the 23503 instead of the dispose's timeout.pg_terminate_backendduring a lock wait no longer log "completed". The exception, the activity and the connection are unchanged.Tests
CopyTests.Raw_export_error, 40 mock cases: raw binary and text export; an error before the first row, before the second, after CopyDone, after CommandComplete, or none; with and without a later read; sync and async.TracingTests.RawBinaryExport_error_after_CopyDone(2 cases), the only test of the catch'sTraceSetException.CopyTests.Raw_copy_error_at_start, 16 cases: all four APIs, sync and async, with an ErrorResponse 42P01 instead of the COPY response, or as a control that disposes right after a successful Begin.Raw_copy_error_at_start, exactly the 8 error cases fail without the new log condition. Seven mutations of the change were each caught by exactly the expected cases.For the reviewer
NpgsqlRawCopyStream, with no API change. The message text and the field_errormatch Fix NpgsqlBinaryExporter's dispose after an error or a connection break #6713.catch (InvalidOperationException)still catches both, whilecatch (ObjectDisposedException)no longer catches the new exception before the dispose.Cancel, the server answers with 57014, and the read turns it into NpgsqlException with an inner TimeoutException, as before; later calls now carry that exception as the inner exception, and "completed" is no longer logged.Dependencies and merge order
Fixes #6704