larryaasen / upgrader

A Flutter package for prompting users to upgrade when there is a newer version of the app in the store.
MIT License
570 stars 280 forks source link

An exception is thrown when UpgradeAlert rebuilds. #370

Open AhmedLSayed9 opened 10 months ago

AhmedLSayed9 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

@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 commented 10 months ago

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 commented 10 months ago

@larryaasen Can you look into this? I've already fixed it in a forked repo, exactly at https://github.com/AhmedLSayed9/upgrader/commit/8f743dbb027b5c0262dbbcc029cdb5a8357f6099. 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 commented 10 months ago

@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 commented 6 months ago

I've forgotten about this.

PR is submitted now :)

larryaasen commented 2 months ago

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".

AhmedLSayed9 commented 2 months ago

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 commented 2 weeks ago

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

AhmedLSayed9 commented 2 weeks ago

I've fixed my PR.