Transaction context fails to relay the original exception if rollback raises. #302
Comments
|
I suspect it's something else causing your situation. Because:
An easy fix would be to check transaction status before calling |
|
I didn't capture the exception message at the time, but the reason I came to file this bug was that I read the message like "can't rollback an already-aborted transaction". All transaction management in my code is use the engine's async with mode, and all SQL to the database goes through GINO (albeit with explicit specification of bind all the time, rather than using the shorthand mechanism). At some point calls of several functions decorated like the above might have been nested, but that (due to the support of nested transactions) shouldn't be an issue. I'll undo my local patch and see if I can reproduce the situation, then report back. |
|
I found the traces in syslog. 😄 This is (excluding other server logging) what I get with just embedding that rollback call in a try: except: pass exception-ignoring block: And this was before the exception-ignoring "fix": So this might be an asyncpg issue? |
|
Aaand... apparently asyncpg and postgresql diverge about whether a rollback on a failed transaction should report failure or not: |
running rollback on failed or already rolled back transactions has no effect and asyncpg would throw exceptions
|
After I reproduced your error, I found I overlooked the error happens when committing the transaction, not during I've submitted #304, which should be able to fix your issue. |
|
Thanks! It looks a little bit rough to inspect an internal _state attribute of asyncpg.Transaction, but then asyncpg isn't really offering any alternative other than trying to commit/rollback and checking inside a generic exception (or ignoring it). |
|
Right. I want to avoid to commit/rollback and just catch any exception, which may bury undesired bugs. |
|
Thanks for the issue report and Tony's PR!
This is not true. A failed transaction in PostgreSQL does not usually have Actually, Therefore the root issue is that, GINO shouldn't call |
|
My intention to check the state is also to prevent exceptions when users call rollback multiple times on the same transaction. It might not be our job though, I'll fix |
|
Yeah I got that. Thanks for the fix! |
|
I'd like to reopen this. If you look at the first backtrace I pasted, the SerializationError was actually raised in a delete operation, and not the final commit call. In this case... I'd expect the transaction to be in the failed state already (at least in postgres; not sure about asyncpg's idea of it). Would the unconditional rollback-on-exception fail then with the same error again? |
|
@skandalfo In my first reply #302 (comment), I was using a demo where the exception happens in an update statement. This should be the same case as you mentioned. When a normal (other than |
|
Yeah Tony's right, the unconditional rollback-on-exception is supposed to succeed. Please try with latest master in your code if possible, and feel free to reopen if bug persists. |
|
I'll make sure I upgrade to master ASAP, and will keep an eye on the logs, but I don't expect to see anything weird now. Thanks for clarifying! |
Description
I'm using PostgreSQL with serializable isolation level transactions. I've tried to write a decorator to make Python object functions retry the whole transaction whenever a transaction is cancelled by a serialization conflict (or a transient database disconnect).
What I Did
I wrote this code for a decorator(that depends on a specific hierarchies on for the class of the object decorated, but that's a detail):
The original coroutine is awaited for within an async with context for the transaction. If a SerializationError is raised, the whole thing is retried.
But this is not working as intended. The reason for that being the case lies in here:
https://github.com/fantix/gino/blob/d074dc4304cb050ba5b49a599b59af1d0d6c8e64/gino/transaction.py#L178
When a SerializationError bubbles up, the context tries to roll the transaction back. But the transaction has already been aborted, and rolling back fails with a different exception, which is the one that comes out of the context manager code.
At that point the caller has lost the information about what the original exception was, and thus my decorator doesn't work.
A possible approach would be to just log any exception raised by the statement in the link, and continue to raise the original exception.
The text was updated successfully, but these errors were encountered: