On Jul 28, 2014, at 1:54 AM, Laurens Van Houtven <_...@lvh.io> wrote:
> Honored twistedeers,
Seriously, we need to come up with a good collective noun for ourselves one
day. We should have a referendum or something.
> Consider the following (blocking) decorator, which runs a function in a
> transaction:
>
> def _with_transaction(f):
> def decorated(self, *args, **kwargs):
> ...
> try:
> result = f(self, conn, *args, **kwargs)
> except:
> txn.rollback()
> raise
> ...
> Where I to translate this logic verbatim to @inlineCallbacks, I get:
>
> def _with_transaction(f):
> @inlineCallbacks
> def decorated(self, *args, **kwargs):
> ...
> try:
> result = yield f(self, conn, *args, **kwargs)
> except:
> yield txn.rollback()
> raise
> ...
>
> However, there’s a bug here! In the except clause: there’s an (implicit)
> current exception, to be re-raised by the bare raise statement.
> Unfortunately, when doing yield txn.rollback(), that conveniently eats said
> exception.
This is actually true of _with_transaction as well. Any code, anywhere, might
call sys.exc_clear(), or do some interpreter shenanigans that accidentally make
the equivalent happen. So when you are calling "rollback" on your transaction
and perhaps running application code in a post-rollback hook (because your
database has that, right? you totally need it for some things).
And of course rollback can *itself* fail which wipes out the exception anyway -
is that what you want to happen in that condition?
> Of course, there’s a fairly simple workaround involving catching
> BaseException and capturing the exception instance explicitly.
>
> I’m wondering if this is just a leaky abstraction, or if I should report it
> as a bug in @inlineCallbacks?
I don't think there's any such thing as "just" a leaky abstraction :-). If an
abstraction leaks in an unspecified way, it's a bug. Report away.
Personally, I'd do something like this:
try:
...
except:
captured = Failure()
yield txn.rollback()
yield captured
because I find that it's more readable - rather than relying on ephemeral,
manipulable state which should (but doesn't necessarily) follow conventions
related to indentation, I explicitly capture the implicit state, on the only
line of code where it's guaranteed to be the right thing.
Also worth noting that yielding the Failure will fail the Deferred returned by
your inlineCallbacks function, implicitly terminating the execution of that
function.
-glyph
_______________________________________________
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
