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

Remove Rpc monad #17

Open
kolmodin opened this issue Apr 28, 2017 · 11 comments
Open

Remove Rpc monad #17

kolmodin opened this issue Apr 28, 2017 · 11 comments

Comments

@kolmodin
Copy link
Member

kolmodin commented Apr 28, 2017

The Rpc monad is defined as

type Rpc a = ExceptT RpcError IO a

which is problematic.
It doesn't compose well. Catching errors in the monad gives a false security that exceptions have been caught, since there are also IO exceptions and pure exceptions.
Currently no other feature are needed from the monad except for exceptions (no pun intended).

I'd like to remove the Rpc monad and just throw IO exceptions, and document properly what method will throw, and on best practices.

@erikd
Copy link

erikd commented Apr 28, 2017

It doesn't compose well.

I'd like to see examples of where you see that, because in the code we have at work ExceptT composes very well. It does so because we have a library of ExceptT combinators to help with the composition.

Catching errors in the monad gives a false security that exceptions have been caught, since there are also IO exceptions and pure exceptions.

The fact that all your exceptions aren't being caught isn't a good argument for doing away with ExceptT. Instead, you should be improving your code to make sure all exceptions are caught and converted to RpcErrors.

For an example of some of our code calling createProcess and trapping exceptions in ExceptT, see:
https://github.com/ambiata/mafia/blob/master/src/Mafia/Cabal/Process.hs#L41-L60

I'd like to remove the Rpc monad and just throw IO exceptions

That would be reasonable if there is only one or two layers of software. I've been spending time chasing exceptions in some of our code. Our code is called mismi, which sits on top of amazonka, which sits on top of http-conduit which sits on top of http-client-tls which sits on top of tls which sits on top of network. The problem I've been chasing is that (very) occasionally we have been getting an exception that bubbles up to the top and kills a complex multi-threaded program.

@kolmodin
Copy link
Member Author

@erikd The ExceptT library I've used doesn't have all your fancy combinators. It looks much more composable with a proper library, indeed.

ambiata-x-eithert and its Ambiata dependencies are not on hackage. How is it intended to be used?

@erikd
Copy link

erikd commented Apr 28, 2017

We use our libraries as git submodules using mafia, a thin wrapper around cabal.

Now that you seem convinced that relying more heavily on ExceptT is a good idea, I can point you to the errors package which has most of what you need.

@kolmodin
Copy link
Member Author

I guess I need to define an alternative liftIO that will catch IO exceptions and put them into RpcError.

@erikd
Copy link

erikd commented Apr 29, 2017

What about handleEitherT from the errors package?

@kolmodin
Copy link
Member Author

Yes, though maybe it'd be nice to provide some utility that converts into RpcError.

Thanks, @erikd, looks like I can continue to use ExceptT :)

@erikd
Copy link

erikd commented Apr 29, 2017

I'm writing a blog post about this stuff. Will let you and Sean read it before I post it publically.

@kolmodin
Copy link
Member Author

@erikd Great, please show some common idioms, that'd be useful.

In the case of gRPC, RpcError may have to contain all other exceptions. Need to look into how to best model that.

@erikd
Copy link

erikd commented Apr 29, 2017

If you can wait another couple of hours, the code for my blog post will probably be ready :). Its spring where you are right? If the weather is good, you should go an enjoy it :).

@erikd
Copy link

erikd commented Apr 29, 2017

First pass at the code for my blog post is at https://github.com/erikd/exceptT-demo

You would mainly be interested in this file.

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