An instance for ExceptT

[parsonsmatt]

module UnliftExcept where

import Control.Monad.Except
import UnliftIO

instance (MonadUnliftIO m, Exception e) => MonadUnliftIO (ExceptT e m) where
    withRunInIO exceptToIO = ExceptT $ try $ do
        withRunInIO $ \runInIO ->
            exceptToIO (runInIO . (either throwIO pure <=< runExceptT))

This is probably Bad for reasons I haven't figured out yet, but it works.

[snoyberg]

Huh, that's... interesting. I think it would work. I wouldn't have thought of this, because I have a knee-jerk reaction to not mix explicit error returns and runtime exceptions.

There is one potential flaw here. In the contract for ExceptT, you have a difference between "runtime exception of type E" and "Left value of type E". This approach would erase that difference. I think it's possible to argue that that would lead to confusion and maybe even violate some of the laws around MonadUnliftIO, but I haven't thought it through.

I'm definitely not ruling this out, it may be worthwhile. It additionally begs some questions around things like StateT and WriterT and whether we can smuggle the mutable state into them via an implicitly created IORef. But that sounds even crazier...

[snoyberg]

Maybe the StateT comments were overeager... ignore me :)

[parsonsmatt]

instance (MonadUnliftIO m) => MonadUnliftIO (StateT s m) where
    withRunInIO stateToIO =
        StateT $ \s -> do
            r <- newIORef s
            a <- withRunInIO $ \runInIO ->
                stateToIO $ \stateAction ->
                    runInIO $ do
                        (a, s') <- runStateT stateAction s
                        writeIORef r s'
                        pure a
            s' <- readIORef r
            pure (a, s')

Yup! This compiles.

[parsonsmatt]

There is one potential flaw here. In the contract for ExceptT, you have a difference between "runtime exception of type E" and "Left value of type E". This approach would erase that difference. I think it's possible to argue that that would lead to confusion and maybe even violate some of the laws around MonadUnliftIO, but I haven't thought it through.

I'm not sure that it does (EDIT: okay, now on coffee cup #2, realizing that i'm about to elaborate at length on exactly what you said), since the call to try means that the e exception gets pulled out of the runtime channel and put back into the Left channel. I think the ambiguity may be from the other side - an e exception thrown via IO that gets caught into the Left vs the IO channel.

I'm trying to think of a scenario where this is a problem.

foo :: ExceptT MyErr IO ()
foo = do
    -- :: ExceptT e m _
    withRunInIO $ \runInIO -> do
        -- :: IO a
        if foo
            then throwIO MyErr -- originally an IO exception
            else runInIO $ throwError MyErr -- orginally an ExceptT Left

In this code block, we have MyErr thrown both as an IO exception, and then via ExceptT (which is converted to an IO exception by runInIO).

Most of the time it seems like people want to use runExceptT as a sort of try :: ExceptT e m a -> m (Either e a), where e is the type of "caught/checked" exceptions and m carries the ability to throw unchecked exceptions. So this behavior might be bad if you want e to be conditionally checked or unchecked. Like,

foo :: ExceptT IOException IO ()
foo = do
    -- we want to catch IOExceptions in Left from opening files
    ExceptT $ try $ openFile "some file" WriteMode
    -- but we don't want to catch IOException in Left from other calls
    liftIO $ ioError $ userError "i should be an unchecked exception lol"

If we call this in-line, then we should be able to catchError and handle only "file opening" issues, while userError gets bubbled up via IO exceptions.

But if we call it via runInIO, then both of those exception values (of the same type) would get pushed into the Left channel.

🤔

Well, we could use MVars 😂

instance (MonadUnliftIO m) => MonadUnliftIO (ExceptT e m) where
    withRunInIO exceptToIO = ExceptT $ do
        exceptionRef <- newEmptyMVar
        a <- withRunInIO $ \runInIO ->
            exceptToIO $ \exceptAction -> do
                ea <- runInIO $ runExceptT exceptAction
                case ea of
                    Left err -> do
                        putMVar exceptionRef err
                        pure undefined -- wow laziness rules
                    Right a ->
                        pure a

        merr <- tryTakeMVar exceptionRef
        case merr of
            Nothing ->
                pure (Right a)
            Just e ->
                pure (Left e)

EDIT: The Exception constraint is unnecessary.

[snoyberg]

The analysis makes sense 👍 . I'm not sure how best to proceed from here though. Some possibilities:

1. Do nothing
2. Add the ExceptT instance (and maybe StateT too, thank you for that)
3. Make a separate orphans package to experiment with these instances in the wild
4. Put out a blog post about this and see if there's feedback

I lean towards (4) overall, but what are your thoughts?

[parsonsmatt]

Well, there's the "theory" behind MonadUnliftIO where it's a higher order Representable functor class, but with a fixed base of IO. These instances are definitely not in line with that, and it becomes more like "MonadUnliftIO m is definable if you can use IO to mimic the behavior of m in IO." So we do get WriterT with the same trick as MonadState.

So, is this useful? Yeah! Definitely. Does it point to a kinda bad pattern? Yeah. A big point of the ReaderT Design Pattern is that ReaderT r IO is just as powerful, quite a bit faster, and a hell of a lot easier to predict than ExceptT e (StateT s (ReaderT r (WriterT w IO))) a.

These instances prove that - "We can Unlift your ExceptT action but it takes careful thought with exceptions and a kinda nasty hack of laziness to make it work okay." "We can Unlift your StateT action but it's just an IORef under the hood, and IO exceptions roll back the state (just like they always would)." "We can Unlift your WriterT action, but it'll build up a huge data structure before you can observe any of it."

Makes me want to write corresponding IO based transformers that have the different behavior. IOStateT that does it's actions in an IORef, so exceptions don't roll back the state. IOWriterT that dumps output into a TQueue or similar. I've already written CheckedT using constraint plucking and IO exceptions to get the best of all possible worlds there.

[snoyberg]

That last point is basically what RIO is doing with the MonadState instance: https://www.stackage.org/haddock/nightly-2020-11-11/rio-0.1.19.0/RIO.html#t:RIO

[effectfully]

Well, we could use MVars
"We can Unlift your ExceptT action but it takes careful thought with exceptions and a kinda nasty hack of laziness to make it work okay."

No, you have to throw an exception, otherwise you don't get short-circuiting and attempt to continue evaluation when you don't in fact have a value to provide to the second argument of >>=. I.e. you pass there undefined and get an explosion. E.g. this

test :: IO ()
test = void . runExceptT $ withRunInIO $ \exceptToIO -> do
    x :: Bool <- exceptToIO $ throwError ()
    print x

results in

*** Exception: Prelude.undefined
CallStack (from HasCallStack):
  error, called at libraries/base/GHC/Err.hs:80:14 in base:GHC.Err

So can you simply wrap errors thrown via throwError into a data type like

newtype ViaUnliftIO e = ViaUnliftIO e

and catch those? Then an e thrown via throwIO won't get caught.

However that still requires e to be an Exception. Maybe it's best to mix the two approaches and use an MVar to store the thrown error and throw a dummy exception for the purpose of short-circuiting.

And what if the underlying action uses unsafeInterleaveIO? Then your tryTakeMVar will fire too early and you'll get the wrong result. So it's probably should be the blocking takeMVar and you should call putMVar in both the branches (Just err vs Nothing, I suppose). This all is rather subtle...

And we haven't even started talking about concurrency.

[parsonsmatt]

Great analysis, thanks!

[isovector]

The state instance doesn't do what you want.

put "written from main thread"
UnliftIO.Async.async $ do
  get  -- "written from main thread"
  put "written from thread 1"
  get  -- "written from thread 1"
  threadDelay 2
threadDelay 1
get  -- "written from main thread"
UnliftIO.Async.async $ do
  threadDelay 10
  get  -- "written from main thread"
threadDelay 1
get  -- "written from thread 1"

[parsonsmatt]

Thanks for testing it out! You're right, with an opaque computation like StateT, that's about as good as can be done. If only it were a free monad computation that could be evaluated step-by-step 😉

[lehins]

There is one potential flaw here. In the contract for ExceptT, you have a difference between "runtime exception of type E" and "Left value of type E". This approach would erase that difference.

I do agree that it would be a problem because any distinction of how exception was thrown is lost and catch will catch the E regardless how it was thrown, be it with throwIO or something like except, throwE or throwError. However I think it can be easily solved with a newtype wrapper (some optimizations with coerce is possible, but omitted for clarity):

newtype InternalException e = InteralException { unInternalException :: e }
  deriving (Eq, Show)
instance Exception e => Exception (InternalException e)

instance (MonadUnliftIO m, Exception e) => MonadUnliftIO (ExceptT e m) where
  withRunInIO exceptToIO =
    withExceptT unInternalException $ ExceptT $ try $
      withRunInIO $ \runInIO ->
        exceptToIO
          (runInIO . (either (throwIO . InteralException) pure <=< runExceptT))

The only other problem I see is that using catchAny and others that operate on SomeException, in the ExceptT e m will still capture the exception e. Consider:

data FooException = FooException
  deriving (Eq, Show)
instance Exception FooException

bar :: IO ()
bar = print =<< runExceptT (action :: ExceptT FooException IO Int)
  where
    action =
      wrapper $ do
        liftIO $ print "Starting"
        _ <- throwError FooException
        liftIO $ print "Impossible"
        pure 5
    wrapper inner =
      catch inner $ \(exc :: FooException) -> 0 <$ liftIO (print $ "WHAT?? " ++ show exc)

This will produce the expected output:

λ> bar
"Starting"
Left FooException

However in example below catchAny will get in the middle of so called pure exceptions from ExcepT:

foo :: IO ()
foo = print =<< runExceptT (action :: ExceptT FooException IO Int)
  where
    action =
      wrapper $ do
        liftIO $ print "Starting"
        _ <- throwError FooException
        liftIO $ print "Impossible"
        pure 5
    wrapper inner = catchAny inner $ \exc -> 0 <$ liftIO (print $ "Gotcha: " ++ show exc)

Therefore instead of the expected Left FooException we get:

λ> foo
"Starting"
"Gotcha: FooException"
Right 0

Yet again, it would be possible to solve this by singling out InternalException and preventing it from being caught in a similar way as with ignoring and rethrowing async exceptions. However, this feels somewhat intrusive onto unliftio package IMHO.

Not sure about MonadUnliftIO laws, but one way or another the suggested approach will violate MonadError law:

In short, all "catching" mechanisms in this library will be unable to catch exceptions thrown by functions in the Control.Exception module, and vice-versa.

because regardless of how we adjust catchAny in unliftio, Control.Exceptions.catch, will still be able to catch exceptions that were meant only for ExceptT monad.

[ewilden]

Can the issue with the State instance be patched using STM Chan (or MVar) instead?

instance (MonadUnliftIO m) => MonadUnliftIO (StateT s m) where
  withRunInIO stateToIO = 
    StateT $ \s -> do
      r <- newChan
      a <- withRunInIO $ \runInIO -> 
        stateToIO $ \stateAction -> 
          runInIO $ do
            (a, s') <- runStateT stateAction s
            writeChan r s'
            pure a
      s' <- readChan r
      pure (a, s')

Then running the async example, I get:

import RIO
import RIO.State
import UnliftIO
import UnliftIO.Async

run :: RIO App ()
run = do
  _ <- flip runStateT "" $ do
    put "written from main thread"
    _ <- async $ do
      (logInfo =<< get) -- "written from main thread"
      put "written from thread 1"
      logInfo =<< get  -- "written from thread 1"
      threadDelay 2
    threadDelay 1
    logInfo =<< get  -- "written from thread 1"
    _ <- async $ do
      threadDelay 10
      logInfo =<< get  -- "written from thread 1"
    threadDelay 1
    logInfo =<< get  -- "written from thread 1"
  pure ()

[fumieval]

Strong -1 to adding StateT and ExceptT. I think the whole point of unliftio is to provide abstraction to ReaderT r IO, but not to make up instances of other monads using arbitrary constructs. If a user wants escape hatches, we should be using newtypes instead of adding them to the library.

cf. https://stackoverflow.com/questions/54779029/how-to-understand-monadunliftios-requirement-of-no-stateful-monads

[epoberezkin]

There is one potential flaw here. In the contract for ExceptT, you have a difference between "runtime exception of type E" and "Left value of type E". This approach would erase that difference. I think it's possible to argue that that would lead to confusion and maybe even violate some of the laws around MonadUnliftIO, but I haven't thought it through.

I should admit that most of the arguments are a bit over my head, but here are my two cents... I found this issue while trying (and getting stuck) writing ExceptT instance that I needed to avoid throwing exceptions where otherwise I would have Either. The use case I have is that I needed to convert IO exceptions returned by different libraries into Either ErrorType a where ErrorType is the type that includes both the possible errors of different dependencies and the logical errors generated by my code - this type is then serialised to be sent back to the user.

Initially I thought to just throw everything as exception and catch it in one place, but besides the fact that it just felt wrong to throw logical errors as exceptions, it was not possible to reliably determine the source of error - different dependencies were throwing the same exception type. So I tried the opposite approach of converting run time exceptions of dependencies into ExceptT exceptions (and my code wasn't throwing anything - it was just threading possible logical errors via ExceptT).

The only other alternatives (all bad) would be to:

1. re-throw errors of dependencies so that they can be differentiated (and still throw logical errors as well) - feels really bad
2. catch runtime exceptions in place and thread Either manually throw the code - very verbose
3. avoid using unliftio entirely and use ExceptT - difficult to handle IO exceptions with ReaderT monad...

So using this instance feels the only correct option, and I don't think that the decision about the instance for ExceptT should depend on the decisions what to do with other instances that were discussed here - these questions seem unrelated.

So +1 to add ExceptT and no opinion on other instances.

The proposed instance works ok, although maybe there is a way to implement it without throwing and immediately catching IO exceptions (it would also remove the need to derive Exception instance)? @parsonsmatt