getsentry / sentry-dart

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

Flutter bindings zoneMismatch error when running on web target #2063

Closed ekuleshov closed 6 days ago

ekuleshov commented 1 month ago

Platform

Flutter Web

Obfuscation

Disabled

Debug Info

Disabled

Doctor

[✓] Flutter (Channel stable, 3.22.0, on macOS 13.6.6 22G630 darwin-arm64, locale en-CA) • Flutter version 3.22.0 on channel stable at *** • Upstream repository https://github.com/flutter/flutter.git • Framework revision 5dcb86f68f (10 days ago), 2024-05-09 07:39:20 -0500 • Engine revision f6344b75dc • Dart version 3.4.0 • DevTools version 2.34.3

[✓] Chrome - develop for the web • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] IntelliJ IDEA Ultimate Edition (version 2024.1.1) • IntelliJ at /Applications/IntelliJ IDEA.app • Flutter plugin version 79.1.3 • Dart plugin version 241.15989.9

[✓] Connected device (3 available) • macOS (desktop) • macos • darwin-arm64 • macOS 13.6.6 22G630 darwin-arm64 • Mac Designed for iPad (desktop) • mac-designed-for-ipad • darwin • macOS 13.6.6 22G630 darwin-arm64 • Chrome (web) • chrome • web-javascript • Google Chrome 124.0.6367.209

[✓] Network resources • All expected network resources are available.

Version

8.2.0

Steps to Reproduce

Launch a Flutter app with Sentry initialized on the web target.

The main method of the app roughly look like this:

void main() async {
  WidgetsFlutterBinding.ensureInitialized();

  await SentryFlutter.init(
    (options) async {
      options.dsn = ...;
    },
    appRunner: () async {
      await _initServices();
      runApp(MyApp(navigatorObservers: [SentryNavigatorObserver()])));
    },
  );
}

Expected Result

The Sentry.init() should not cause issue with Flutter bindings in the web target platform.

Actual Result

The following error/warning is shown when app is started on the web target. When running on Android, iOS or MacOS target no such warning is shown.

Zone mismatch.
The Flutter bindings were initialized in a different zone than is now being used. This will likely cause confusion and bugs as any zone-specific configuration will inconsistently use the configuration of the original binding initialization zone or this zone based on hard-to-predict factors such as which zone was active when a particular callback was set.
It is important to use the same zone when calling `ensureInitialized` on the binding as when calling `runApp` later.
To make this warning fatal, set BindingBase.debugZoneErrorsAreFatal to true before the bindings are initialized (i.e. as the first statement in `void main() { }`).

Are you willing to submit a PR?

No

ueman commented 1 month ago

This is caused by the WidgetsBinding being initialized in a different zones than where runApp is called. In order to fix this, you can remove the widgetsflutter binding initialzation. Sentry does this internally for you. Also the appRunner callback is run inside a custom zone to catch exceptions bubbling up to the root zone, which is what causes the whole mismatch.

You could also not use the appRunner callback, which would fix the zone mismatch, but it would cause exception in the root zone to not be reported

ekuleshov commented 1 month ago

If that is the case, why this error is only happening on web only?

It seems like Sentry init method does too many things at once (its own init, zone creation, exception handling, etc), including things not asked for, such as widget bindings. It also hijacks the default exception handler and does not leave an option to customize your own, e.g. log exception and stack traces to console when debugging in ide.

ueman commented 1 month ago

IO platform don't need the zone, only web needs it. On IO platforms, the platform dispatcher onError callback does what the zone does on web. Sentry does allow you to override exception handlers though, you just have to set them before initializing sentry. If you do it afterwards, you'll obviously override Sentry's settings. But then again, Sentry's purpose kinda is to set those exception handlers. Initializing the WidgetsBinding is also needed because otherwise Sentry can't do platform communication to set settings native to the platform. It does a lot because you need to do a lot to catch all errors.

(I'm no Sentry employee btw)

ekuleshov commented 1 month ago

IO platform don't need the zone, only web needs it. On IO platforms, the platform dispatcher onError callback does what the zone does on web.

I see, though it is hidden "magic" and things could have been more explicit, or at least documented in the SentryFlutter.init(..) method docs.

But then again, Sentry's purpose kinda is to set those exception handlers. Initializing the WidgetsBinding is also needed because otherwise Sentry can't do platform communication to set settings native to the platform. It does a lot because you need to do a lot to catch all errors.

I thought Sentry's purpose it to provide a service to track those exceptions and other stuff... 😀 How that handling is configured is an implementation detail. Somehow the "do your magic" way didn't work with my app...

PS: I'm coming from Crashlytics experience where things are left to a developer to configure explicitly where framework simply provided API to report the errors without trying to do everything for you.

kahest commented 1 month ago

@ekuleshov we'll consider your input 👍 (cc @buenaflor). Are your questions answered? Btw. thanks @ueman for chiming in 🙏

aprizzo commented 3 weeks ago

I'm having the same problem. I have tried to remove the WidgetsFlutterBinding.ensureInitialized() when using Flutter web, but then it doesn't log errors to Sentry anymore. I had opened another GitHub issue that got closed because it's a duplicate of this one, but I added all the details there.

buenaflor commented 2 weeks ago

@aprizzo just tested an example with your code snippet and even after removing WidgetsFlutterBinding.ensureInitialized() it still does report errors to Sentry. can you confirm again?

I don't think removing WidgetsFlutterBinding.ensureInitialized() would at all be related to this since (as mentioned above) this is done internally anyway

stefanosiano commented 2 weeks ago

hi @aprizzo I checked the other thread and found a small issue there. The way to init the SDK should be

void main() async {

  await runZonedGuarded(() async {
    await SentryFlutter.init(
      (options) => options
        // set all other options
        ..dsn = 'dsn',

      appRunner: () => runApp(SentryScreenshotWidget(child: App())),
    );
  }, (exception, stackTrace) async {
    // This is important!
    await Sentry.captureException(exception, stackTrace: stackTrace);
  });
}

Basically the second parameter of runZonedGuarded is a function that is called with all the errors happening inside that zone, and you need to call await Sentry.captureException(exception, stackTrace: stackTrace); to catch them, or they will be missed. Please, let us know if you can see the errors with this change. We will improve the situation when tackling https://github.com/getsentry/sentry-dart/issues/1943

ekuleshov commented 2 weeks ago

@stefanosiano does .captureException() call need to be awaited?

Also, when the SentryFlutter.init() configures options.logger, will that exception going to be logged with that logger?

stefanosiano commented 2 weeks ago

hi @ekuleshov .captureException() shouldn't need to be awaited. Actually, Sentry should just work even without that call, as long as you provide an appRunner to SentryFlutter.init and you are using flutter >= 3.3 More on https://github.com/getsentry/sentry-dart/tree/main/flutter#usage

bizz84 commented 6 days ago

I've just come across this issue. I originally attempted to fix it like this:

void main() async {
  // Initialize the binding on native builds only (but not on web builds).
  // More details here: https://github.com/getsentry/sentry-dart/issues/2063
  if (!kIsWeb) {
    WidgetsFlutterBinding.ensureInitialized();
  }
  await initializeFirebaseApp();
  await SentryFlutter.init(...);
}

But that didn't work, as the binding must be initialized before Firebase init is called (both on web and non-web builds).

Eventually, I ended up fixing it like this:

void main() async {
  await SentryFlutter.init(
    (options) {
      options.dsn = Env.sentryDsn;
      options.environment = getFlavor().name;
    },
    appRunner: () async {
      // * There's no need to call WidgetsFlutterBinding.ensureInitialized()
      // * since this is already done internally by SentryFlutter.init()
      // * More info here: https://github.com/getsentry/sentry-dart/issues/2063
      await initializeFirebaseApp();
      // run the app
      runApp(...);
    },
  );
}

I've tested this approach and can confirm it works on all platforms.

buenaflor commented 6 days ago

@bizz84 what's the error when initializing firebase before sentry?

buenaflor commented 6 days ago

I'm closing this issue as a dupe of https://github.com/getsentry/sentry-dart/issues/1943

pls let's continue any further discussion there

ekuleshov commented 2 days ago

Apparently there is a problem with internal Sentry's call to WidgetsFlutterBinding.ensureInitialized(). It looks like it is called after the (options) {} callback is invoked, so you can't use any code in that callback that requires bindings to be initialized. For example, can't use the PackageInfo.fromPlatform() in order to set options.release.