Misc #19740
closedBlock taking methods can't differentiate between a non-local return and a throw
Description
Opening this as Misc, as at this stage I don't have a fully formed feature request.
Ref: https://github.com/ruby/ruby/commit/1a3bcf103c582b20e9ea70dfed0ee68b24243f55
Ref: https://github.com/ruby/timeout/pull/30
Ref: https://github.com/rails/rails/pull/29333
Context¶
Rails has this problem in the Active Record transaction API.
The way it works is that it yields to a block, and if no error was raised the SQL transaction is committed, otherwise it's rolled back:
User.transaction do
do_thing
end # COMMIT
or
User.transaction do
raise SomeError
end # ROLLBACK
The problem is that there are more ways a method can be exited:
User.transaction do
return # non-local exit
end
User.transaction do
throw :something
end
In the case of a non-local return, we'd want to commit the transaction, but in the case of a throw, particularly since it's internally used by Timeout.timeout
since Ruby 2.1, we'd rather consider that an error and rollback.
But as far as I'm aware, there is not way to distinguish the two cases.
def transaction
returned = false
yield
returned = true
ensure
if $!
# error was raised
elsif returned
# no uniwnd
else
# non-local return or throw, don't know
end
end
I think it could be useful to have a way to access the currently thrown object, similar to $!
for such cases, or some other way to tell what is going on.
There is some discussion going on in https://github.com/ruby/timeout/pull/30 about whether Timeout
should throw or raise, and that may solve part of the problem, but regardless of where this leads, I think being able to check if something is being thrown would be valuable.
cc @matthewd (Matthew Draper)
FYI @jeremyevans0 (Jeremy Evans) @Eregon (Benoit Daloze)
Updated by byroot (Jean Boussier) over 1 year ago
- Related to Bug #11344: Thread.handle_interrupt(TimeoutError => :never) が効かない added
Updated by byroot (Jean Boussier) over 1 year ago
- Related to Bug #8730: "rescue Exception" rescues Timeout::ExitException added
Updated by Eregon (Benoit Daloze) over 1 year ago
I'm not sure it's good to be able to differentiate, because then it could be a mess if different gems consider return/throw/break differently, e.g. if Rails wants to commit on return
but some other library only wants to commit on "natural return", or some difference in how they treat throw
, etc.
Personally if I would design such a transaction API I would only commit on natural return, I think people shouldn't "escape" the block via return/break/throw if they want to commit it.
But I can also see the other side of the argument as discussed in https://github.com/ruby/timeout/pull/30, with the expected vs exceptional control flow.
Updated by matz (Yukihiro Matsumoto) over 1 year ago
- Status changed from Open to Closed
Unlike other languages e.g., C++ or Java, Ruby's throw
is for non-local return, so that there's no reason to treat return
and throw
differently.
Use exceptions to represent failures. I understand this issue was caused by timeout
misused catch/trhow
. But timeout
is fixed now.
Matz.