BeforeNotify API is less powerful than before

[parsonsmatt]

In bugsnag-haskell, a BugsnagEvent carried a BugsnagException which had a field beOriginalException, which would allow you to modify the original exception for further processing.

setOriginalException :: (Exception e) => e -> Bugsnag.Exception -> Bugsnag.Exception
setOriginalException e exn =
  exn
    { beOriginalException =
        Just (toException e)
    }

We use this to remove AnnotatedException wrapper and SomeAsyncException for reporting so that the next beforeNotify 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 of Event -> Event. This means that the same e is passed to every BeforeNotify function, and a beforeNotiy can't modify that original e 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.

beforeNotify \originalException event -> 
    ... msum $ map exception_originalException (event_exceptions event) ...

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.

newtype BeforeNotify = BeforeNotify
    { _unBeforeNotify :: forall e. Exception.Exception e => e -> Event -> Event
    }

There's not really a lot you can do with a totally polymorphic Exception e => e ... - just show, toException, displayException. To do anything useful with it, you'd need to fromException . toException it.

I'd suggest replacing the polymorphic type with a SomeException. This would allow you to make beforeNotify a bit more friendly:

beforeNotify
    :: (SomeException -> Event -> Event)
    -> BeforeNotify
beforeNotify = BeforeNotify

And, if you want to make it only apply on a specific exception type, you can still do that:

beforeNotifyOnException
  :: forall e. Exception e
  => (e -> Event -> Event)
  -> BeforeNotify
beforeNotifyOnException k = 
  BeforeNotify \someException event ->
    fromMaybe event (\e -> k e event) (fromException someException)

(which is essentially how updateEventFromOriginalException is defined anyway - just one fewer toException call)

[pbrisbin]

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 🤔