Sentry-dart: FlutterErrorIntegration does not respect FlutterErrorDetails.silent flag

[ueman]

1. Please describe the issue and how to reproduce it, or link to a small reproducible application:

The FlutterErrorIntegration does not respect the silent flag of FlutterErrorDetails. There is probably a good reason for that, so maybe Sentry should respect that?

[marandaneto]

@ueman good point, there's no reason, the flag was unknown to me.

so just to confirm, what we could do is:

if (details.silent != true) {
  // sentry captures the event
} else {
  // bail out
}

is this the suggested behavior?

[ueman]

The docs state

This is used by certain exception handlers that catch errors that could be triggered by environmental conditions (as opposed to logic errors). For example, the HTTP library sets this flag so as to not report every 404 error to the console on end-user devices, while still allowing a custom error handler to see the errors even in release builds.

So I would probably do it like you suggested but also provide the option to report silent errors.

[marandaneto]

that's a good idea, although Sentry offers the beforeSend callback to filter out events that you don't want to send over the wire, ideally, you'd not even capture them, so a flag would be ideal.

I'd say we could go for respecting the silent != true OR options.reportSilentFlutterErrors == true, captures the event.

options.reportSilentFlutterErrors would be false by default btw, the naming is just an example, maybe we could find something better, wdyt?

[ueman]

The callback would also not know about that silent flag :( Otherwise the option sounds good to me.

[marandaneto]

@ueman yep, I guess not raising an event if the silent flag is true makes sense, but as you said, some people might still want it, adding an option (reportSilentFlutterErrors) would solve both use-cases, thank you for raising this.

[marandaneto]

Also, consider the improvements discussed here #242

Sending the content of the FlutterErrorDetails as a message rather than only the stack makes sense for some type of errors.

[marandaneto]

@ueman very true about some FlutterError errors.

sometimes there are no stack traces and FlutterErrorDetails.toString() has the offending file.
sometimes there's a stack trace, but the offending file is not in there, but it appears in the FlutterErrorDetails.toString()
I don't see a way around it other than always logging the FlutterErrorDetails.toString() as part of the exception.

it'll give some noise for issues that you can figure out just by the stack traces, but sometimes are gonna be helpful, wdyt?

[ueman]

I guess so.

Some thoughts though:

• The docs state that some diagnostics information is compacted or truncated in release builds. That should be looked at. Maybe it is not that noisy in release builds.
• I would like to raise an issue on the Flutter repo. I would really like to know how they think errors should be reported. There must be a reason that the offending file is kinda "hidden". Maybe we overlook something.
• FlutterErrorDetails are also used in packages which could make them more unpredictable

[kuhnroyal]

From my experience, the details are mostly empty in release build, as they are primarily used for the structured errors that you see in the ide or in the red screen, which only works in debug mode. Probably also disabled in release for performance.

[marandaneto]

thanks for the input, so I just tried on release mode, and surprisingly, both errors in question are not even triggered on release mode.

A RenderFlex overflowed by 1175 pixels on the right.

and

A build function returned null.\nThe offending widget is:\n Builder\nBuild functions must never return null.\nTo return an empty space that causes the building widget to fill available room, return \"Container()\". To return an empty space that takes as little room as possible, return \"Container(width: 0.0, height: 0.0)\".

[kuhnroyal]

Ah yes, render errors and AssertionError are not reported in release mode. All the code to figure these things out is stripped out in release mode.

[marandaneto]

yep, so we can't do much here, I'd say that rendering errors should trigger FlutterError.onError because it could be a regression that was published to production and you won't know there are errors unless users complain.

[ueman]

I've raised an issue on the Flutter repo: https://github.com/flutter/flutter/issues/72598

[marandaneto]

thank you @ueman

[ueman]

Okay, sooo:
Most of these are caused by AssertionErrors even though they are passed as FlutterErrors to the handler. I think this is pretty misleading, but there is nothing I can do 🤷‍♂️ So they can't be caught in release mode because they get stripped. I've added this information in this PR https://github.com/getsentry/sentry-dart/pull/251
I assume this https://github.com/flutter/flutter/issues/72598 is not going to help either.

So if the silent flag PR gets merged this issue can be closed.

[marandaneto]

thank you @ueman for helping us to figure this out, appreciate it.

it'll be available in the next release.