Skip to content

fix: use ConnectionResetError in TLSUpgradeProto.connection_lost#1312

Open
yemreck wants to merge 1 commit intoMagicStack:masterfrom
yemreck:fix/tls-upgrade-connection-lost
Open

fix: use ConnectionResetError in TLSUpgradeProto.connection_lost#1312
yemreck wants to merge 1 commit intoMagicStack:masterfrom
yemreck:fix/tls-upgrade-connection-lost

Conversation

@yemreck
Copy link

@yemreck yemreck commented Mar 12, 2026

Summary

Fixes #1310.

TLSUpgradeProto.connection_lost() synthesizes a generic ConnectionError("unexpected connection_lost() call") when the server closes the connection during SSL negotiation (exc=None). This error type is not handled by asyncpg's own Connection._cancel(), which only catches ConnectionResetError:

# asyncpg/connection.py - Connection._cancel()
except ConnectionResetError as ex:
    # On some systems Postgres will reset the connection
    # after processing the cancellation command.

The generic ConnectionError falls through to the except (Exception, ...) handler and leaks into application code, causing confusing unhandled errors.

The problem in practice

When pool_pre_ping sends a trivial query (fetchrow(";")) that gets cancelled (e.g. due to asyncio task cancellation under load), the following cascade occurs:

  1. fetchrow(";") gets CancelledError
  2. The finally block tries await tr.rollback()
  3. The rollback triggers Connection._cancel() via the cancel side-channel
  4. _cancel() opens an SSL connection → TLSUpgradeProto → server closes during negotiation → connection_lost(exc=None)
  5. ConnectionError("unexpected connection_lost() call") propagates unhandled to application code

The fix

Use ConnectionResetError instead of ConnectionError. This is:

  • Semantically correct — the server reset/closed the connection during SSL negotiation
  • Properly handledConnection._cancel() already catches ConnectionResetError explicitly
  • Consistent — error message follows the same format as the sibling error in data_received ('PostgreSQL server at "{host}:{port}" rejected SSL upgrade')
  • Backward compatibleConnectionResetError is a subclass of ConnectionError, so any existing except ConnectionError catches still work

Test plan

  • Existing test suite passes (no tests reference "unexpected connection_lost")
  • Verify ConnectionResetError is caught by Connection._cancel()'s existing handler

@yemreck yemreck marked this pull request as ready for review March 12, 2026 13:25
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.

ConnectionError from _cancel() during CancelledError not caught, crashes callers

1 participant