Open parsonsmatt opened 1 year ago
I'd suggest replacing the polymorphic type with a
SomeException
I can't remember if there was any reason I did what I did, and this sounds perfectly reasonable to me. Wanna PR it?
Sidenote: but I wonder if we can get away with such a change as only a minor package bump :thinking:
In
bugsnag-haskell
, aBugsnagEvent
carried aBugsnagException
which had a fieldbeOriginalException
, which would allow you to modify the original exception for further processing.We use this to remove
AnnotatedException
wrapper andSomeAsyncException
for reporting so that the nextbeforeNotify
in the chain can work properly.bugsnag-hs
lacks this field entirely - upstream issue.But, the
BeforeNotify
API was modified to account for this - now the function type is(forall e. Exception e => e -> Event -> Event)
instead ofEvent -> Event
. This means that the samee
is passed to everyBeforeNotify
function, and abeforeNotiy
can't modify that originale
that is provided.Well, that's probably fine - there's two ways to get the exception in processing. The
SomeException
on the event's exceptions, and the one that is passed in.Indeed I can even see an advantage to that, because you now have both the actual orginal exception, as well as the exception currently contained in the
Bugsnag.Exception
.On a slightly unrelated note, the API around
BeforeNotify
is a bit awkward.There's not really a lot you can do with a totally polymorphic
Exception e => e ...
- justshow
,toException
,displayException
. To do anything useful with it, you'd need tofromException . toException
it.I'd suggest replacing the polymorphic type with a
SomeException
. This would allow you to makebeforeNotify
a bit more friendly:And, if you want to make it only apply on a specific exception type, you can still do that:
(which is essentially how
updateEventFromOriginalException
is defined anyway - just one fewertoException
call)