Fundamental problem here is that we have nested transaction() method
calls, but they all deal with the same transaction, isn't it?
I see three possible ways to handle rollback!():
1. What you say: rollback throws an exception that unwinds the stack
all the way to the top-most transaction() call. By the way, it may be
a good idea to use throw/catch instead of raise/rescue here, because
this would make it less likely that rollback exception is intercepted
by a rescue clause in the block given to transaction() method.
2. Some sort of connection state that knows when a transaction was
rolled back but not "completed" yet, and raises an error if any
database operation is attempted.
3. Begin a new transaction immediately after rollback!()
It seems obvious to me that (2) is worse than (1) (if you are going to
blow up, do so at the earliest opportunity).
Intuitively, (1) seems obviously better than (3) because it probably
covers 9 cases out of 10.
But... I'm in doubt for the following reasons:
(1) is already covered (by just raising an error). What about the
following scenario though:
1. do database operations X, Y
2. figure out that operation Z is impossible
3. rollback X and Y, and do something else.
As long as I have this code in an application, everything is fine and
dandy. What if I need to do it in a library method, which is not
guaranteed to be the top-most transaction()?
So, (1) covers 90% of cases, but those are already covered by other
means. (3), on the other hand, covers the remaining 10%.
Another benefit of (3) is that it will not break any existing code
that relies on the ability to do database operations after rollback!()
As for the principle of least surprise, (1) and (3) will both surprise
someone. If your brain (like mine) is wired so that rollback is ALWAYS
an exception, (3) is a surprise. But I'd avoid using rollback!() in
the first place.
If you are from a C or PHP 3 background (error handling through return
codes, no such thing as an exception), then you will use rollback!()
and (1) will surprise you a lot, because this piece of code:
transaction do |trx|
log "hmm, there is a problem"
contains no indication that log() won't be called.