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

ClientIO isn't useful. #71

Open
TravisWhitaker opened this issue Oct 30, 2019 · 8 comments
Open

ClientIO isn't useful. #71

TravisWhitaker opened this issue Oct 30, 2019 · 8 comments

Comments

@TravisWhitaker
Copy link
Contributor

For better or for worse, the set of exceptions that may be thrown in IO is always open. This makes types like ExceptT e IO a essentially useless. runClientIO can actually return an IO that throws any exception, and checking for Left ClientError is just extra noise. I always use this function when interacting with this library (and all other libraries that use this pattern):

runThrowClientIO :: ClientIO a -> IO a
runThrowClientIO m = do
    res <- runExceptT m
    case res of Left e  -> throwIO e
                Right a -> pure a

Library users can be allowed to migrate away from ClientIO if they choose in a largely backwards-compatible way. The trick is to define a class like this:

class MonadIO m => MonadClient m where
    throwClientError :: ClientError -> m a

instance MonadClient IO where
    throwClientError = throwIO

instance MonadClient ClientIO where
    throwClientError = throwError

Functions in the library can then be lifted from ClientIO a to MonadClient m => m a without breaking too much existing code. Users are then free to choose ClientIO, IO, or to provide an instance for whatever monad they're already working in.

@lucasdicioccio
Copy link
Member

Yeah I sort of agree with you and was adding ClientIO when I wanted to sort of get a better control on how to cleanly handle cases with http2-thrown exceptions in a single stream in the client code itself (before that, I could live with IO-about-everything but found unsatisfying the lack of guidance in the docs). I've yet to make up my mind about the pattern, the typeclass-style could definitely work.

Allow me a bit more time please: I'll be spending my OSS-time budget working on my gRPC libs in the near term.

@TravisWhitaker
Copy link
Contributor Author

If you're open to the transition I'd be happy to open a PR implementing the MonadClient-driven approach. Just wanted to see how you felt about it before proceeding with that work.

@lucasdicioccio
Copy link
Member

lucasdicioccio commented Nov 6, 2019 via email

@ProofOfKeags
Copy link
Collaborator

Hi. @lucasdicioccio gave me the go ahead to work on maintenance for this library and I'm very interested in getting rid of ClientIO altogether. I don't have a great idea of the extended dependents of this library and as a result don't have a great idea if the backwards compatibility is a real concern or not. Especially if we can just bump major versions and rip the bandaid off altogether. @TravisWhitaker If you are still interested in this change happening I'd love to discuss further, otherwise I'll figure out what to do at some point.

@TravisWhitaker
Copy link
Contributor Author

@ProofOfKeags Great, I'd be happy to help out any way I can. I'm definitely in favor of just eliminating ClientIO with a major version bump.

@ProofOfKeags
Copy link
Collaborator

The first thing I need to figure out is how to release 0.10.0.1. I ostensibly have the hackage permissions to do so at this point but I have never actually published to hackage before. After that, I think eliminating ClientIO is the next priority.

@SkamDart
Copy link

SkamDart commented Aug 5, 2022

@ProofOfKeags Is removing ClientIO still on the roadmap?
Happy to work with @TravisWhitaker or yourself to get this into the next release.

@ProofOfKeags
Copy link
Collaborator

I have bandwidth to review and merge etc but not to do it myself so yes-ish.

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

4 participants