Exceptions not handled by user code should be `unhandled: true`

[ueman]

Currently all exceptions are marked as handled. This decision was made because exceptions in Flutter don't crash the app and thus sessions should not be marked as crashed.

Though we want to mark exceptions as unhandled to indicate that an exception caught by the root zone handler was not handled by the user.

There should be a way to differentiate.

This issue is a result of a discussion on Discord.

[bruno-garcia]

This problem is relevant to Unity too where unhandled errors (in C# scripts) also don't crash the process. But could render a blank screen

[marandaneto]

I don't see how this would be possible without changing our protocol, we also cannot forget that Flutter Apps can also crash in the native layer or if Flutter itself has a bug so we need 2 things.

handled by the user vs not handled by the user. app didn't crash vs app crashed.

both things are already filterable via the mechanism btw.

[bruno-garcia]

I don't see how this would be possible without changing our protocol, we also cannot forget that Flutter Apps can also crash in the native layer or if Flutter itself has a bug so we need 2 things.

Similar to Unity. A handled: false if it came from a Mechanism other than the Unity error handler would mean a crash.

handled by the user vs not handled by the user. app didn't crash vs app crashed. both things are already filterable via the mechanism btw.

Let's discuss this again on the API evolution Thursday.

[marandaneto]

I'll write down a DACI that introduces a new flag that would be added to the protocol, which would mean an unhandled error happened but it didn't crash the app, JS and RN have the same issue, this should consider the other platforms too.

[ueman]

Are there any updates on this?

[marandaneto]

Are there any updates on this?

unfortunately not, I'll try to get back at it next quarter, since it's a change on a few SDKs, frontend, and also ingestion, requires planning and prioritization, cc @bruno-garcia

[ueman]

In the meantime, would it be possible to add a tag like uncaught = true for each exception which isn't handled but also doesn't crash the app? I do understand that the UI wouldn't highlight it in any way, but it would at least make it possible to filter for it.

[marandaneto]

If you want to query/filter non-handled events on Dart/Flutter using Sentry Discover, you can search for e.g. is:unresolved handled:yes since the manually captured events using Sentry.captureException(...) don't set the handled flag at all. Thanks @ueman for the reminder.

[ueman]

Crashlytics also records unhandled exceptions in Flutter as fatal which is kinda the same as the unhandeld in Sentry.

[marandaneto]

The only problem for us doing it is that it breaks the release health feature yet, otherwise, all sessions will be considered "Crashed" instead of "Errored" only. I'm trying to make it part of https://github.com/getsentry/sentry/discussions/34590

[mugbug]

For this matter, I managed to mark some exceptions as unhandled manually by adding an EventProcessor that modifies the event to be sent and toggles the handled value of event.exceptions.mechanism when initializing Sentry. Of course, I just toggled that value in cases it made sense to our use cases. I think this could be done using the beforeSend parameter as well.

For context: In our case we wanted to make sure those errors were considered on the Release Health metrics

[marandaneto]

https://github.com/getsentry/sentry-dart/issues/1116 is a larger initiative but I don't expect something right away, I suggest changing this on the next major (v7). So unhandled exceptions coming from signal handlers (FlutteError, zone guard, etc) would be marked as handled: false. Only manually captured exceptions would be handled: true. IIRC Unity and .NET MAUI already do that, can you confirm @bruno-garcia or @mattjohnsonpint ?

[mattjohnsonpint]

Correct.

[marandaneto]

We need to be sure that once there's an unhandled exception on the Hybrid side, and the session is marked as crashed on the Native side (last state of the session), since the app won't crash, the Native side has to start a new and fresh session.

[marandaneto]

Example where this should be changed, https://github.com/getsentry/sentry-dart/blob/b47809a5f032149d70c9c8cdc226a1b71189ae88/dart/lib/src/default_integrations.dart#L45 Places that have handled: true should be handled: false in case they are unhandled handlers.

[denrase]

@marandaneto The 'handled' flag on the mechanism is an optional. Am i assuming correctly that null and false are both interpreted as 'false' on the server?

[marandaneto]

@marandaneto The 'handled' flag on the mechanism is an optional. Am i assuming correctly that null and false are both interpreted as 'false' on the server?

I'd say null | true is the same (confusing, yes), so we need to flip to false. https://develop.sentry.dev/sdk/event-payloads/types/#typedef-Mechanism

[denrase]

Ok, so we not only need to look at Mechanisms that explicitly set handled true, but also those that don't set the value at all. 👍

[marandaneto]

Ok, so we not only need to look at Mechanisms that explicitly set handled true, but also those that don't set the value at all. 👍

Nope, let's be a bit more pragmatic here, where it is handled: true, let's do handled: false. Take as an example: final mechanism = Mechanism(type: 'runZonedGuarded', handled: true); Becomes final mechanism = Mechanism(type: 'runZonedGuarded', handled: false); Where we don't pass a handled flag, we don't add it, keep it as it is.

[marandaneto]

Ok, so we not only need to look at Mechanisms that explicitly set handled true, but also those that don't set the value at all. 👍

just to be clear, the omission of a mechanism means, the error was handled, likely manually captured, that's why null | true are the same. handled: false is the only option left out to mark an error as not handled, crash, etc.