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:
wai/auto-update/Control/AutoUpdate.hs
Line 195 in 2188619
Changing:
Right val -> return val
to
Right val -> return $! val
But I'm open to other approaches.