Skip to content

Discussion about behavior #7

Closed
Closed
@jjb

Description

@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.timeout should 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 Exception

These 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!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions