fix: retry a raw retriable asyncpg error, so connect-time failures retry on every sqlalchemy - #58
Merged
Merged
Conversation
…try on every sqlalchemy
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.
Replaces #57, which GitHub closed when its stacked base (#56) merged.
Problem
_is_retriable_linkonly recognised a retriable asyncpg error wrapped in aDBAPIError. Errors raised while SQLAlchemy is executing a statement always arrive wrapped. Errors raised by anasync_creatorat connect time arrive differently depending on the SQLAlchemy version:asyncpg.PostgresConnectionError(class 08)DBAPIError, retriedSo the same db-retry release retried a connect-time lost connection on 2.1 and gave up on 2.0. The 2.1 behaviour is the one
CONTEXT.mddescribes ("a lost connection (class08)" is retriable, andRETRIABLE_ASYNCPG_ERRORS"is the whole of it"). The 2.0 gap came from the wrapper check, not from a deliberate rule.Change
A chain link is now retriable when the asyncpg error it carries, either the raw exception or a
DBAPIError'sorig.__cause__, is inRETRIABLE_ASYNCPG_ERRORS. It is still oneisinstanceagainst the tuple, as ADR 0002 describes, and the40003invariant is unchanged; a rawStatementCompletionUnknownErrorcase pins it.Behaviour change for SQLAlchemy 2.0 users: under
postgres_retry, a connect-timePostgresConnectionError(and subclasses) is now retried. A rawSerializationErrororPostgresConnectionErrorraised anywhere in the chain, including from direct asyncpg use inside a decorated coroutine, is now retried too.Tests
test_is_retriable: raw40001and08000are retriable; raw40003and a raw basePostgresErrorare not.test_a_connect_time_connection_error_is_retried: anasync_creatorthat raisesPostgresConnectionErroris called once per attempt underpostgres_retry.Before the fix, the raw-error cases failed on both versions and the connect-time test failed on 2.0.54 only. After it, the full suite passes against Postgres at 100% coverage on sqlalchemy 2.0.54 and 2.1.0 (versions confirmed from inside the test interpreter);
just lint-cigreen.