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.

Sign up for GitHub

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

Jump to bottom

An exception is thrown when UpgradeAlert rebuilds. #370

Open
AhmedLSayed9 opened this issue Jan 1, 2024 · 10 comments · May be fixed by #410
Open

An exception is thrown when UpgradeAlert rebuilds. #370

AhmedLSayed9 opened this issue Jan 1, 2024 · 10 comments · May be fixed by #410
Labels
Candidate to be closed need more information Further information is requested

Comments

@AhmedLSayed9
Copy link

Using the following sample from the examples with a button added to perform a rebuild:

class MyApp extends StatefulWidget {
  const MyApp({super.key});

  @override
  State<MyApp> createState() => _MyAppState();
}

class _MyAppState extends State<MyApp> {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Upgrader Example',
      home: UpgradeAlert(
        upgrader: Upgrader(
          debugLogging: true,
          debugDisplayAlways: true,
        ),
        child: Scaffold(
          appBar: AppBar(title: Text('Upgrader Example')),
          body: Center(
            child: TextButton(
              onPressed: () {
                setState(() {});
              },
              child: Text('Checking...'),
            ),
          ),
        ),
      ),
    );
  }
}

First build is done without errors. but if I ignore the dialog and press the button to perform a rebuild, the following error is thrown:

flutter: upgrader: initialize() not called. Must be called first.
flutter: 
#0      Upgrader.verifyInit (package:upgrader/src/upgrader.dart:384:7)
#1      Upgrader.appName (package:upgrader/src/upgrader.dart:390:5)
#2      Upgrader.body (package:upgrader/src/upgrader.dart:396:41)
#3      UpgradeAlertState.checkVersion.<anonymous closure> (package:upgrader/src/upgrade_alert.dart:143:36)
#4      new Future.delayed.<anonymous closure> (dart:async/future.dart:427:39)
#5      Timer._createTimer.<anonymous closure> (dart:async-patch/timer_patch.dart:18:15)
#6      _Timer._runTimers (dart:isolate-patch/timer_impl.dart:398:19)
#7      _Timer._handleMessage (dart:isolate-patch/timer_impl.dart:429:5)
#8      _RawReceivePort._handleMessage (dart:isolate-patch/isolate_patch.dart:184:12)

the upgrader log:

flutter: upgrader: instantiated.
flutter: upgrader: build UpgradeAlert
flutter: upgrader: need to evaluate version
flutter: upgrader: blocked: false
flutter: upgrader: debugDisplayAlways: true
flutter: upgrader: debugDisplayOnce: false
flutter: upgrader: hasAlerted: false
flutter: upgrader: shouldDisplayUpgrade: true
flutter: upgrader: shouldDisplayReleaseNotes: shouldDisplayReleaseNotes
flutter: upgrader: current locale: en_US
flutter: upgrader: languageCode: en
@AhmedLSayed9
Copy link
Author

I think the error occurs because we pass a different Upgrader instance to UpgradeAlert.

But I need to pass a different Upgrader instance with different minAppVersion. This is necessary when we use minAppVersion from Remote Config which can be updated in real time and force current users to stop using the app.

@larryaasen
Copy link
Owner

@AhmedLSayed9 I assume the problem is that you are instantiating Upgrader multiple times. As you can see in this example, you should use a final variable with Upgrader: https://github.com/larryaasen/upgrader/blob/f7f0e7da00b809e61ea2f55d8e0d0d0d074aa7f0/example/lib/main-min-app-version.dart#L26C29-L26C29

@AhmedLSayed9
Copy link
Author

AhmedLSayed9 commented Jan 1, 2024 •
edited
Loading

I see.

I think we can avoid that by storing the Upgrader instance at the state class internally to avoid such mistakes.

Also, I was expecting to be able to update minAppVersion at runtime and re-evaluate UpgradeAlert.

Can I open a PR that offer a solution for both things? Let me know if it should land on master or custom.

@AhmedLSayed9
Copy link
Author

AhmedLSayed9 commented Jan 14, 2024 •
edited
Loading

@larryaasen
Can you look into this?
I've already fixed it in a forked repo, exactly at AhmedLSayed9@8f743db.
Let me know If I should open a PR with the fix.

The fix allowed me to re-evaluate and show UpgradeAlert again when minAppVersion changes "using remote config real time updates".

@larryaasen
Copy link
Owner

@AhmedLSayed9 I looked at your code briefly and I wonder if that change will pass the unit tests. You can submit a PR if you would like and I can enable the CI action to run and review the unit test results.

@AhmedLSayed9 AhmedLSayed9 linked a pull request May 1, 2024 that will close this issue
Open
@AhmedLSayed9
Copy link
Author

I've forgotten about this.

PR is submitted now :)

@larryaasen
Copy link
Owner

I don't think this is an issue anymore with the latest changes in version 10.0.0.
You should now be able to avoid this:

The fix allowed me to re-evaluate and show UpgradeAlert again when minAppVersion changes "using remote config real time updates".

@larryaasen larryaasen added need more information Further information is requested Candidate to be closed labels Sep 13, 2024
@AhmedLSayed9
Copy link
Author

I don't think this is an issue anymore with the latest changes in version 10.0.0. You should now be able to avoid this:

I've tested it again with version 10.3.0 and the issue still exists.

@AhmedLSayed9
Copy link
Author

@larryaasen
Can you give a look? Let me know if I should fix my PR.

@AhmedLSayed9
Copy link
Author

I've fixed my PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Candidate to be closed need more information Further information is requested
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow passing different Upgrader instances AhmedLSayed9/upgrader
2 participants
@larryaasen @AhmedLSayed9