Fix transaction extensions to handle failures correctly #25
Conversation
The `intercepting_failure` method was not handling the block's output correctly when it was a failure. It wasn't throwing `:halt` with the failure as the given handlers were raising and therefore the throwing part was never reached. We modify the method to pass the failure to the handler and skip the throwing part, while extensions need to work in two phases: 1 - From within the transaction block, they intercept a failure a first time and assign it to a `result` variable available from the outer scope. After that, they raise to rollback the transaction but return the `result` variable after that. 2 - From outside the transaction block, they throw again the failure when the `result` is actually a failure. We also make public the `throw_failure` method so that other extensions can create similar behavior. We also add a warning in the ActiveRecord extension to make it clear that the `:requires_new` option is not yet supported for this approach. We should find a way to make it work in the future. Closes #23
There was a problem hiding this comment.
@waiting-for-dev Changes look good. Thank you!
It seems like the job of writing an extension has become a bit more complicated with this change, but if that's what's necessary to provide correct behaviour, then that's fine :) The extensions still seem straightforward enough to follow.
I've left one suggestion about how we can simplify things a little. Happy for you to decide what you think is best for that.
Yeah, unfortunately now everything becomes more cumbersome, but unfortunately I can't see any other way to workaround the two mechanisms breaking the control flow in sequence :/ |
waiting-for-dev
deleted the
waiting-for-dev/fix_failure_and_transaction
branch
Overview
The
intercepting_failuremethod was not handling the block's output correctly when it was a failure. It wasn't throwing:haltwith the failure as the given handlers were raising and therefore the throwing part was never reached.We modify the method to pass the failure to the handler and skip the throwing part, while extensions need to work in two phases:
1 - From within the transaction block, they intercept a failure a first time and assign it to a
resultvariable available from the outer scope. After that, they raise to rollback the transaction but return theresultvariable after that.2 - From outside the transaction block, they throw again the failure when the
resultis actually a failure.We also make public the
throw_failuremethod so that other extensions can create similar behavior.We also add a warning in the ActiveRecord extension to make it clear that the
:requires_newoption is not yet supported for this approach. We should find a way to make it work in the future.Closes #23
Screenshots/Screencasts
Details