getsentry / sentry-dart

Sentry SDK for Dart and Flutter
https://sentry.io/for/flutter/
MIT License
746 stars 233 forks source link

Use PlatformDispatcher.onError instead of custom zone starting with Flutter 3.3 #988

Closed WasserEsser closed 1 year ago

WasserEsser commented 2 years ago

Platform:

Starting with Flutter 3.3, apps should now use PlatformDispatcher.onError instead of running the app in a custom zone to catch errors. I believe sentry_flutter is using a custom zone to catch errors.

Release notes: https://medium.com/flutter/whats-new-in-flutter-3-3-893c7b9af1ff#:~:text=In%20this%20release%2C%20instead%20of%20using%20a%20custom%20Zone%2C%20you%20should%20catch%20all%20errors%20and%20exceptions%20by%20setting%20the%20PlatformDispatcher.onError%20callback. The error handling page was also updated to reflect these changes: https://docs.flutter.dev/testing/errors

ueman commented 2 years ago

This scenario is already supported, but probably not as well documented as the old approach. It's also not yet enabled by default.

The following approach adheres to the new guidelines:

void main() async {
 // Do not use the appRunner callback, because that wraps your code with a Zone
 await Sentry.init(
   (options) { 
      options.addIntegration(OnErrorIntegration());
   }
  );
  runApp(MainApp());
}

For some more information, see https://github.com/getsentry/sentry-dart/pull/915

WasserEsser commented 2 years ago

@ueman Thanks for this, the integration works but unlike mentioned in #915, my debugPrint calls do not send any breadcrumbs to the server.

kuhnroyal commented 2 years ago

@ueman Thanks for this, the integration works but unlike mentioned in #915, my debugPrint calls do not send any breadcrumbs to the server.

debugPrint only creates breadcrumbs, not events. The breadcrumbs will be attached to the next event that is sent to the server.

marandaneto commented 2 years ago

We'd need to check the PlatformDispatcher.onError availability at runtime and use it instead of the zoned guard. Right now it's possible adding the integration manually but we can do better.

WasserEsser commented 2 years ago

@ueman Thanks for this, the integration works but unlike mentioned in #915, my debugPrint calls do not send any breadcrumbs to the server.

debugPrint only creates breadcrumbs, not events. The breadcrumbs will be attached to the next event that is sent to the server.

But isn't an exception an event? Shouldn't the breadcrumbs still appear?

ueman commented 2 years ago

Any exceptions causes an event to be sent. A call to debugPrint is not an exception, so it will not be send as an event. However, each call to debugPrint will be recorded as a breadcrumb which is attached to events which may or may not be send afterwards. debugPrint calls will also only be recorded in release builds, because there's no good way to intercept it without loosing logs in debug mode.

I'm not sure if that answers your question, but I'm also not really sure if I understand your question 😅

WasserEsser commented 2 years ago

I'm not sure if that answers your question, but I'm also not really sure if I understand your question 😅

This part answered it:

debugPrint calls will also only be recorded in release builds, because there's no good way to intercept it without loosing logs in debug mode.

Our debugPrints did not show up as breadcrumbs on our issues. I was wondering why they weren't being sent over along with all the other breadcrumbs because I couldn't find any documentation on this specific behavior. Now it makes sense why they didn't show up because we're trying to see them during development.

ueman commented 2 years ago

Testing exceptions in debug mode doesn't really make sense in Flutter, because unfortunately there's a lot of information stripped in release builds, especially a lot of FlutterError and UI related things. So always make sure to test exception reporting in release builds.

Vinzent03 commented 2 years ago

Am I right that when switching to PlatformDispatcher the option enablePrintBreadcrumbs doesn't work anymore, because it needs the zone?

ueman commented 2 years ago

Yes.

Technically, you can use both at the same time, but that wouldn't make any sense since only the zone would receive errors.

From my experience though, recording print statements isn't really worth it. Most people use some kind of a logger anyway and you can easily integrate Sentry into it. And most libraries never print anything at all. Of course your case might be entirely different.

Vinzent03 commented 2 years ago

Yeah, I'm using a logger, so will record the Breadcrumbs myself. Just wanted to clarify.

marandaneto commented 2 years ago

@WasserEsser I'd keep this open till we make the onError by default.

denrase commented 2 years ago

It seems that the PlatformDispatcher.onError callback is available upwards of Flutter 3.1:

https://github.com/getsentry/sentry-dart/blob/d90ceb9af40c435c7f57f16ced9e6805d8b015dd/flutter/lib/src/integrations/on_error_integration.dart#L116

Using it instead as default instead of the zone should be done starting with version 3.3

We don't have a direct way to check the flutter version with which the app using sentry was compiled with:

https://github.com/flutter/flutter/issues/61814

Is there another way to check for Flutter version 3.3? Maybe a method that was exposed in Flutter 3.3 which we could try (and fail) to call? Or am i interpreting this behaviour incorrectly? /cc @ueman

ueman commented 2 years ago

Even though that callback is available pre 3.3, there's a bug which makes it incompatible. The method signature was different between io and web making it unusable. So 3.3 is one of the earliest version where it could be used. Maybe the beta before it is also working. See https://github.com/getsentry/sentry-dart/pull/915#issuecomment-1172531005

To check whether the PlatformDispatcher.onError is available use

https://github.com/getsentry/sentry-dart/blob/7987216d70e68ca0c3e58361bf48e6528f6392d1/flutter/lib/src/integrations/on_error_integration.dart#L112-L129