eregon
commented
2 years ago There is some discussion in https://github.com/ruby/timeout/pull/14#issuecomment-1123470017 regarding keeping the usage of throw or not to interrupt the timeout block.
IMHO it'd be better to use Exception (or subclass of Exception), but I think that needs a new issue on https://bugs.ruby-lang.org/ to discuss and decide.
It's unclear if it's OK to just use Timeout::Error inside the block because that's < RuntimeError, so it might be more likely to be caught accidentally.
Yes, more tests are welcome.
jjb
I have this experimental improvement to timeout: https://github.com/jjb/better_timeout/. It's a refresh of an old project and I'm still exploring its goals. For the purpose of this discussion we can ignore the gem itself.
While making the gem I made some expanded tests for stdlib timeout.
Here's the code that runs for the expanded tests: https://github.com/jjb/better_timeout/blob/master/test/error_lifecycle.rb
This file (from the linked line down) shows behavior for ruby 2.4+, including current HEAD of ruby/timeout: https://github.com/jjb/better_timeout/blob/master/test/test_timeout-2.4-3.0.rb#L67
I have the scenarios described with a comment above each
test_N. I have the behavior annotated with "expected", "weird", and "bad" (my opinion, and the topic of this discussion).There are three things I'd like to discuss.
1. should an exception always be raised in outer?
In my opinion,
Timeout.timeoutshould always raise an exception when it times out.If we think it shouldn't in some scenarios, then I think that behavior should be controlled by the invocation of
Timeout.timeout(in a new option), not by whether or not the inner code happens to rescue ExceptionThese tests seem to explicitly state that internal code should be allowed to swallow all exceptions, so I'm guessing I won't have any luck trying to convince this project to change 😅 https://github.com/ruby/timeout/blob/master/test/test_timeout.rb#L34-L60
I tried translating https://bugs.ruby-lang.org/issues/8730 but didn't understand the rationale behind enforcing the not-raise behavior.
2. weird behavior in test_2
In
test_2, exception to raise is not specified, but inner code does rescue Exception. Regardless of this, the inner code is not able to rescue the error. I haven't yet looked into why this is.3. are you interested in putting my tests into the test suite?
Are you interested in a PR which puts these tests into the ruby/timeout test suite? I'll redo them so that they are more in the style of the current tests, and try to rework them so that they don't use global variables, or at least use much safer namespacing if that's not possible.
If you got this far, thanks for reading! Let me know what you think!