-
Notifications
You must be signed in to change notification settings - Fork 70
feat: support for exception context propagation #165
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
feat: support for exception context propagation We specialize the `throwIO` call using a newly implemented `rethrowIO'` which behaves as `rethrowIO` from base 4.21 when available or like the previous `throw` implementation. In short: - Before `base-4.21`, the code is exactly as before - After `base-4.21`, the code does not override the backtrace annotations and instead uses `rethrowIO`. Example of usage / changes: The following code: ```haskell {-# LANGUAGE DeriveAnyClass #-} import Control.Concurrent.Async import Control.Exception import Control.Exception.Context import Control.Exception.Annotation import Data.Typeable import Data.Traversable import GHC.Stack data Ann = Ann String deriving (Show, ExceptionAnnotation) asyncTask :: HasCallStack => IO () asyncTask = annotateIO (Ann "bonjour") $ do error "yoto" asyncTask' :: HasCallStack => IO () asyncTask' = annotateIO (Ann "bonjour2") $ do error "yutu" main = do -- withAsync asyncTask wait concurrently asyncTask asyncTask' -- race asyncTask asyncTask' ``` When run without this commit leads to: ``` ASyncException.hs: Uncaught exception ghc-internal:GHC.Internal.Exception.ErrorCall: yoto HasCallStack backtrace: throwIO, called at ./Control/Concurrent/Async/Internal.hs:630:24 in async-2.2.5-50rpfAJ7BEc1o5OswtTMUN:Control.Concurrent.Async.Internal ``` When run with this commit: ``` *** Exception: yoto Ann "bonjour" HasCallStack backtrace: error, called at /home/guillaume//ASyncException.hs:15:3 in async-2.2.5-inplace:Main asyncTask, called at /home/guillaume//ASyncException.hs:23:16 in async-2.2.5-inplace:Main ```
aa03c27
to
91c00c5
Compare
Is there anything else that needs to be done while this is WIP? It would be excellent for exception annotations to propagate through. |
Sorry, the only reason is that I had not took time to work on that. I'm using this MR since month at work and did not had any surprising behavior. Could be turned to "ready", if it builds with older GHC (I think so), I don't see any reason why not processing any further. Even if I missed some proper |
No worries, and thanks for the work! I tried backporting this to
But sadly GHC 9.10 seems noisier - your example code gives
If you spot any obvious problem, let me know. |
My understanding of As always with exception, I need to experiment to really understand what is going on (I have an infinite respect for people who can reason about exception in their head without experimenting ;)
I need to get back into this MR actually. It seems that your branch mrkline@bb9d703 is based on one commit that is not what's inside this MR. Your commit is bb9d703, which is really alike the commit on my local repo, 5cbf92365b4501d9c102b810e8f6159b778ae615, they all are on branch I'm lost in the history there, sorry, it may be my fault (not worked on this MR since a long time and I'm used to try a lot of thing locally, change branch, force push, ...). Do you remember how you ended with commit bb9d703 and branch |
Using GHC 9.12: Indeed,
With no
(So I'm wrong, throwing with However, throwing an exception caught with
So as a first approximation, it seems that your implementation should be correct. I'll continue the experimentation, maybe that's different with ghc 9.10. |
With GHC 9.10.2:
There is no difference between This is surprising. |
Haa, I think I got it. The problem is that in newest base, From
So the reason for the duplicate call stack is that it is part of the context AND part of To be honest, I think we can keep that. This is painful, indeed, but that's part of a work in progress in the exception handling logic. Adding more complexity would, IMHO, just lead to problem in the future. |
This change is in order to have exception context propagation.
Everywhere we "re"-
throw
exception withthrow
, we instead userethrow
which does not remove the exception context. I've introducedrethrowIO'
which is just a backward compatibility wrapper overrethrowIO
andthrowIO
.I will use this commit at work for a bit of time in order to gather some feedback and maybe comeback with a more robust solution.
Example of usage / changes:
The following code:
When run without this commit leads to:
When run with this commit: