Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Messenger] Swallow Exception on Rollback#54355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:7.4
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
if there is an exception rolling back the transaction then the originalexception will be hidden.
d468c1a tod0619baCompare| $entityManager->getConnection()->rollBack(); | ||
| try { | ||
| $entityManager->getConnection()->rollBack(); | ||
| } catch (\Throwable $e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| }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 |
Uh oh!
There was an error while loading.Please reload this page.
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.