[Messenger] Swallow Exception on Rollback #54355
Open
+20
−1
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
if there is an exception rolling back the transaction then the original exception will be hidden.
d468c1a to
d0619ba
Compare
rodnaph
commented
Mar 20, 2024
| $entityManager->getConnection()->rollBack(); | ||
| try { | ||
| $entityManager->getConnection()->rollBack(); | ||
| } catch (\Throwable $e) { |
There was a problem hiding this comment.
Suggested change
| } catch (\Throwable $e) { | |
| } catch (\Throwable $e) { | |
| $exception = new \Exception($e->getMessage(), $e->getCode(), $exception); |
Is it better to pass along some notification of rollback failure? This will interfere with the check a few lines below though, so would need to be structured differently.
|
Similar issue: doctrine/orm#7545 |
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 a database level exception occurs, eg. constraint violations, it may not be possible to rollback the active transaction when handing the exception in the Messenger middleware. In this case an exception is thrown from rollback, and the original exception is lost.
I've added a try/catch to swallow any exception rolling back, so the original exception will be thrown as expected. I don't know if there is better behaviour here, just swallowing the exception seems quite poor. We could create a new exception with the rollback error, adding the original exception to it?
I also tried checking if there was an active transaction before rolling back, but that does not seem to be a reliable method for catching this condition.
I've marked this as neither a bugfix or a feature... it's a minor improvement, but let me know if I should update the description.