-
Notifications
You must be signed in to change notification settings - Fork 177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Try with timeout rethrow #942
Conversation
4d348cb
to
0fe4356
Compare
Hmmm, can you clarify the issue you're seeing that's prompting the changes here? I'm not sure I follow your PR description details. |
The wrapped function When By changing Effectively this means that a pattern like this (for instance in
|
b592770
to
618dab2
Compare
The previous behaviour was to return any exception as res.
Codecov Report
@@ Coverage Diff @@
## master #942 +/- ##
==========================================
- Coverage 80.44% 79.98% -0.46%
==========================================
Files 36 36
Lines 2950 2953 +3
==========================================
- Hits 2373 2362 -11
- Misses 577 591 +14
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
If I recall corretly I ran into this a while back when the connection was closed during |
@@ -33,29 +33,33 @@ function try_with_timeout(f, shouldtimeout, delay, iftimeout=() -> nothing) | |||
notify(cond, f()) | |||
catch e | |||
@debugv 1 "error executing f in try_with_timeout" | |||
notify(cond, e) | |||
isopen(timer) && notify(cond, e, error = true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to check isopen(timer)
here? It seems like it wouldn't matter whether it's open or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iftimeout()
may cause f
to throw. In that scenario I'd rather pick up the TimeoutError
in wait(cond)
and swallow the exception from f
The previous behaviour was to return any exception as res which I believe was a bug.