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.

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

[release/8.0][browser] BrowserWebSocket.ReceiveAsync after server initiated close #97002

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Jan 15, 2024

Fixes #96359
main PR for Net9 #96618 , this is not 1-to-1 merge.

Description

Pending incoming WebSocket messages are lost after we receive close message from the server side.

Customer Impact

This is user reported issue #96359

Regression

Probably introduced in Net7

Testing

New automated unit tests in this PR. Existing automated unit tests.

Risk

This is non-trivial change.
The impact is users of .Net on the browser, typically Blazor.

@pavelsavara pavelsavara added arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm labels Jan 15, 2024
@pavelsavara pavelsavara added this to the 8.0.x milestone Jan 15, 2024
@pavelsavara pavelsavara self-assigned this Jan 15, 2024
@ghost
Copy link

ghost commented Jan 15, 2024

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

Issue Details

Fixes #96359
main PR for Net9 #96618

Description

Pending incoming messages are lost after we receive close from server side.

Customer Impact

This is user reported issue #96359

Regression

Probably introduced in Net7

Testing

New automated unit tests in this PR. Existing automated unit tests.

Risk

This is non-trivial change.
The impact is users of .Net on the browser, typically Blazor.

Author: pavelsavara
Assignees: pavelsavara
Labels:

arch-wasm, area-System.Runtime.InteropServices.JavaScript, os-browser

Milestone: 8.0.x

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@pavelsavara pavelsavara marked this pull request as ready for review January 16, 2024 10:52
@pavelsavara pavelsavara requested review from lewing and kg as code owners January 16, 2024 10:52
@carlossanlop
Copy link
Member

Today is Code Complete for the February Release.
@pavelsavara can you please address the feedback?
@kg @maraf @lewing Can one of you please sign-off this backport if all looks good?

@rbhanda rbhanda modified the milestones: 8.0.x, 8.0.3 Jan 18, 2024
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jan 18, 2024
@pavelsavara
Copy link
Member Author

ST Log
MT Log

[22:19:01] info: Finished:    System.Net.WebSockets.Client.Tests.dll
[22:19:01] info: 
[22:19:01] info: === TEST EXECUTION SUMMARY ===
[22:19:01] info: Total: 300, Errors: 0, Failed: 0, Skipped: 0, Time: 501.939665s

@pavelsavara pavelsavara merged commit 5e9e29f into dotnet:release/8.0-staging Jan 23, 2024
116 of 119 checks passed
@pavelsavara pavelsavara deleted the browser_ws_net8_read_after_close_staging branch January 23, 2024 14:50
@github-actions github-actions bot locked and limited conversation to collaborators Feb 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants