Convert driver exceptions when starting transactions #6812
Merged
Conversation
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
|
Let's get it integration-tested. Even though the unit test passes, and the code works as expected on MySQL, we want to be sure that it works the same on other platforms as well or at least know that it doesn't. |
lcobucci
force-pushed
the
convert-exceptions-on-begin-transaction
branch
from
5fa77a9 to
5c1255f
Compare
lcobucci
commented
Feb 25, 2025
lcobucci
commented
Feb 25, 2025
lcobucci
commented
Feb 25, 2025
lcobucci
commented
Feb 25, 2025
lcobucci
force-pushed
the
convert-exceptions-on-begin-transaction
branch
3 times, most recently
from
6e47746 to
432b89d
Compare
morozov
reviewed
Mar 1, 2025
This optimises how this test is executed by leveraging two connections and killing the one executing the tests. It also aims at making the whole conversion process available to other platforms. Signed-off-by: Luís Cobucci <[email protected]>
For long-running processes, it's possible that the wrapped connection is no longer valid and starting a transaction will lead to driver exceptions. DBAL Connection wasn't converting these, leaking internal details of the platform and not providing a consistent API (all other methods would throw a `ConnectionLost` exception for the scenario above). This ensures we do have the expected behaviour for this edge-case. Signed-off-by: Luís Cobucci <[email protected]>
This extends the functional test for connection loss detection to verify the behaviour against Postgres and ensures the ExceptionConverter for that API behaves as expected. Signed-off-by: Luís Cobucci <[email protected]>
This is required because the MySQLi API only returns a boolean result that we were simply not using. Signed-off-by: Luís Cobucci <[email protected]>
lcobucci
force-pushed
the
convert-exceptions-on-begin-transaction
branch
from
432b89d to
b6eb520
Compare
morozov
approved these changes
Mar 1, 2025
|
Thanks, @lcobucci! |
|
I'm fine with releasing it any time. |
It would be great to have it released, then |
Jean85
added a commit
to facile-it/doctrine-mysql-come-back
that referenced
this pull request
Mar 13, 2025
doctrine/dbal#6812 introduced a behavioral change on lost connection. Now all drivers behave the same, and close the connection explicitly before throwing.
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.
Summary
The wrapped connection may no longer be valid for long-running processes, and starting a transaction will cause driver exceptions.
DBAL Connection wasn't converting these, leaking internal details of the platform and not providing a consistent API (all other methods would throw a
ConnectionLostexception for the scenario above).This ensures we do have the expected behaviour for this edge case.