Skip to content
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

Should timeout/race forcibly unmask async exceptions? #74

Open
brandon-leapyear opened this issue May 7, 2021 · 2 comments
Open

Should timeout/race forcibly unmask async exceptions? #74

brandon-leapyear opened this issue May 7, 2021 · 2 comments

Comments

@brandon-leapyear
Copy link
Contributor

timeout and race use async exceptions to cancel a timed-out action. IMO, the fact that these use async exceptions are an implementation detail, so I would think the user shouldn't have to care about the fact that these use async exceptions.

For example, if someone were to do

action `finally` timeout (60 * 1000000) cleanup

I think the user would be surprised that it doesn't timeout.

@snoyberg
Copy link
Member

snoyberg commented May 7, 2021

It's a fair point, but I'd be concerned about differences in behavior versus upstream functions. We already have that with bracket, and I'm already concerned that's too much of a distinction. I'm not saying no, because I think you have a valid point, just some reservation.

To throw it into the mix, another interesting possibility would be throwing an exception when timeout or race are called with async exceptions are masked.

But more directly on the merits: there may be arguably-valid reasons to do something like that, such as ensuring that some work happens before terminating the thread. For example:

mask $ \restore -> race (restore oneThing) (bracket_ (putStrLn "starting second thing") (putStrLn "stopping second thing") (restore secondThing))

I'm not arguing this is great code. Virtually any code that uses mask is bad code (including my own!). But it's worth considering.

@brandon-leapyear
Copy link
Contributor Author

brandon-leapyear commented May 7, 2021

This is an old work account. Please reference @brandonchinn178 for all future communication


Yeah, I haven't thought about this any more than surface-level, and I'm pretty sure the code I'm thinking of can be written in a better way anyway. But I'll leave this ticket up just for future reference, in case people want to add on to the conversation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants