Annotate SomeException with Graphula seed - #108
Closed
pbrisbin wants to merge 3 commits into
Closed
Conversation
The seed is useful to re-run in a deterministic way, which is why we
prepend it to any `HUnitFailure` exception messages. This commit adds
the same treatment to all other exceptions by catching `SomeException`
and using `checkpoint` to add it as an annotation.
Here is an example:
```
Before:
uncaught exception: GenerationFailureMaxAttemptsToInsert
GenerationFailureMaxAttemptsToInsert (Just "entity already exists by this key") School}
After:
uncaught exception: AnnotatedException
AnnotatedException {annotations = [Annotation @GraphulaSeed (GraphulaSeed (-8068470525722554834)),Annotation @callstack [("checkpoint",SrcLoc {srcLocPackage = "graphula-2.0.0.0-ArtCvOVJMvo1gDWVYi6qKp", srcLocModule = "Graphula", srcLocFile = "src/Graphula.hs", srcLocStartLine = 272, srcLocStartCol = 8, srcLocEndLine = 272, srcLocEndCol = 18})]], exception = GenerationFailureMaxAttemptsToInsert (Just "entity already exists by this key") School}
```
Changing the thrown exception type is a risk, since any outer `catch`es
our users (or other libraries) have in place may no longer catch it. But
using `checkpoint` and changing `e` to `AnnotatedException e` should be
tolerable, due to the fact that `AnnotatedException.catch` exists, which
catches both types at once.
That is why the output is relatively ugly, because we want to rely on
`checkpoint` and `AnnotatedException`, so this is what you get.
I also noticed that the existing code changes the `HUnitFailure`
constructor used (both `Reason` and `ExpectedBotGot` are turned into
`Reason` by way of `formatFailureReason`). Ideally, we would not do that
(and there's no real need), but I'm leaving it alone for now, erring on
the side of a smaller diff for this change.
Closes #44.
This is necessary because we now the example throws an `AnnotatedException GenerationFailure` and not a bare `GenerationFailure` exception. This is an example of the sort of change our users might have to make because of the seed annotation feature.
cdmren
reviewed
Jul 30, 2026
| logFailingSeed :: MonadUnliftIO m => Int -> SomeException -> m a | ||
| logFailingSeed seed ex = case fromException ex of | ||
| Just hf -> rethrowHUnitWith ("Graphula with seed: " <> show seed) hf | ||
| _ -> checkpoint (Annotation $ GraphulaSeed seed) $ throwIO ex |
Contributor
There was a problem hiding this comment.
At this point I think it might make more sense to use the built-in exception context support from base (since base 4.20, so it's available in a stackage LTS) instead of annotated-exception.
Suggested change
| _ -> checkpoint (Annotation $ GraphulaSeed seed) $ throwIO ex | |
| _ -> | |
| throwIO $ | |
| addExceptionContext (GraphulaSeed seed) $ -- guard this line in a CPP conditional | |
| ex |
Member
Author
There was a problem hiding this comment.
Well that certainly sounds much better, and it doesn't change the exception type (I assume), so the downsides are gone. And with the CPP, we don't have to drop any support.
Member
Author
|
I'm going to take a new swing in a new PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Open Questions:
You can see in my second commit the kind of thing our users might have to do. It's an unusual thing to be doing, but it will be broken by this "fix".
fix: annotate SomeException with Graphula seed
ad00327
The seed is useful to re-run in a deterministic way, which is why we
prepend it to any
HUnitFailureexception messages. This commit addsthe same treatment to all other exceptions by catching
SomeExceptionand using
checkpointto add it as an annotation.Here is an example:
Changing the thrown exception type is a risk, since any outer
catchesour users (or other libraries) have in place may no longer catch it. But
using
checkpointand changingetoAnnotatedException eshould betolerable, due to the fact that
AnnotatedException.catchexists, whichcatches both types at once.
That is why the output is relatively ugly, because we want to rely on
checkpointandAnnotatedException, so this is what you get.I also noticed that the existing code changes the
HUnitFailureconstructor used (both
ReasonandExpectedBotGotare turned intoReasonby way offormatFailureReason). Ideally, we would not do that(and there's no real need), but I'm leaving it alone for now, erring on
the side of a smaller diff for this change.
Closes #44.
chore: update README to use annotated-exception's try
ad5f5b7
This is necessary because now the example throws an
AnnotatedException GenerationFailureand not a bareGenerationFailureexception.This is an example of the sort of change our users might have to make
because of the seed annotation feature.