grpc / grpc-haskell

gRPC library binding for Haskell.
Apache License 2.0
154 stars 16 forks source link

Remove Rpc monad #17

Open kolmodin opened 7 years ago

kolmodin commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

Discussion started here; https://plus.google.com/+ErikdeCastroLopo/posts/TbRiXuNucED Suggestion to try tryEitherT https://github.com/ambiata/x/blob/3a37cf0ace1c7223f3467c0c000fac9085c602ef/x-eithert/src/X/Control/Monad/Trans/Either.hs#L236-L238

kolmodin commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

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

erikd commented 7 years ago

What about handleEitherT from the errors package?

kolmodin commented 7 years ago

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 commented 7 years ago

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

kolmodin commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

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.