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

Using Get.isDialogOpen is crashing the app #57

Closed
jlubeck opened this issue Apr 27, 2020 · 19 comments
Closed

Using Get.isDialogOpen is crashing the app #57

jlubeck opened this issue Apr 27, 2020 · 19 comments
Assignees

Comments

@jlubeck
Copy link

jlubeck commented Apr 27, 2020

This used to work before, but for some reason I am now getting a Failed assertion: boolean expression must not be null when calling if (Get.isDialogOpen)

using 2.0.1-dev (I initially saw this on 1.20.0-dev but upgraded to see if it would fix it)

[✓] Flutter (Channel beta, v1.17.0-3.2.pre, on Mac OS X 10.15.4 19E287, locale
    en-US)
    • Flutter version 1.17.0-3.2.pre at /Users/jan/flutter
    • Framework revision 2a7bc389f2 (6 days ago), 2020-04-21 20:34:20 -0700
    • Engine revision 4c8c31f591
    • Dart version 2.8.0 (build 2.8.0-dev.20.10)

And this is my MaterialApp

GetMaterialApp(
      initialRoute: '/login',
      onGenerateRoute: Router.generateRoute,
      navigatorObservers: [
        GetObserver(MiddleWare.observer),
      ],
    );

I'm thinking it must be something on the latest 1.17.0-3.2pre since it's the only thing that changed before this broke

@jlubeck
Copy link
Author

jlubeck commented Apr 27, 2020

Using this same code that we used before, I can't get it to crash, BUT, Get.IsDialogOpen is always returning false

import 'package:flutter/material.dart';
import 'package:get/get.dart';
import 'package:http/http.dart' as http;

class TestScreen extends StatelessWidget {
  void login() {
    getTestError();
    getTestError();
  }

  Future<dynamic> getTestError() async {
    final response =
        await http.get('http://www.mocky.io/v2/5e97d4673000007900b6e08c');
    if (response.statusCode == 200) {
      return response.body;
    } else {
      print('error');
      if (!Get.isDialogOpen) {
        print('show error dialog');
        await Get.defaultDialog(
          title: 'Error',
          content: Text('There was an error'),
          confirm: FlatButton(
            child: Text('OK'),
            onPressed: () {
              Get.back();
            },
          ),
        );
      }
    }
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: Container(
          child: FlatButton(
            child: Text('Test'),
            onPressed: login,
          ),
        ),
      ),
    );
  }
}

@jonataslaw
Copy link
Owner

jonataslaw commented Apr 27, 2020

GetMaterialApp(
      initialRoute: '/login',
      onGenerateRoute: Router.generateRoute, // update to namedRoutes.
     // OnGenerateRoute is not working in the latest version of the master. Named routes are 
     // completely broken in Flutter, so I created this other API to do alternative work on it to make it work.
 //     navigatorObservers: [
 //        GetObserver(MiddleWare.observer),
 //     ],
      routingCallback: MiddleWare.observer, // you dont need more from GetObserver
    );

I was looking at that at the time.
Fixed on 2.0.4-dev

And thinking about how I'm going to backport to Stable without regression. Flutter needs to update stable to urgent 1.17.0, the API for 1.12.0 is very limited.
I removed the Dialog modified in version 2.0 because the new Flutter API allows me to do something like this callback, in the stable one I will probably have to revert the dialogs API update.

@jlubeck
Copy link
Author

jlubeck commented Apr 27, 2020

Using 2.0.4-dev and that snippet for the config now crashes my app on launch with this error:

GetMaterialApp(
      initialRoute: '/test',
      onGenerateRoute: Router.generateRoute,
      routingCallback: MiddleWare.observer,
    );
flutter: ══╡ EXCEPTION CAUGHT BY WIDGETS LIBRARY ╞═══════════════════════════════════════════════════════════
flutter: The following NoSuchMethodError was thrown building GetMaterialApp(dirty, state:
flutter: _GetMaterialAppState#726ed):
flutter: The method '[]' was called on null.
flutter: Receiver: null
flutter: Tried calling: []("/test")
flutter:
flutter: The relevant error-causing widget was:
flutter:   GetMaterialApp file:///Users/jan/Projects/flutter/rpm/lib/app.dart:10:12
flutter:
flutter: When the exception was thrown, this was the stack:
flutter: #0      Object.noSuchMethod (dart:core-patch/object_patch.dart:53:5)
flutter: #1      _GetMaterialAppState.build (package:get/src/root/root_widget.dart:166:46)
flutter: #2      StatefulElement.build (package:flutter/src/widgets/framework.dart:4619:28)
flutter: #3      ComponentElement.performRebuild (package:flutter/src/widgets/framework.dart:4502:15)
flutter: #4      StatefulElement.performRebuild (package:flutter/src/widgets/framework.dart:4675:11)
flutter: #5      Element.rebuild (package:flutter/src/widgets/framework.dart:4218:5)
flutter: #6      ComponentElement._firstBuild (package:flutter/src/widgets/framework.dart:4481:5)
flutter: #7      StatefulElement._firstBuild (package:flutter/src/widgets/framework.dart:4666:11)
flutter: #8      ComponentElement.mount (package:flutter/src/widgets/framework.dart:4476:5)
flutter: ...     Normal element mounting (7 frames)
flutter: #15     Element.inflateWidget (package:flutter/src/widgets/framework.dart:3446:14)
flutter: #16     Element.updateChild (package:flutter/src/widgets/framework.dart:3214:18)
flutter: #17     ComponentElement.performRebuild (package:flutter/src/widgets/framework.dart:4527:16)
flutter: #18     _InheritedProviderScopeMixin.performRebuild (package:provider/src/inherited_provider.dart:220:11)
flutter: #19     Element.rebuild (package:flutter/src/widgets/framework.dart:4218:5)
flutter: #20     ComponentElement._firstBuild (package:flutter/src/widgets/framework.dart:4481:5)
flutter: #21     ComponentElement.mount (package:flutter/src/widgets/framework.dart:4476:5)
flutter: ...     Normal element mounting (7 frames)
flutter: #28     SingleChildWidgetElementMixin.mount (package:nested/nested.dart:223:11)
flutter: #29     Element.inflateWidget (package:flutter/src/widgets/framework.dart:3446:14)
flutter: #30     Element.updateChild (package:flutter/src/widgets/framework.dart:3214:18)
flutter: #31     RenderObjectToWidgetElement._rebuild (package:flutter/src/widgets/binding.dart:1148:16)
flutter: #32     RenderObjectToWidgetElement.mount (package:flutter/src/widgets/binding.dart:1119:5)
flutter: #33     RenderObjectToWidgetAdapter.attachToRenderTree.<anonymous closure> (package:flutter/src/widgets/binding.dart:1061:17)
flutter: #34     BuildOwner.buildScope (package:flutter/src/widgets/framework.dart:2607:19)
flutter: #35     RenderObjectToWidgetAdapter.attachToRenderTree (package:flutter/src/widgets/binding.dart:1060:13)
flutter: #36     WidgetsBinding.attachRootWidget (package:flutter/src/widgets/binding.dart:941:7)
flutter: #37     WidgetsBinding.scheduleAttachRootWidget.<anonymous closure> (package:flutter/src/widgets/binding.dart:922:7)
flutter: (elided 11 frames from class _RawReceivePortImpl, class _Timer, dart:async, and dart:async-patch)
flutter:
flutter: ════════════════════════════════════════════════════════════════════════════════════════════════════

@jonataslaw
Copy link
Owner

Okay, fixing the routes on the Master broke the onGenerateRoute on the Dev. If you used namedRoutes instead of onGenerateRoute, the error wouldn't appear, but either way, I'm going to reverse the fix on the Master and open an issue there, this should be fixed by them, definitely.

@jonataslaw
Copy link
Owner

2.0.5-dev will work to you

@jlubeck
Copy link
Author

jlubeck commented Apr 27, 2020

ok, 2.0.5-dev fixes the sample code which was always returning false... but my original crash is still there... will try to make reproducible code

@jlubeck
Copy link
Author

jlubeck commented Apr 27, 2020

I can't for the life of me replicate outside my project... But do you have ANY idea why it COULD return null? I'm not sure what else to check.

This is what I'm seeing on the Debugger:

Screen Shot 2020-04-27 at 3 06 47 PM

@jonataslaw
Copy link
Owner

Using GetMaterialApp, it is impossible for this value to return null, because from the first route, it already initializes with a value true/false.
The only way it is null is that the dialog is not receiving a routeName, and this was fixed in the last update. Check your pubspec and use flutter pub update more than once, if possible, use flutter clean too, the flutter keeps cache of packages also inside the build folder.

@jonataslaw
Copy link
Owner

GetMaterialApp
initialRoute: '/'

MaterialApp init route '/'
when it push initial route, GetObserver put on Get.isDialog(false) Get.currentRoute('/')
The value change when you open dialog.

Enable the logs and see if you find this reference as soon as the app starts:
[GOING TO ROUTE] /

At the very moment that this log is written, Get.isDialogOpen receives false.
If you have problems with the Observer, and this log is not recorded, it means that Get.isDialogOpen will receive null.

If you saw this code in the log and Get.isDialogOpen returned null, it can only be a remnant of the old update.

A quick way to confirm this is to use a widget from the same tile:

Open a snackbar, and test Get.isSnackbarOpen instead of Get.isDialogOpen. The snackbar API has not changed, which means that it never failed, if it receives null too, the error is in the observer, if it receives true or false, the error is in remnants of old updates.

@jlubeck
Copy link
Author

jlubeck commented Apr 27, 2020

I do see the log. Can you tell me where you are setting the initial Get.isDialog(false) ?
I can't find it in your code so it looks like indeed I have an old version still, but want to make sure it is there when I upgrade

@jonataslaw
Copy link
Owner

Get.dialog is just a fullscreen page. Her name is 'dialog' by default (in the future I will give you the option to define the name of the dialog), and it is called as a normal named route internally.
GetObserver only listens to route events and whenever a "dialog" is currentRoute, it will set Get.isDialogOpen to true, when you close it, the previous route (which is not a dialog) will be in the foreground, changing the currentRoute , and making Get.isDialogOpen false.

Who manages all this is the Routing class, which heard events from GetObserver.

@jlubeck
Copy link
Author

jlubeck commented Apr 27, 2020

Ok, I think I found the issue.
When I navigate to one of my screens, this is what I was seeing in the logs:

flutter: [REPLACE ROUTE] /billing-log
flutter: [NEW ROUTE] /billing

Looking for REPLACE ROUTE in your code, I found only 1 hit on route_observer: didReplace

This is the code for didReplace

  void didReplace({Route newRoute, Route oldRoute}) {
    super.didReplace(newRoute: newRoute, oldRoute: oldRoute);
    if (Get.isLogEnable) print("[REPLACE ROUTE] ${oldRoute?.settings?.name}");
    if (Get.isLogEnable) print("[NEW ROUTE] ${newRoute?.settings?.name}");

    final routeSend = Routing(
        removed: null, // add '${oldRoute?.settings?.name}' or remain null ???
        isBack: false,
        route: newRoute,
        current: '${newRoute?.settings?.name}',
        previous: '${oldRoute?.settings?.name}',
        args: newRoute?.settings?.arguments,
        isSnackbar: null,
        isBottomSheet: null,
        isDialog: null,
        previousArgs: null);

    if (routing != null) {
      routing(routeSend);
    }
    Get.setRouting(routeSend);
  }

And in there all the snackbars, bottomSheet and dialog are being set to null.

@jonataslaw
Copy link
Owner

Ok, acho que encontrei o problema.
Quando navego para uma das minhas telas, é isso que eu estava vendo nos logs:

flutter: [REPLACE ROUTE] /billing-log
flutter: [NEW ROUTE] /billing

Procurando REPLACE ROUTEno seu código, encontrei apenas 1 resultado em route_observer:didReplace

Este é o código para didReplace

  void  didReplace ({ Route newRoute, Route oldRoute}) {
     super . didReplace (newRoute : newRoute, oldRoute : oldRoute);
    if ( Get .isLogEnable) print ( "[REPLACE ROUTE] $ { oldRoute? .settings? .name }" );
    if ( Get .isLogEnable) print ( "[NEW ROUTE] $ { newRoute? .settings? .name }" );

    final routeSend =  Routing (
        removed :  null , // adicione '$ {oldRoute? .settings? .name}' ou permaneça nulo ??? 
        isBack :  false ,
        route : newRoute,
        current :  '$ { newRoute? .settings? .name }' ,
        anterior :  '$ { oldRoute? .settings? .name }' ,
        args : newRoute ? .settings ? .arguments,
        isSnackbar :  null ,
        isBottomSheet :  null ,
        isDialog :  null ,
        previousArgs :  null );

    if (routing ! =  null ) {
       routing (routeSend);
    }
    Get . setRouting (routeSend);
  }

E lá todas as barras de lanches, bottomSheet e caixa de diálogo estão sendo definidos como nulos.

You got it!
I never thought of a situation that could be a problem, but I think you just found one. Get.offNamed() will assign null to a dialog instead of false because the route is not closed, it is replaced, therefore, the new route will not assign false, because it will not be "called", it was just exchanged for another route.

@jlubeck
Copy link
Author

jlubeck commented Apr 27, 2020

By the way, I also see the null setting on didRemove

So in those 2 cases, Get.isDialogOpen will return null and crash the app

@jlubeck
Copy link
Author

jlubeck commented Apr 27, 2020

I guess you don't need to set isDialog to false in there if it might bring other problems.

But maybe you can do this instead?

  static bool get isDialogOpen => _routing.isDialog ?? false;

@jonataslaw
Copy link
Owner

didRemove should not change the current status of a dialog, because you can open a dialog and remove a specific route (example, remove an ADS page if it is a payment dialog) if something happens in it for example, in which case the dialog will continue Open.
Apparently I will have to make these variables of Global scope in the class so that instead of assigning null, they remain with the same value in place of didRemove.

@jlubeck
Copy link
Author

jlubeck commented Apr 27, 2020

That sounds like some extra work for you... in the meantime, FYI, doing this on get_main fixes the issue for me:

  static bool get isSnackbarOpen => _routing.isSnackbar ?? false;

  /// check if dialog is open
  static bool get isDialogOpen => _routing.isDialog ?? false;

  /// check if bottomsheet is open
  static bool get isBottomSheetOpen => _routing.isBottomSheet ?? false;

Wether or not having dialogs as null in the library is needed, I think it is safe to assume that no dialog is open when it is set as null right?. So I think this is a clean and safe solution for this problem. What do you think?

@jonataslaw
Copy link
Owner

That sounds like some extra work for you... in the meantime, FYI, doing this on get_main fixes the issue for me:

  static bool get isSnackbarOpen => _routing.isSnackbar ?? false;

  /// check if dialog is open
  static bool get isDialogOpen => _routing.isDialog ?? false;

  /// check if bottomsheet is open
  static bool get isBottomSheetOpen => _routing.isBottomSheet ?? false;

Wether or not having dialogs as null in the library is needed, I think it is safe to assume that no dialog is open when it is set as null right?. So I think this is a clean and safe solution for this problem. What do you think?

I thought about this approach, it works for substituted routes, but if someone in the future needs to remove a route that is not the current route, this may cause problems, so I modified GetObserver to not change these callbacks in case of removing a route.
I believe it is solved in 2.0.6-dev

@jlubeck
Copy link
Author

jlubeck commented Apr 27, 2020

Not sure I understood that final logic, but, it works!
So closing this one.
Thanks as always!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants