search

haskell-servant / servant

Servant is a Haskell DSL for describing, serving, querying, mocking, documenting web applications and more!
https://docs.servant.dev/
1.83k stars 413 forks source link

Broken MonadCatch instance on ClientM monad #723

Closed ondrap closed 7 years ago

ondrap commented 7 years ago

Something seems to be broken in the ClientM monad from servant-client.

import Control.Monad.Catch (onException)

snapshotSave :: ClientM ()
snapshotSave  = do
  liftIO (putStrLn "init")
  void apiCall `onException` liftIO (putStrLn "exception")
  liftIO (putStrLn "done")

The onException (and catch and all the others) just seems to be ignored. I tried this on servant 0.9.1.1 quite thoroughly, it seems not to work with 0.10 either (the MonadBaseControl doesn't seem to help, I haven't tested this a lot).

The MonadCatch is automatically derived, so this might be an error somewhere else.

phadej commented 7 years ago

Are you sure that error doesn't go into MonadError ServantError, most expected errors go there.

http://hackage.haskell.org/package/servant-client-0.10/docs/Servant-Client.html#t:ServantError

ondrap commented 7 years ago

It does, sorry, my fault. It's somewhat confusing having both...

phadej commented 7 years ago

At least in my usage where web-server uses servant-client to make further requests I never catch unchecker (MonadCatch) exceptions, they are rare and unrecoverable (and framework fails original request with 500 internal error), so far I never seen. Based on our monitoring 500 "never' happens. (Disclaimer: Data.Binary.decode throws impure exceptions, and those I do sometimes catch, I have mixed feelings about that impurity though).

OTOH, if you think there is some recoverable error, than IMO we should extend ServantError. Please open another issue if it's so.

Javran commented 5 years ago

Just want to make a quick note for people like me running into this "exception in ClientM not being caught" problem: make sure to use things from Control.Monad.Except, which is meant for receiving ServantErrors, rather than Control.Monad.Catch, which is more similar to Exception handling in IO.

I do however feel it's a bit confusing that ServantError is an instance of Exception, and that got me straight into thinking about exceptions handling in IO and spent few hours figuring out why catch caught nothing.