BeforeSend.run report.getError().getException() returns BugsnagException

[M66B]

Expected behavior

return original exception

Observed behavior

returns BugsnagException

Steps to reproduce

Use version 4.18+

Version

4.18+

Additional information

The return type of report.getError().getException() should either be a Throwable or a BugsnagException. Throwable is preferred because instanceof can be used to quickly and safely determine the exception type, whereas BugsnagException requires a string compare on the exception name.

[M66B]

getException Returns the thrown Throwable.

https://docs.bugsnag.com/platforms/android/customizing-error-reports/#getexception

[mattdyoung]

Hi @M66B

Thanks for the feedback!

Yes, this is definitely an inconsistency between the behaviour and the documentation but I'm not sure we'd be able to implement your proposed solution as in the general case the error report may be stored and delivered later (e.g. if the app is offline or crashes before the report can be sent). In these cases I don't think we can easily re-create the original exception using the right class.

We'll discuss this and agree the way forward, which may just be to align the documentation with the true behaviour if that's the best we can do.

[M66B]

I think this is rather disappointing, especially because it is a breaking change.

I had lots of exceptions unnecessary reported due to this change, hitting the rate limit.

[M66B]

Is there any news on this or am I stuck on an old Bugsnag version?

[tomlongridge]

@M66B - unless I misunderstand your use case, you shouldn't need to stay on the older version.

To summarise, the behaviour is that a BugsnagException (subclass of Throwable) is now always returned whereas previously this was only used for non-JVM (i.e. NDK) errors. From this BugsnagException object, the getCause method will return the Throwable that caused the problem if it's available.

However this return value should be null-checked before use as it will be null if:

• the error didn't originate from the JVM
• you attempt to access it in a beforeSend callback and the report has been serialised to disk -- i.e. the report is not being sent at the time of the crash due to lack of network connectivity. (Note that the above is only the case for beforeSend - it will be present in a beforeNotify callback for a JVM error as this is always triggered at crash-time.)

[M66B]

Thanks for clarifying @tomlongridge

I have moved my custom checks from beforeSend to beforeNotify and switched to error.getException().getCause(). Can you please confirm this is what you meant?

May I suggest to change the return type of getException to BugsnagException ?

[tomlongridge]

@M66B - yes, that's correct.

We're looking at a documentation update to clarify this.

[M66B]

Unfortunately, things do not work as expected: beforeNotify seems not to be called, so I had to revert the referenced commit to downgrade Bugsnag again :-(

What can be the cause of this?

Note that beforeNotify was called in version 4.17.2, which I know for sure because extra tab pages are being added. In fact I discovered this by missing these extra tab pages.

[tomlongridge]

The changes referenced look OK to me.

One thing to mention is that only JVM errors trigger the beforeNotify (see Adding Callbacks), but I presume you're trying this with Java exceptions?

Have you been able to determine that beforeNotify isn't triggered at all, as opposed to exiting early in one of the conditionals (from line 219)? I'm not aware of any issues between these two versions with this not firing, so my recommendation would be to remove as much complexity from the callback as possible and check via a debugger or logger whether the callback is fired.

[M66B]

The problem appears to be that calling Bugsnag.notify results in an empty error.getException().getCause() in beforeNotify.

Since the app catches most exceptions and reports these with Bugsnag.notify (which will filter a number of exceptions) this is not going to work :-(

[M66B]

I didn't test this, but I am wondering if setIgnoreClasses does work in this case. Not that it will be useful because custom filtering is done to filter exception 'contains' or 'startsWith' getMessage.

[M66B]

Bugsnag has been quite useful, but I am considering recent versions as broken. In any case there were undocumented breaking API changes. Therefore this issue should not be closed.

Note that I don't mean this in a bad way.

[M66B]

Proposal: why not add a callback for custom filtering based on the original exception?

[tomlongridge]

Apologies - having investigated this further, it appears that the BugsnagException returned by the error's getException method is a reconstructed representation of the Throwable that caused the error, not a wrapper as I had initially understood to be the case. Therefore calling getCause from this BugsnagException returns the cause of the Throwable (if there was one) not the original thrown object.

This change was necessary to allow the callbacks to work consistently with NDK errors and with errors that couldn't be sent at the time of the crash, so are serialised to file and sent later. Any logic that relies on the type of the exception for pre-4.18 implementations, cannot be guaranteed to work as expected for these scenarios as the exception won't be of the original type.

We are looking at straightening this out in a future major release so that access to the original Throwable is possible again. But we need to do some restructuring in order to guarantee this for all scenarios.

In the meantime, I would suggest you either use the setIgnoreClasses list (if you have an exact name) or you can use the error's getExceptionName to access the exception class name in the callback (assuming you do not have any minification/mangling in place).

[M66B]

No worries @tomlongridge As developer I understand that development isn't easy!

Unfortunately, getExceptionName won't work in my case because the exception message needs to be checked as well and sometimes also the underlying cause exception.

Background: JavaMail throws MessagingException for everything and anything and the custom logic is meant to filter things away that are no problem (like connection errors).

Maybe I can use the main thread last chance exception handler to do this, but it would be better if there was a working "before" hook for this in Bugsnag.

I guess this issue needs to be reopened until there is a solution for this.

[mattdyoung]

We're reworking this area in the next major release and can confirm that the use case you describe will be supported by the changes.

[M66B]

@mattdyoung Thanks for letting me know!

[mattdyoung]

The interface has now been reworked and improved in v5.0.0, so you can now get a reference to the actual Throwable via event.originalError in an OnError callback:
https://docs.bugsnag.com/platforms/android/customizing-error-reports/#errors

[M66B]

Thanks for actively letting me know @mattdyoung
I will try to update soon.

[mattdyoung]

Please note that we've made a number of improvements to the interface for v5.0.0, some of which are breaking changes so please refer to the upgrade guide:
https://github.com/bugsnag/bugsnag-android/blob/v5.0.0/UPGRADING.md

[M66B]

I was already aware of that, but thanks @mattdyoung. If you like, I can report about my migration experience. I will attempt that likely next week.

[M66B]

I can't upgrade because this issue is not solved and the workaround for it cannot be used anymore:

#533

If you are interested in my upgrade experience, please see this patch (untested):

M66B/FairEmail@b8ca986

I have one question: does resumeSession/pauseSession start/stop sending events to the Bugsnag server? If not, what is the right way to dynamically enable/disable reporting (privacy!).

Suggestion: add Bugsnag.leaveBreadcrumb with Map<String, String> as parameter too. In my case all Bugsnag logic is at one place, but that might not always be the case, causing extra work to upgrade leaveBreadcrumb calls.

[mattdyoung]

Hi @M66B

The #533 is solved, which has been discussed separately under that issue.

Re this question:

I have one question: does resumeSession/pauseSession start/stop sending events to the Bugsnag server? If not, what is the right way to dynamically enable/disable reporting (privacy!).

The client sends both sessions and events to Bugsnag. Event are the error reports whereas sessions are just used in conjunction with events to calculate what proportion of sessions resulted in an unhandled exception (the stability score).

When initializing the Bugsnag client, you can disable auto-capture of sessions:
https://docs.bugsnag.com/platforms/javascript/configuration-options/#autotracksessions
and disable auto-capture of errors:
https://docs.bugsnag.com/platforms/javascript/configuration-options/#autodetecterrors

If you wanted to dynamically stop sending data to Bugsnag after initialization, this can be done by using conditional logic in the onSession and onError callbacks to return false which would prevent the session or event from being sent:
https://docs.bugsnag.com/platforms/javascript/configuration-options/#onsession
https://docs.bugsnag.com/platforms/javascript/configuration-options/#onerror

Thanks for all the feedback!

[M66B]

@mattdyoung it has been done, thanks for the help!

[M66B]

@mattdyoung everything seems to work as expected!