fix(connectivity_plus): improve network callback unregistration handling

[KirillBorodin]

PR Description

fix: safely unregister Android network callback to prevent crash when callback already unregistered or null

Problem

The app crashes with IllegalArgumentException: NetworkCallback was not registered when the Flutter engine is destroyed, particularly during background work with WorkManager. This happens because Android's ConnectivityManager checks if networkCallback.networkRequest is null before unregistering:

if (networkCallback.networkRequest == null) {
    throw new IllegalArgumentException("NetworkCallback was not registered");
}

Since networkRequest is an internal field that we cannot check directly, and it can become null in various scenarios (double cleanup, process recreation, etc.), we need to handle the unregistration defensively.

Related Issues

https://github.com/fluttercommunity/plus_plugins/issues/1025

Checklist

- [x] I read the [Contributor Guide](https://github.com/fluttercommunity/plus_plugins/blob/main/CONTRIBUTING.md) and followed the process outlined there for submitting PRs.
- [x] I titled the PR using Conventional Commits.
- [x] I did not modify the CHANGELOG.md nor the plugin version in pubspec.yaml files.
- [x] All existing and new tests are passing.
- [x] The analyzer (flutter analyze) does not report any problems on my PR.

Breaking Change

Does this PR require plugin users to manually update their apps?

- [ ] Yes, this is a breaking change (add ! in the title).
- [x] No, this is *not* a breaking change.

[KirillBorodin]

@vbuberen , could you please review the PR? The fix will prevent the Android app from crashing. We've had quite a few users experiencing this crash over the last 30 days.

Thank you!

[vbuberen]

@vbuberen , could you please review the PR? The fix will prevent the Android app from crashing. We've had quite a few users experiencing this crash over the last 30 days.

Why don't you use the connectivity_plus from your fork? You can specify your repo as a source for download in the pubspec: https://docs.flutter.dev/packages-and-plugins/using-packages#dependencies-on-unpublished-packages

As I see the main change is only

finally {
        networkCallback = null;
      }

while the rest looks like some AI generated code as well as the PR description itself.

Were you able to confirm that this actually solves the bug that your users experience? Since there is no easy way to reproduce the issue I would like to ask you to use the package from your fork and get back later with data on whenever the issue is gone.

Also, since you already have the crash in Crashlytics, I would appreciate tech data part from the issue with details on which devices/Android versions it happens.

[bitsydarel]

@vbuberen , could you please review the PR? The fix will prevent the Android app from crashing. We've had quite a few users experiencing this crash over the last 30 days.

Why don't you use the connectivity_plus from your fork? You can specify your repo as a source for download in the pubspec: https://docs.flutter.dev/packages-and-plugins/using-packages#dependencies-on-unpublished-packages

As I see the main change is only

finally {
        networkCallback = null;
      }

while the rest looks like some AI generated code as well as the PR description itself.

Were you able to confirm that this actually solves the bug that your users experience? Since there is no easy way to reproduce the issue I would like to ask you to use the package from your fork and get back later with data on whenever the issue is gone.

Also, since you already have the crash in Crashlytics, I would appreciate tech data part from the issue with details on which devices/Android versions it happens.

Hi @vbuberen ,

We use GitHub Copilot Enterprise for pull request descriptions in our organisation.

We will use our internal fork and see if the issue is resolved.

We will come back to you with information.

@KirillBorodin, please provide the device and other information requested by @vbuberen. Thanks.

[KirillBorodin]

Stacktrace:

          Fatal Exception: java.lang.IllegalArgumentException: NetworkCallback was not registered
       at android.net.ConnectivityManager.unregisterNetworkCallback(ConnectivityManager.java:5578)
       at dev.fluttercommunity.plus.connectivity.ConnectivityBroadcastReceiver.onCancel(ConnectivityBroadcastReceiver.java:90)
       at dev.fluttercommunity.plus.connectivity.ConnectivityPlugin.teardownChannels(ConnectivityPlugin.java:51)
       at dev.fluttercommunity.plus.connectivity.ConnectivityPlugin.onDetachedFromEngine(ConnectivityPlugin.java:29)
       at io.flutter.embedding.engine.FlutterEngineConnectionRegistry.remove(FlutterEngineConnectionRegistry.java:272)
       at io.flutter.embedding.engine.FlutterEngineConnectionRegistry.remove(FlutterEngineConnectionRegistry.java:280)
       at io.flutter.embedding.engine.FlutterEngineConnectionRegistry.removeAll(FlutterEngineConnectionRegistry.java:288)
       at io.flutter.embedding.engine.FlutterEngineConnectionRegistry.destroy(FlutterEngineConnectionRegistry.java:123)
       at io.flutter.embedding.engine.FlutterEngine.destroy(FlutterEngine.java:499)
       at dev.fluttercommunity.workmanager.BackgroundWorker.stopEngine$lambda$6(BackgroundWorker.kt:150)
       at android.os.Handler.handleCallback(Handler.java:959)
       at android.os.Handler.dispatchMessage(Handler.java:100)
       at android.os.Looper.loopOnce(Looper.java:257)
       at android.os.Looper.loop(Looper.java:342)
       at android.app.ActivityThread.main(ActivityThread.java:9634)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:619)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:929)
        

[vbuberen]

We use GitHub Copilot Enterprise for pull request descriptions in our organisation.

It is great and many devs use such tools these days, including me, but it doesn't mean that the generated result should just be blindly copy-pasted without any review.

[KirillBorodin]

We use GitHub Copilot Enterprise for pull request descriptions in our organisation.

It is great and many devs use such tools these days, including me, but it doesn't mean that the generated result should just be blindly copy-pasted without any review.

@vbuberen , could you please kindly point on the "copy-pasted without any review" code in the PR? Since you mentioned:

finally {
        networkCallback = null;
      }

This will guarantee that the networkCallback instance is set to null even if unregisterNetworkCallback throws an exception, preventing the exception from happening again.

Also, the changes improve code readability by splitting the unregistering of the broadcast receiver and network callback, depending on the Android OS version.

Thank in advance!

[vbuberen]

could you please kindly point on the "copy-pasted without any review" code in the PR?

You don't need to tag person in every single comment, since there will be a notification anyway. I was talking more about the PR description and title here, since it clearly not reviewed:

1. Title is incorrect and has incorrect package name, so not really following the contribution guide.
2. The description of the issue describes the problem with IllegalArgumentException: NetworkCallback was not registered by saying that networkRequest field of the networkCallback becomes null/unaccessible, etc., but the code change is about making the networkCallback itself null, which is a bit the opposite from the exception body itself. This is why I requested to validate the fix with the fork.

But let's also talk about the code itself:

1. The fix is just finally part that I quoted already before. Rest of splitting into some functions - just regular thing that some AI model does (even the name safely.. in functions is the same) that is absolutely not needed to solve the issue raised.
2. Usage of RequiresApi to specify that at least Android API 21 is required, while the package has min Flutter 3.19.0 that already implies that the project using the package will have min API 21 (Android 5) for sure.

With that being said I am interested only in confirmation of the fix and not in further discussion on how somebody uses AI tools. It was already obvious from the initial version of the PR. Thus, I would ask to continue the discussion about the topic of the issue only here.

[KirillBorodin]

In addition, a few thoughts:

Since the crash indicates that the network callback was not registered, this means that for some reason, onListen() of EventChannel.StreamHandler wasn't called in ConnectivityBroadcastReceiver, OR connectivity.getConnectivityManager().registerDefaultNetworkCallback(networkCallback); wasn't called.

We can check when onListen() is called (perhaps when the EventChannel starts being listened to on the Dart side). Also, we need to understand when onCancel() is called, as for some reason it seems to happen before onListen().

[bitsydarel]

2. Usage of RequiresApi to specify that at least Android API 21 is required, while the package has min Flutter 3.19.0 that already implies that the project using the package will have min API 21 (Android 5) for sure.

Point 1 is preference/subjective, even a junior dev could call his function the same.

Point 2:

This does not require consumers of the package to update their minimum version, since you do have the if statement on line 89 making the call only to applications on API Level 24. Same as the line 45.

I am assuming @KirillBorodin has added the annotation to the method to silence the Linter warning of using an API that is only available on API 21.

So the second point is invalid.

@KirillBorodin after the release of one of our Flutter app with this changes, please come back here with data so we can merge this PR.

@vbuberen We will come back to you, after we've gathered the data. Thank you.

[bitsydarel]

@KirillBorodin Let's book some time to discuss the investigation report of this bug and please only return to this ticket only when you have enough data that the fix does resolve the issue.

[vbuberen]

It seems that you also have problems with reading other people's messages, so I would ask you to do so again.

You don't need to tag person in every single comment, since there will be a notification anyway.

And re-read point 2 in that message as well, since you clearly misunderstood it and wrote some non-sense which wasn't even stated in my message about consumers or whatever.

Finally, will say it again for the last time - I am interested in data regarding whenever fix works now only and not going to discuss anything else anymore here.

[bitsydarel]

It seems that you also have problems with reading other people's messages, so I would ask you to do so again.

You don't need to tag person in every single comment, since there will be a notification anyway.

And re-read point 2 in that message as well, since you clearly misunderstood it and wrote some non-sense which wasn't even stated in my message about consumers or whatever.

Finally, will say it again for the last time - I am interested in data regarding whenever fix works now only and not going to discuss anything else anymore here.

Hey mate,

This is the Flutter community, so you don’t have to be talking to contributors in discourteous way. It’s not the community values. So please have some manners. Thank you.

If you receive notifications either way, what impact does it have on you if you’re tagged? Unless there’s a code of conduct prohibiting it, I don’t understand why you would require contributors to behave differently in this scenario.