Skip to content

Conversation

ilonatommy
Copy link
Member

@ilonatommy ilonatommy commented Mar 7, 2024

Follow up for #97002.

  1. Add tests for receiving messages after closing the socket in 2 situations:
  • C# and JS synchronized the websocket states after close and C# knows server closed it (client checked the WS state) -> throw exception.
  • Client did not check the WS state, JS got the "onClose" event but it did not get propagated to C# -> receive the buffered messages.
  1. Change logic of closing:
  • We used to inform C# at once about receiving "onClose" event in JS (setting "closeReceived = true" and changing the state). This lead to a situation where we could get "receive" request from the client when WS was already in "closeReceived" state and we were not leaving a possibility to receive buffered messages.
  • Now we let the "receive" event to arrive and leave a promise in JS even when WS got already closed.

@ilonatommy ilonatommy added arch-wasm WebAssembly architecture area-VM-threading-mono labels Mar 7, 2024
@ilonatommy ilonatommy self-assigned this Mar 7, 2024
@pavelsavara pavelsavara added area-System.Net.Http os-browser Browser variant of arch-wasm and removed area-VM-threading-mono labels Mar 7, 2024
Copy link
Contributor

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

@ilonatommy ilonatommy marked this pull request as ready for review March 8, 2024 13:02
{
Assert.True(false, $"Unexpected exception: {e}");
}
Assert.True(false, "Expected WebSocketException not thrown.");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to Assert.Fail when CI passes.

@carlossanlop
Copy link
Contributor

carlossanlop commented Mar 11, 2024

Today is code complete for the April Release. Please get a Tactics approval and merge this before 4pm PT, otherwise it will have to wait until the May Release.

@pavelsavara
Copy link
Member

Today is code complete for the April Release. Please get a Tactics approval and merge this before 4pm PT, otherwise it will have to wait until the May Release.

We are not ready to rush it in.

@ilonatommy
Copy link
Member Author

Closing in favor of #99673.

@ilonatommy ilonatommy closed this Mar 13, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Apr 13, 2024
@karelz karelz added this to the 8.0.x milestone Jun 24, 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.Net.Http os-browser Browser variant of arch-wasm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants