Project

General

Profile

Actions

Misc #19740

closed

Block taking methods can't differentiate between a non-local return and a throw

Added by byroot (Jean Boussier) over 1 year ago. Updated over 1 year ago.

Status:
Closed
Assignee:
-
[ruby-core:113954]

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)


Related issues 2 (0 open2 closed)

Related to Ruby master - Bug #11344: Thread.handle_interrupt(TimeoutError => :never) が効かないClosedActions
Related to Ruby master - Bug #8730: "rescue Exception" rescues Timeout::ExitExceptionRejected08/04/2013Actions
Actions #1

Updated by byroot (Jean Boussier) over 1 year ago

  • Related to Bug #11344: Thread.handle_interrupt(TimeoutError => :never) が効かない added
Actions #2

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.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0