core-libraries-committee
core-libraries-committee copied to clipboard
Exception backtrace proposal: Part 5: Annotation collection
Tracking ticket: https://github.com/haskell/core-libraries-committee/issues/164
In #202 @parsonsmatt raised an useful design goal for the exception annotation mechanism described in https://github.com/haskell/core-libraries-committee/issues/200: the user should be able to easily collect all annotations attached for an exception and any nested exceptions it may contain. This ensures that the exception annotation mechanism can cover all of the user-cases allowed by annotation-exceptions
.
Motivation
In #202 we introduced a WhileHandling
ExceptionAnnotation
to capture causal chains of exceptions:
newtype WhileHandling = WhileHandling SomeException
However, this introduces a problem: How does one reliably collect all ExceptionAnnotations
from a SomeException
given that some may be buried in a WhileHandling
annotation?
Moreover, this issue is not specific to WhileHandling
: it is common for libraries is to catch exceptions and rethrow them nested within their own exception type. See, for instance, http-client
's HttpException
type (specifically, the InternalException
constructor of HttpExceptionContent
).
Being able to reliably identify all annotations (e.g. call-stacks) associated with a given exception is important for observability.
Proposal
To address this goal, I propose the following additions to the Exception
and ExceptionAnnotation
classes:
class Exception a where
...
-- | Extract any exceptions nested within an exception.
nestedExceptions :: a -> [SomeException]
nestedExceptions = []
class ExceptionAnnotation a where
...
-- | Extract any exceptions nested within an annotation.
exceptionAnnotationNestedExceptions :: a -> [SomeException]
exceptionAnnotationNestedExceptions _ = []
The default definitions here are a bit of a compromise: while they ensure that the additions do not constitute a breaking change, they also mean that nestedExceptions
is liable to be incorrect for existing exception types.
This allows us to provide the following from Control.Exception.Annotation
:
flattenAnnotations :: Exception a => a -> [SomeExceptionAnnotation]
flattenAnnotations e = concat
[ exceptionAnnotations e
, concatMap flattenAnnotations (concatMap (\(SomeExceptionAnnotation ann) -> exceptionAnnotationNestedExceptions ann) (exceptionAnnotations e))
, concatMap flattenAnnotations (nestedExceptions e)
]
I think that this is useful right now, even without any exception annotation work being ready and merged. This would be directly useful in at least two contexts - hs-opentelemetry
#43 and bugsnag-haskell
#76, both of which are about "digging through" the exception hierarchy to see what's underlying.
I do wonder if a Tree
is a more appropriate data structure. A user could still toList
it for a totally flat representation. But the tree structure would be useful for understanding the nested nature of the type.
@parsonsmatt we cannot use Tree
from containers
in base
, unfortunately. In principle one could move its definition into base
and make containers
to reuse it, but it's going to be a proposal of its own and a controversial one.
What's the plan here? If it's useful without #202, I guess we can go for a vote.
@bgamari what's the plan here? Shall we vote?
@bgamari unless there is a feedback from your side on the question above, I'll close as dormant in two weeks.
I would be fine with moving ahead to a vote. To my mind the proposal is in an acceptable state.
Dear CLC members, let's vote on the proposal to extend class Exception
and class ExceptionAnnotation
with members to extract nested exceptions. This is not a breaking change because new members have default definitions.
class Exception a where
...
-- | Extract any exceptions nested within an exception.
nestedExceptions :: a -> [SomeException]
nestedExceptions = []
class ExceptionAnnotation a where
...
-- | Extract any exceptions nested within an annotation.
exceptionAnnotationNestedExceptions :: a -> [SomeException]
exceptionAnnotationNestedExceptions _ = []
@tomjaguarpaw @hasufell @mixphix @parsonsmatt @angerman @velveteer
+1. I don't know much about the area, but I trust on @parsonsmatt's opinion and motivation above in https://github.com/haskell/core-libraries-committee/issues/250#issuecomment-1930345876.
+1 from me. The fact that @parsonsmatt and @bgamari both consider this useful, makes me believe this is the right direction. While class changes always have me a bit uneasy, as this is not breaking anything right now. I've no objections.
+1
I abstain
I don't feel like I have a deep enough understanding of the direction of travel here to vote in favour but likewise I don't feel like I should vote against the wishes of the people who do know what they're doing. I am very uneasy about fiddling around with SomeException
because, if I understand correctly, it's one element of our exception machinery that GHC is aware of and uses. I feel like everything we're doing with exceptions here ought to be done in some way that doesn't fiddle with SomeException
itself, but I understand there are good reasons for doing that (especially making these new lovely exception features automatically available to existing code) and I don't have the knowledge or time to determine what any alternative should be. In conclusion, I don't think I can do anything other than abstain.
+1
Thanks all, that's enough votes to approve.