[wasm] Support network status information #71941

maraf · 2022-07-11T13:32:21Z

Support these network status related API on browser

NetworkInterface.GetIsNetworkAvailable
NetworkChange.NetworkAvailabilityChanged

TODO

Resolve how it should behave when the browser API is missing
~~Try to control a devtools network status in unit test~~
Try to use playwright and WBT for unit test
Try blazor sdk static assets test

dotnet-issue-labeler · 2022-07-11T13:32:29Z

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

ghost · 2022-07-11T13:32:30Z

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Support these network status related API on browser

NetworkInterface.GetIsNetworkAvailable
NetworkChange.NetworkAvailabilityChanged

Fixes #38988

Author:	maraf
Assignees:	maraf
Labels:	`arch-wasm`
Milestone:	-

ghost · 2022-07-11T13:32:50Z

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

Issue Details

Support these network status related API on browser

NetworkInterface.GetIsNetworkAvailable
NetworkChange.NetworkAvailabilityChanged

Fixes #38988

Author:	maraf
Assignees:	maraf
Labels:	`arch-wasm`, `area-System.Net`, `new-api-needs-documentation`
Milestone:	-

maraf · 2022-07-11T13:32:51Z

/azp run runtime-wasm

azure-pipelines · 2022-07-11T13:33:01Z

Azure Pipelines successfully started running 1 pipeline(s).

src/libraries/System.Net.NetworkInformation/src/System.Net.NetworkInformation.csproj

...tem.Net.NetworkInformation/src/System/Net/NetworkInformation/NetworkAddressChange.Browser.cs

maraf · 2022-07-12T09:40:38Z

/azp run runtime-wasm

azure-pipelines · 2022-07-12T09:40:50Z

Azure Pipelines successfully started running 1 pipeline(s).

maraf · 2022-07-12T11:40:50Z

/azp run runtime-wasm

azure-pipelines · 2022-07-12T11:41:03Z

Azure Pipelines successfully started running 1 pipeline(s).

maraf · 2022-07-12T14:45:12Z

The browser API is not supported on V8 and NodeJS. I'm not sure how to deal with this in general, throw or let it fail in JS?

cc @pavelsavara @lewing @kg

kg · 2022-07-12T14:48:14Z

The browser API is not supported on V8 and NodeJS. I'm not sure how to deal with this in general, throw or let it fail in JS?

cc @pavelsavara @lewing @kg

I would throw. Then tests can cleanly check for it

maraf · 2022-07-12T15:43:16Z

I would throw. Then tests can cleanly check for it

Throw when the API is not available at runtime or throw "statically" when running on V8/nodejs? Do we have any general guidance for optional browser APIs?

radical · 2022-07-12T16:20:06Z

...tem.Net.NetworkInformation/src/System/Net/NetworkInformation/NetworkAddressChange.Browser.cs

+
+namespace System.Net.NetworkInformation
+{
+    public partial class NetworkChange


Why not do this like other platform specific partial classes like NetworkAddressChange.Unix.cs?

I don't understand what do you mean. Can you please give me a clue?

...tem.Net.NetworkInformation/src/System/Net/NetworkInformation/NetworkAddressChange.Browser.cs

radical · 2022-07-12T16:32:43Z

.../System.Net.NetworkInformation/src/System/Net/NetworkInformation/NetworkAddressChange.OSX.cs

@@ -39,6 +39,7 @@ public partial class NetworkChange
        private static readonly AutoResetEvent s_runLoopStartedEvent = new AutoResetEvent(false);
        private static readonly AutoResetEvent s_runLoopEndedEvent = new AutoResetEvent(false);

+        [UnsupportedOSPlatform("browser")]


IIUC, this shouldn't be needed in platform specific implementation files, since they are included in the build only for the relevant platforms. Same for others.

Are you sure? If I hadn't have them, the ApiCompat was complaining.

Make sure to include the main .cs file also. What was the output from apicompat?

If I just drop this one and try to compile with

.\dotnet.cmd build /p:TargetOS=Browser /p:TargetArchitecture=wasm /p:Configuration=Debug .\src\libraries\System.Net.NetworkInformation\src\

it fails with

....\.nuget\packages\microsoft.dotnet.apicompat\7.0.0-beta.22358.3\build\Microsoft.DotNet.ApiCompat.targets(52,5): error : Compat issues with assembly System.Net.NetworkInformation: [....\src\libraries\System.Net.NetworkInformation\src\Sy stem.Net.NetworkInformation.csproj] ....\.nuget\packages\microsoft.dotnet.apicompat\7.0.0-beta.22358.3\build\Microsoft.DotNet.ApiCompat.targets(52,5): error : CannotChangeAttribute : Attribute 'System.Runtime.Versioning.UnsupportedOSPlatformAttribute' on 'System.Net.NetworkInformation.NetworkChange .NetworkAddressChanged' changed from '[UnsupportedOSPlatformAttribute("browser")]' in the contract to '[UnsupportedOSPlatformAttribute("illumos")]' in the implementation. [....\src\libraries\System.Net.NetworkInformation\src\System.Net.NetworkInformation .csproj] ....\.nuget\packages\microsoft.dotnet.apicompat\7.0.0-beta.22358.3\build\Microsoft.DotNet.ApiCompat.targets(66,5): error : ApiCompat failed for '....\artifacts\bin\System.Net.NetworkInformation\Debug\net7.0-tvos\System.Net.NetworkInformat ion.dll' [....\src\libraries\System.Net.NetworkInformation\src\System.Net.NetworkInformation.csproj] ....\.nuget\packages\microsoft.dotnet.apicompat\7.0.0-beta.22358.3\build\Microsoft.DotNet.ApiCompat.targets(52,5): error : Compat issues with assembly System.Net.NetworkInformation: [....\src\libraries\System.Net.NetworkInformation\src\Sy stem.Net.NetworkInformation.csproj] ....\.nuget\packages\microsoft.dotnet.apicompat\7.0.0-beta.22358.3\build\Microsoft.DotNet.ApiCompat.targets(52,5): error : CannotChangeAttribute : Attribute 'System.Runtime.Versioning.UnsupportedOSPlatformAttribute' on 'System.Net.NetworkInformation.NetworkChange .NetworkAddressChanged' changed from '[UnsupportedOSPlatformAttribute("browser")]' in the contract to '[UnsupportedOSPlatformAttribute("illumos")]' in the implementation. [....\src\libraries\System.Net.NetworkInformation\src\System.Net.NetworkInformation .csproj] ....\.nuget\packages\microsoft.dotnet.apicompat\7.0.0-beta.22358.3\build\Microsoft.DotNet.ApiCompat.targets(66,5): error : ApiCompat failed for '....\artifacts\bin\System.Net.NetworkInformation\Debug\net7.0-ios\System.Net.NetworkInformati on.dll' [....\src\libraries\System.Net.NetworkInformation\src\System.Net.NetworkInformation.csproj] ....\.nuget\packages\microsoft.dotnet.apicompat\7.0.0-beta.22358.3\build\Microsoft.DotNet.ApiCompat.targets(52,5): error : Compat issues with assembly System.Net.NetworkInformation: [....\src\libraries\System.Net.NetworkInformation\src\Sy stem.Net.NetworkInformation.csproj] ....\.nuget\packages\microsoft.dotnet.apicompat\7.0.0-beta.22358.3\build\Microsoft.DotNet.ApiCompat.targets(52,5): error : CannotChangeAttribute : Attribute 'System.Runtime.Versioning.UnsupportedOSPlatformAttribute' on 'System.Net.NetworkInformation.NetworkChange .NetworkAddressChanged' changed from '[UnsupportedOSPlatformAttribute("browser")]' in the contract to '[UnsupportedOSPlatformAttribute("illumos")]' in the implementation. [....\src\libraries\System.Net.NetworkInformation\src\System.Net.NetworkInformation .csproj] ....\.nuget\packages\microsoft.dotnet.apicompat\7.0.0-beta.22358.3\build\Microsoft.DotNet.ApiCompat.targets(66,5): error : ApiCompat failed for '....\artifacts\bin\System.Net.NetworkInformation\Debug\net7.0-osx\System.Net.NetworkInformati on.dll' [....\src\libraries\System.Net.NetworkInformation\src\System.Net.NetworkInformation.csproj]

...tem.Net.NetworkInformation/src/System/Net/NetworkInformation/NetworkAddressChange.Browser.cs

radical · 2022-07-12T16:40:01Z

...tem.Net.NetworkInformation/src/System/Net/NetworkInformation/NetworkAddressChange.Browser.cs

+            add => throw new System.PlatformNotSupportedException(System.SR.SystemNetNetworkInformation_PlatformNotSupported);
+            remove => throw new System.PlatformNotSupportedException(System.SR.SystemNetNetworkInformation_PlatformNotSupported);


The corresponding tests should be run for browser, and check for PNSE.

Just curious, are we writing a test for every API that is not supported on any platform?
Because this is already not supported on illumos and solaris. Should I run the test also on these platforms?

...aries/System.Net.NetworkInformation/tests/FunctionalTests/NetworkAvailabilityChangedTests.cs

...tem.Net.NetworkInformation/src/System/Net/NetworkInformation/NetworkAddressChange.Browser.cs

…workInformation/NetworkInterfacePal.Browser.cs Co-authored-by: Ankit Jain <radical@gmail.com>

…ressChanged.

Co-authored-by: Ankit Jain <radical@gmail.com>

src/mono/wasm/runtime/network-information.ts

# Conflicts: # src/libraries/System.Net.NetworkInformation/ref/System.Net.NetworkInformation.cs # src/libraries/apicompat/ApiCompatBaseline.NetCoreAppLatestStable.txt # src/mono/wasm/runtime/exports.ts

maraf · 2022-08-08T12:52:12Z

/azp run runtime-wasm

azure-pipelines · 2022-08-08T12:52:24Z

Azure Pipelines successfully started running 1 pipeline(s).

ghost · 2022-10-07T17:02:41Z

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

ghost · 2022-11-07T11:02:13Z

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

maraf added 3 commits July 11, 2022 14:54

Support NetworkInterface.GetIsNetworkAvailable on browser.

648a4a6

Support NetworkChange.NetworkAvailabilityChanged on browser.

3fbd1d7

Enable tests.

67bb6de

maraf added the arch-wasm WebAssembly architecture label Jul 11, 2022

maraf requested a review from pavelsavara July 11, 2022 13:32

maraf self-assigned this Jul 11, 2022

dotnet-issue-labeler bot added area-System.Net new-api-needs-documentation labels Jul 11, 2022

pavelsavara reviewed Jul 11, 2022

View reviewed changes

src/libraries/System.Net.NetworkInformation/src/System.Net.NetworkInformation.csproj Outdated Show resolved Hide resolved

pavelsavara reviewed Jul 11, 2022

View reviewed changes

...tem.Net.NetworkInformation/src/System/Net/NetworkInformation/NetworkAddressChange.Browser.cs Show resolved Hide resolved

maraf added 4 commits July 12, 2022 09:19

Ensure change listener is attached only once.

4919918

Skip test for NetworkInterface.GetIsNetworkAvailable on NodeJS and V8.

2b8856e

PSNE with error message.

d314fca

UnsupportedOSPlatform for everything not supported on browser.

7eb6f8d

Update ApiCompatBaseline.

2baf83f

maraf marked this pull request as ready for review July 12, 2022 14:45

maraf requested a review from lewing as a code owner July 12, 2022 14:45

radical reviewed Jul 12, 2022

View reviewed changes

...tem.Net.NetworkInformation/src/System/Net/NetworkInformation/NetworkAddressChange.Browser.cs Outdated Show resolved Hide resolved

radical reviewed Jul 12, 2022

View reviewed changes

...tem.Net.NetworkInformation/src/System/Net/NetworkInformation/NetworkAddressChange.Browser.cs Show resolved Hide resolved

radical reviewed Jul 12, 2022

View reviewed changes

...aries/System.Net.NetworkInformation/tests/FunctionalTests/NetworkAvailabilityChangedTests.cs Outdated Show resolved Hide resolved

radical reviewed Jul 12, 2022

View reviewed changes

...tem.Net.NetworkInformation/src/System/Net/NetworkInformation/NetworkAddressChange.Browser.cs Outdated Show resolved Hide resolved

maraf and others added 8 commits July 13, 2022 10:01

Update src/libraries/System.Net.NetworkInformation/src/System/Net/Net…

2ee970e

…workInformation/NetworkInterfacePal.Browser.cs Co-authored-by: Ankit Jain <radical@gmail.com>

Enable NetworkAvailabilityChanged tests that don't include NetworkAdd…

1a1eca2

…ressChanged.

Rename OnNetworkChanged to OnNetworkAvailabilityChanged

947d325

Co-authored-by: Ankit Jain <radical@gmail.com>

Unsubscribe browser events.

2afa020

Enable more tests.

dfa5a6c

Fix assigning the listener.

1d71019

Rename network_wasm_online to network_wasm_is_online.

23c1eb8

Throw when the API is not available.

febd72b

This was referenced Jul 18, 2022

jit.1 work item failing on mono #67888

Closed

Test failure JIT/Performance/CodeQuality/BenchmarksGame/regex-redux/regex-redux-5/regex-redux-5.sh #66625

Closed

pavelsavara reviewed Aug 8, 2022

View reviewed changes

src/mono/wasm/runtime/network-information.ts Outdated Show resolved Hide resolved

maraf added 2 commits August 8, 2022 14:50

Merge remote-tracking branch 'upstream/main' into WasmNetworkInfo

bc03544

# Conflicts: # src/libraries/System.Net.NetworkInformation/ref/System.Net.NetworkInformation.cs # src/libraries/apicompat/ApiCompatBaseline.NetCoreAppLatestStable.txt # src/mono/wasm/runtime/exports.ts

Feedback.

5ccf469

runfoapp bot mentioned this pull request Aug 8, 2022

Methodical_others test JIT/Methodical/Coverage/copy_prop_byref_to_native_int crashing #69832

Open

maraf added this to the 8.0.0 milestone Sep 7, 2022

maraf marked this pull request as draft September 7, 2022 09:07

ghost closed this Oct 7, 2022

maraf reopened this Oct 8, 2022

ghost closed this Nov 7, 2022

ghost locked as resolved and limited conversation to collaborators Dec 7, 2022

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[wasm] Support network status information #71941

[wasm] Support network status information #71941

maraf commented Jul 11, 2022 •

edited

dotnet-issue-labeler bot commented Jul 11, 2022

ghost commented Jul 11, 2022

ghost commented Jul 11, 2022

maraf commented Jul 11, 2022

azure-pipelines bot commented Jul 11, 2022

maraf commented Jul 12, 2022

azure-pipelines bot commented Jul 12, 2022

maraf commented Jul 12, 2022

azure-pipelines bot commented Jul 12, 2022

maraf commented Jul 12, 2022

kg commented Jul 12, 2022

maraf commented Jul 12, 2022 •

edited

radical Jul 12, 2022

maraf Jul 14, 2022 •

edited

radical Jul 12, 2022

maraf Jul 13, 2022

radical Jul 13, 2022

maraf Jul 14, 2022 •

edited

radical Jul 12, 2022

maraf Jul 14, 2022

maraf commented Aug 8, 2022

azure-pipelines bot commented Aug 8, 2022

ghost commented Oct 7, 2022

ghost commented Nov 7, 2022

		add => throw new System.PlatformNotSupportedException(System.SR.SystemNetNetworkInformation_PlatformNotSupported);
		remove => throw new System.PlatformNotSupportedException(System.SR.SystemNetNetworkInformation_PlatformNotSupported);

[wasm] Support network status information #71941

[wasm] Support network status information #71941

Conversation

maraf commented Jul 11, 2022 • edited

dotnet-issue-labeler bot commented Jul 11, 2022

ghost commented Jul 11, 2022

ghost commented Jul 11, 2022

maraf commented Jul 11, 2022

azure-pipelines bot commented Jul 11, 2022

maraf commented Jul 12, 2022

azure-pipelines bot commented Jul 12, 2022

maraf commented Jul 12, 2022

azure-pipelines bot commented Jul 12, 2022

maraf commented Jul 12, 2022

kg commented Jul 12, 2022

maraf commented Jul 12, 2022 • edited

radical Jul 12, 2022

Choose a reason for hiding this comment

maraf Jul 14, 2022 • edited

Choose a reason for hiding this comment

radical Jul 12, 2022

Choose a reason for hiding this comment

maraf Jul 13, 2022

Choose a reason for hiding this comment

radical Jul 13, 2022

Choose a reason for hiding this comment

maraf Jul 14, 2022 • edited

Choose a reason for hiding this comment

radical Jul 12, 2022

Choose a reason for hiding this comment

maraf Jul 14, 2022

Choose a reason for hiding this comment

maraf commented Aug 8, 2022

azure-pipelines bot commented Aug 8, 2022

ghost commented Oct 7, 2022

ghost commented Nov 7, 2022

maraf commented Jul 11, 2022 •

edited

maraf commented Jul 12, 2022 •

edited

maraf Jul 14, 2022 •

edited

maraf Jul 14, 2022 •

edited