throw/catch is used for non-local control flow, not for exceptional situations. For exceptional situations, raise should be used instead. A timeout is an exceptional situation, so it should use raise, not throw/catch.
Timeout's implementation that uses throw/catch internally causes serious problems. Consider the following code:
defhandle_exceptionsyieldrescueException=>exchandle_error# e.g. ROLLBACK for databasesraiseensurehandle_exitunlessexc# e.g. COMMIT for databasesendTimeout.timeout(1)dohandle_exceptionsdodo_somethingendend
This kind of design ensures that all exceptions are handled as errors, and ensures that all exits (normal exit, early return, throw/catch) are not handled as errors. With Timeout's throw/catch implementation, this type of code does not work, since a timeout triggers the normal exit path.
This switches Timeout.timeout to use raise/rescue internally. It adds a Timeout::ExitException subclass of exception for the internal raise/rescue, which Timeout.timeout will convert to Timeout::Error for backwards compatibility. Timeout::Error remains a subclass of RuntimeError.
This is how timeout used to work in Ruby 2.0. It was changed in Ruby 2.1, after discussion in [Bug #8730] (commit https://github.com/ruby/timeout/commit/238c003c921e in the timeout repository). I think the change from using raise/rescue to using throw/catch has caused significant harm to the Ruby ecosystem at large, and reverting it is the most sensible choice.
From the translation of [Bug #8730], it appears the issue was that someone could rescue Exception and not reraise the exception, causing timeout errors to be swallowed. However, such code is broken anyway. Using throw/catch causes far worse problems, because then it becomes impossible to differentiate between normal control flow and exceptional control flow.
Also related to this is [Bug #11344], which changed how Thread.handle_interrupt interacted with Timeout.
Added by jeremyevans (Jeremy Evans) over 2 years ago
[ruby/timeout] Raise exception instead of throw/catch for timeouts
(https://github.com/ruby/timeout/pull/30)
throw/catch is used for non-local control flow, not for exceptional situations.
For exceptional situations, raise should be used instead. A timeout is an
exceptional situation, so it should use raise, not throw/catch.
Timeout's implementation that uses throw/catch internally causes serious problems.
Consider the following code:
This kind of design ensures that all exceptions are handled as errors, and
ensures that all exits (normal exit, early return, throw/catch) are not
handled as errors. With Timeout's throw/catch implementation, this type of
code does not work, since a timeout triggers the normal exit path.
See https://github.com/rails/rails/pull/29333 for an example of the damage
Timeout's design has caused the Rails ecosystem.
This switches Timeout.timeout to use raise/rescue internally. It adds a
Timeout::ExitException subclass of exception for the internal raise/rescue,
which Timeout.timeout will convert to Timeout::Error for backwards
compatibility. Timeout::Error remains a subclass of RuntimeError.
This is how timeout used to work in Ruby 2.0. It was changed in Ruby 2.1,
after discussion in [Bug #8730] (commit
https://github.com/ruby/timeout/commit/238c003c921e in the timeout repository). I
think the change from using raise/rescue to using throw/catch has caused
significant harm to the Ruby ecosystem at large, and reverting it is
the most sensible choice.
From the translation of [Bug #8730], it appears the issue was that
someone could rescue Exception and not reraise the exception, causing
timeout errors to be swallowed. However, such code is broken anyway.
Using throw/catch causes far worse problems, because then it becomes
impossible to differentiate between normal control flow and exceptional
control flow.
Also related to this is [Bug #11344], which changed how
Thread.handle_interrupt interacted with Timeout.
https://github.com/ruby/timeout/commit/f16545abe6
Co-authored-by: Nobuyoshi Nakada nobu@ruby-lang.org