yesodweb/wai

Possibly avoidable impure exception in auto-update

MichaelXavier opened this issue · 1 comments

In auto-update it looks like you insulate the worker loop by catching exceptions and converting them to impure exceptions:

...
a <- catchSome $ maybe (updateAction us) id (updateActionModify <*> maybea)
...

-- | Turn a runtime exception into an impure exception, so that all 'IO'
-- actions will complete successfully. This simply defers the exception until
-- the value is forced.
catchSome :: IO a -> IO a
catchSome act = Control.Exception.catch act $ \e -> return $ throw (e :: SomeException)

It makes sense to me why you'd need to protect that loop. However, the user gets back an action IO a to demand the current value. I think it's reasonable for them to expect that when they evaluate that IO a, any exception would get thrown and if it didnt, the auto-update succeeded. However, if they don't strictly evaluate the a, then it can work its way into less defensively coded areas and blow up on evaluation. I had some code that was running something like tryAny demand and the exception escaped.

I'd like to suggest replacing catchSome with the equivalent of tryAny from safe-exceptions. The worker loop would still be safe from async exceptions. Then in the demand function you give to the user, pass that through either throw pure so they still get an IO a but the exception gets forced in a predictable place. What do you think?

Seems reasonable. It may be possible to each this with just a bit of strictness in the current code, e.g. at:

Right val -> return val

Changing:

Right val -> return val

to

Right val -> return $! val

But I'm open to other approaches.