Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(flutter_todos): fix zone mismatch error #4225

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

LukasMirbt
Copy link
Contributor

Status

READY

Breaking Changes

NO

Description

Fixes zone mismatch error by removing runZonedGuarded and handling errors through PlatformDispatcher instead.

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

@@ -13,12 +13,14 @@ void bootstrap({required TodosApi todosApi}) {
log(details.exceptionAsString(), stackTrace: details.stack);
};

PlatformDispatcher.instance.onError = (error, stack) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate a bit regarding the advantage of using PlatformDispatcher instead of wrapping runApp in a guarded zone?

Also we should probably using the WidgetsBinding to access the platform dispatcher instance as per:

Consider avoiding static references to this singleton through PlatformDispatcher.instance and instead prefer using a binding for dependency resolution such as WidgetsBinding.instance.platformDispatcher. See PlatformDispatcher.instance for more information about why this is preferred.

Copy link
Contributor Author

@LukasMirbt LukasMirbt Aug 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi!

Yea, sure!

The reason I opened this PR is because I got the following error in the console when running the flutter_todos example:

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() { }).

The migration guide for this error in the Flutter documentation suggests that

The best way to silence this message is to remove use of Zones from within the application.

(I also couldn't find any way to make the error go away and keep runZonedGuarded).

Also we should probably using the WidgetsBinding to access the platform dispatcher instance as per:

I based the PR on the following snippet from the Handling errors in Flutter documentation:

import 'package:flutter/material.dart';
import 'dart:ui';

void main() {
  MyBackend myBackend = MyBackend();
  PlatformDispatcher.instance.onError = (error, stack) {
    myBackend.sendError(error, stack);
    return true;
  };
  runApp(const MyApp());
}

Consider avoiding static references to this singleton through PlatformDispatcher.instance and instead prefer using a binding for dependency resolution such as WidgetsBinding.instance.platformDispatcher. See PlatformDispatcher.instance for more information about why this is preferred.

The same documentation mentions that

The only place that WidgetsBinding.instance.platformDispatcher is inappropriate is if access to these APIs is required before the binding is initialized by invoking runApp() or WidgetsFlutterBinding.instance.ensureInitialized(). In that case, it is necessary (though unfortunate) to use the PlatformDispatcher.instance object statically.

so it might make sense to use PlatformDispatcher.instance directly if we don't also want to add WidgetsFlutterBinding.instance.ensureInitialized().

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I opened this PR is because I got the following error in the console when running the flutter_todos example

You just ran the example with no changes and saw the error? I'll try to see if I can reproduce, thanks for the context!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can confirm that I'm still seeing this error on Flutter 3.24.1 on multiple platforms

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks I'll review this later today, apologies for the delay!

Copy link
Owner

@felangel felangel Aug 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LukasMirbt what platforms are you seeing this on? I've tested using Flutter 3.24.1 on iOS, Android, and Web and am not seeing any errors

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify, it's a warning - not an error. The app still runs and behaves normally but the warning below can be seen in the console (you may have to scroll up a bit).

I was able to reproduce this on iOS, Android and web on different machines.

It's worth noting that I also get another error first when running the example:

Error (Xcode): ../../../../.pub-cache/hosted/pub.dev/win32-5.2.0/lib/src/guid.dart:32:9: Error: Type 'UnmodifiableUint8ListView' not found.

Which I solve with
flutter pub upgrade

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() { }`).
[log] #0      BindingBase.debugCheckZone.<anonymous closure> (package:flutter/src/foundation/binding.dart:504:29)
      #1      BindingBase.debugCheckZone (package:flutter/src/foundation/binding.dart:509:6)
binding.dart:509
      #2      _runWidget (package:flutter/src/widgets/binding.dart:1463:18)
binding.dart:1463
      #3      runApp (package:flutter/src/widgets/binding.dart:1399:3)
binding.dart:1399
      #4      bootstrap.<anonymous closure> (package:flutter_todos/bootstrap.dart:21:11)
bootstrap.dart:21
      #5      _rootRun (dart:async/zone.dart:1399:13)
zone.dart:1399
      #6      _CustomZone.run (dart:async/zone.dart:1301:19)
zone.dart:1301

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran pub upgrade to resolve the win32 issue but don't see any Zone mismatch error 🤔

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine either way I'll get this merged shortly, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran pub upgrade to resolve the win32 issue but don't see any Zone mismatch error 🤔

Strange 🤔

It's fine either way I'll get this merged shortly, thanks!

Alright, no problem! 👍

@felangel felangel added the example Example application label Aug 15, 2024
@felangel felangel merged commit 8cd56bc into felangel:master Aug 27, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
example Example application
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants