dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.36k stars 4.74k forks source link

NetworkChange NetworkAddressChanged event throws NetworkInformationException on Android #77822

Closed chriskarlsson closed 1 year ago

chriskarlsson commented 2 years ago

Description

NetworkInformationException is thrown when trying to subscribe to NetworkChange.NetworkAddressChanged on Android.

There is no indication in the source code that Android is UnsupportedOSPlatform but Android is not listed as supported in Microsoft Learn.

Reproduction Steps

Add the following to MainActivity of a clean Android or Maui application project:

NetworkChange.NetworkAddressChanged += (sender, args) =>
{
    Console.WriteLine($"Sender: {sender} args: {string.Join(", ", args.ToString())}");
};

Expected behavior

Console output when network address changes.

Actual behavior

NetworkInformationException is thrown with the quite cryptic exception message "Success".

Regression?

This was working fine when the application ran on Mono. It also works fine on .Net6-ios and .Net6-mac.

Known Workarounds

No response

Configuration

Which version of .NET is the code running on? - Behavior seen on both .Net6-android with and without Maui What OS and version, and what distro if applicable? - Android 12/API 31 What is the architecture (x64, x86, ARM, ARM64)? - ARM

Other information

No response

ghost commented 2 years ago

Tagging subscribers to this area: @dotnet/ncl See info in area-owners.md if you want to be subscribed.

Issue Details
### Description NetworkInformationException is thrown when trying to subscribe to NetworkChange.NetworkAddressChanged on Android. There is no indication in the source code that Android is UnsupportedOSPlatform but Android is not listed as supported in [Microsoft Learn](https://learn.microsoft.com/en-us/dotnet/api/system.net.networkinformation.networkchange.networkaddresschanged?view=net-6.0). ### Reproduction Steps Add the following to MainActivity of a clean Android or Maui application project: ``` NetworkChange.NetworkAddressChanged += (sender, args) => { Console.WriteLine($"Sender: {sender} args: {string.Join(", ", args.ToString())}"); }; ``` ### Expected behavior Console output when network address changes. ### Actual behavior NetworkInformationException is thrown with the quite cryptic exception message "Success". ### Regression? This was working fine when the application ran on Mono. It also works fine on .Net6-ios and .Net6-mac. ### Known Workarounds _No response_ ### Configuration Which version of .NET is the code running on? - Behavior seen on both .Net6-android with and without Maui What OS and version, and what distro if applicable? - Android 12/API 31 What is the architecture (x64, x86, ARM, ARM64)? - ARM ### Other information _No response_
Author: chriskarlsson
Assignees: -
Labels: `area-System.Net`, `untriaged`
Milestone: -
ghost commented 2 years ago

Tagging subscribers to 'arch-android': @steveisok, @akoeplinger See info in area-owners.md if you want to be subscribed.

Issue Details
### Description NetworkInformationException is thrown when trying to subscribe to NetworkChange.NetworkAddressChanged on Android. There is no indication in the source code that Android is UnsupportedOSPlatform but Android is not listed as supported in [Microsoft Learn](https://learn.microsoft.com/en-us/dotnet/api/system.net.networkinformation.networkchange.networkaddresschanged?view=net-6.0). ### Reproduction Steps Add the following to MainActivity of a clean Android or Maui application project: ``` NetworkChange.NetworkAddressChanged += (sender, args) => { Console.WriteLine($"Sender: {sender} args: {string.Join(", ", args.ToString())}"); }; ``` ### Expected behavior Console output when network address changes. ### Actual behavior NetworkInformationException is thrown with the quite cryptic exception message "Success". ### Regression? This was working fine when the application ran on Mono. It also works fine on .Net6-ios and .Net6-mac. ### Known Workarounds _No response_ ### Configuration Which version of .NET is the code running on? - Behavior seen on both .Net6-android with and without Maui What OS and version, and what distro if applicable? - Android 12/API 31 What is the architecture (x64, x86, ARM, ARM64)? - ARM ### Other information _No response_
Author: chriskarlsson
Assignees: -
Labels: `area-System.Net`, `os-android`, `untriaged`
Milestone: -
am11 commented 2 years ago

(aside from the NetworkAddressChanged event issue)

NetworkInformationException is thrown with the quite cryptic exception message "Success".

If this exception is thrown from https://github.com/dotnet/runtime/blob/13733cdd969842de9c36235d730a604ce55ee729/src/libraries/System.Net.NetworkInformation/src/System/Net/NetworkInformation/NetworkAddressChange.Unix.cs#L172-L176 it shouldn't print "Success". @akoeplinger, looks like there is also something wrong in the error mapping? 🤔

steveisok commented 2 years ago

@simonrozsival, please take a look.

simonrozsival commented 2 years ago

I can reproduce the bug locally. I'll continue investigating the issue tomorrow.

akoeplinger commented 1 year ago

@simonrozsival did you happen to find out whether we could support this?

simonrozsival commented 1 year ago

@akoeplinger I looked into it and I didn't find a good 1:1 replacement Android API. There are some ConnectivityManager APIs that seemed promissing but they don't behave the same way (for example ConnectivityManager.registerNetworkCallback). I will need to revisit this and look for some alternative ways of implementing it if it's even possible with the current Android restrictions.

akoeplinger commented 1 year ago

I wonder if we could start a timer/polling loop whenever someone registers the event that would continuously look for network changes

simonrozsival commented 1 year ago

We could do that. It would certainly work but wouldn't that be quite inefficient to do on a mobile device?

simonrozsival commented 1 year ago

I'm experimenting with PeriodicTimer to poll network information. I'm still not convinced that this is the optimal implementation but it's likely good enough. I need to do more testing before I submit the PR for full review. For the time being I opened a draft PR and any suggestions are welcome.

akoeplinger commented 1 year ago

With https://github.com/dotnet/runtime/pull/80548 being merged we should be done here right? I don't think it makes sense to backport this to 7.0

simonrozsival commented 1 year ago

I don't think we should backport the implementation. I think we can close this issue now.