purescript / purescript-transformers

Monad and comonad transformers
BSD 3-Clause "New" or "Revised" License
69 stars 44 forks source link

StateT's catchError discards changes to state #129

Open cscalfani opened 4 years ago

cscalfani commented 4 years ago

StateT's catchError doesn't take into account that the state can change in the call to is first parameter:

instance monadErrorStateT :: MonadError e m => MonadError e (StateT s m) where
  catchError (StateT m) h =
    StateT \s -> catchError (m s) (\e -> case h e of StateT f -> f s)

StateT's function m COULD change state and if it does, that state change is lost, i.e. it's neither captured nor is it passed to f.

This is ONLY noticeable when you have a Stack where StateT is ABOVE ExceptT otherwise StateT's catchError will NOT be called.

After many tests, I've come to the conclusion that this CANNOT be fixed. Here was my initial attempt to fix it:

instance monadErrorStateT :: MonadError e m => MonadError e (StateT s m) where
  catchError (StateT m) h =
    StateT \s -> m s >>= \t@(Tuple _ s') -> catchError (pure t) (\e -> case h e of StateT f -> f s')

The problem is that ExceptT is BELOW StateT in the Stack. That means that m s >>= ... will Short-circuit, which is NOT the desired behavior here.

I cannot see any way to capture the changes made to the state by the first call that won't Short-circuit.

So, the only question that remains is how to communicate this information in the docs. The only place that makes sense in the StateT docs.

cscalfani commented 4 years ago

For reference, I've added a Gist called Monad Transformer Problem.

There's a link in the Read Me file to try it in a minimal example program.

natefaubion commented 4 years ago

This is a general problem of Monad Transformers (particularly ExceptT/MonadError) not commuting, and the ordering of the layers having important significance. This behavior happens when you layer it with anything that has some sort of output context, like WriterT as well. If you unwrap the types:

WriterT w (ExceptT e m) a
ExceptT e m (Tuple a w)
m (Either e (Tuple a w))

You'll see that you either have an error or the Writer context. That means if an error occurs, there's no way to observe any output context for layers above ExceptT/MonadError.

hdgarrood commented 4 years ago

Thanks for the report. I’m not sure I’d call this a bug; rather, I think it’s a limitation of the transformer approach. I agree with @natefaubion: I see this as one example of a more general issue that the order in which you stack your transformers can affect what’s possible in surprising ways, which is something that would be good to document here.

hdgarrood commented 4 years ago

lol, I was halfway through writing a response which would have been almost exactly the same as yours

natefaubion commented 4 years ago

FWIW, I think that this should be documented in general for the library (transformers don't commute), and particularly with ExceptT/MonadError since it is the one that causes the most trouble. The general guideline is that ExceptT should really always be your outermost layer if you have it... but there are cases where you do really want an error to revert the state (backtracking).

natefaubion commented 4 years ago

Also MaybeT.