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

MonadUnliftIO #69

Open
rvl opened this issue Dec 28, 2020 · 5 comments
Open

MonadUnliftIO #69

rvl opened this issue Dec 28, 2020 · 5 comments

Comments

@rvl
Copy link

rvl commented Dec 28, 2020

Thanks for this highly useful library.

Would you consider switching from using exceptions to unliftio?
With UnliftIO.Exception, the exception handlers mask asynchronous exceptions by default, and it's less easy to catch asynchronous exceptions.
The classes MonadIO, MonadCatch, and MonadMask are all replaced by MonadUnliftIO.

I would encourage users to switch.

@pbrisbin
Copy link
Contributor

pbrisbin commented Jul 14, 2022

👋 I found this issue because we also use MonadUnliftIO all over, and using retry has introduced some odd MonadMask constraints. It's possible to define MonadUnliftIO versions in terms of the MonadMask ones:

recovering
  :: MonadUnliftIO m
  => RetryPolicyM m
  -> [RetryStatus -> Handler m Bool]
  -> (RetryStatus -> m a)
  -> m a
recovering policy hs f = withRunInIO $ \runInIO ->
  Retry.recovering
      (transPolicy runInIO policy)
      (map ((.) $ transHandler runInIO) hs)
    $ runInIO
    . f

transPolicy :: (forall a . m a -> n a) -> RetryPolicyM m -> RetryPolicyM n
transPolicy f (RetryPolicyM p) = RetryPolicyM $ f . p

transHandler :: (m a -> n a) -> Handler m a -> Handler n a
transHandler f (Handler h) = Handler $ f . h

And I'm considering making an unliftio-retry package that re-exports Control.Retry along with such replacements.

Should I proceed on that, or is there a chance retry will itself switch?

@MichaelXavier
Copy link
Contributor

@ozataman any opinions on this?

  • I think we'd just need unliftio-core which has no problematic dependencies, just base and transformers.
  • It looks like somebody did fork a retry-unliftio repo but it doesn't seem to be published to hackage.
  • There are some monads that you might use the retry combinators in that do not support MonadUnliftIO currently or maybe ever. So breaking compatibility like this might not be ideal.
  • We could however add a module like Control.Retry.UnliftIO that provides unliftio versions of everything and people could import that if they prefer.

@pbrisbin LMK what you think of the alternate module idea. If everybody likes it and you have the time, I think publishing retry with this separate module might be easier on everyone than maintaining a separate package.

@pbrisbin
Copy link
Contributor

The separate module idea sounds great! Though I think UnliftIO.Retry would make more sense as the name, that seems to be how all such modules get named. I'd be happy to PR such a module here.

@MichaelXavier
Copy link
Contributor

Good call on the module name, we should follow conventions of the unliftio world. I'll leave the door open to let Oz weigh in if he wants to but I feel like there's little downside to this: it's non-breaking and opt in, unliftio-core introduces no new transitive dependencies, and it gives people a forward-looking way to use the library. The only downside is the duplication in code but retry is fairly stable at this point so I think most of that cost will be paid once.

@pbrisbin I'd appreciate the PR greatly when you get the chance! Thank you for stepping up!

@pbrisbin
Copy link
Contributor

@MichaelXavier Here you go, #81

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

3 participants