fix: incorrect failover error rewrapping - #1274
Merged
Merged
Conversation
karenc-bq
force-pushed
the
fix/incorrect-failover-error-rewrapping
branch
from
August 24, 2026 23:35
2372028 to
bedffbf
Compare
sophia-bq
approved these changes
Aug 25, 2026
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.
Description
3 fixes: one to how failover errors surface to SQLAlchemy consumers, and two that block upgrading from 3.0.0.
1. Failover errors reach SQLAlchemy unchanged
The dialect mixin caught
FailoverSuccessErrorindo_execute/do_executemanyand re-raisedpep249.OperationalError. The substitution was unnecessary, and it broke two things.It was unnecessary because SQLAlchemy classifies by walking
orig.__class__.__mro__and matching bases by class name againstsqlalchemy.exc.FailoverErroralready derives from the wrapper'spep249.OperationalError, so the family already mapped tosqlalchemy.exc.OperationalErrorwithout any rewrap. Measured both ways: samesqlalchemy.excclass, differing only in.orig.What it broke:
DBAPIError.origbecame the substitute, soisinstance(err.orig, FailoverSuccessError)stopped matching. 3.0.0's shipped example used that check.is_disconnectno longer received aFailoverError, so it delegated toMySQLDialect_mysqlconnector.is_disconnect, which readse.errno. The wrapper's PEP-249 errors have noerrno, and SQLAlchemy callsis_disconnectat the top ofConnection._handle_dbapi_exceptionoutside anytry, so the resultingAttributeErrorescaped and the application'sexcept DBAPIErrornever ran.Reproduced on a real Aurora MySQL failover: a successful failover surfaced to the consumer as
AttributeError: 'OperationalError' object has no attribute 'errno'. The connection was usable; only the signal telling the application to retry was lost.docs/examples/MySQLSQLAlchemyFailover.pycatchesOperationalError, which that bypasses.The rewrap also made
is_disconnectunreachable on thedo_executepath, where it is needed. Removing it restores the override that keeps a recoverable failover from invalidating the pooled connection.This change also defaults
errnoandsqlstatetoNoneonpep249.Error. Becausedialect.loaded_dbapiis this package, SQLAlchemy's MySQL dialect probese.errnoon anything that is anOperationalErrororInterfaceError. Nine wrapper errors qualify: theFailoverErrorfamily plusQueryTimeoutError,AwsConnectErrorandReadWriteSplittingError. Each was one unguarded probe away from the sameAttributeError.2. Type stubs are no longer runtime dependencies
boto3-stubsandtypes_aws_xray_sdkwere declared in[tool.poetry.dependencies], which made upgrading impossible for some consumers.3. The pre-3.1.0 dialect import path resolves again
SqlAlchemyOrmMysqlDialectmoved tosqlalchemy_dialects/mysql.py::AwsWrapperMySQLConnectorDialectwithout an alias, so code that imported the class directly broke withModuleNotFoundError. URL-based configuration was unaffected: the driver nameaws_wrapper_mysqlconnectordid not change. This adds a shim for the direct-import case, with aDeprecationWarning.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.