Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Raise exception instead of throw/catch for timeouts (#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: ```ruby def handle_exceptions yield rescue Exception => exc handle_error # e.g. ROLLBACK for databases raise ensure handle_exit unless exc # e.g. COMMIT for databases end Timeout.timeout(1) do handle_exceptions do do_something end end ``` 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 rails/rails#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 238c003 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. Co-authored-by: Nobuyoshi Nakada <nobu@ruby-lang.org>
- Loading branch information