Skip to content

Set SignalR WebSocketFactory for all environments#65359

Open
BekAllaev wants to merge 1 commit intodotnet:mainfrom
BekAllaev:browser-websocketfactory-configuration
Open

Set SignalR WebSocketFactory for all environments#65359
BekAllaev wants to merge 1 commit intodotnet:mainfrom
BekAllaev:browser-websocketfactory-configuration

Conversation

@BekAllaev
Copy link

@BekAllaev BekAllaev commented Feb 8, 2026

Set SignalR WebSocketFactory for all environments

This pull request makes a minor adjustment to the handling of the WebSocketFactory property in the HttpConnectionFactory class. The change ensures that WebSocketFactory is always set in the shallow copy of HttpConnectionOptions, regardless of the operating system.

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Summary of the changes:
Always assign WebSocketFactory when creating a shallow copy of HttpConnectionOptions, not only on non-browser platforms. Previously it was only assigned inside the !OperatingSystem.IsBrowser() block.

Description

Fixes #63742

Previously, WebSocketFactory was only assigned for non-browser environments. This change moves the assignment outside the browser check, ensuring WebSocketFactory is consistently set regardless of the environment.
Copilot AI review requested due to automatic review settings February 8, 2026 12:17
@BekAllaev BekAllaev requested a review from halter73 as a code owner February 8, 2026 12:17
@github-actions github-actions bot added the area-signalr Includes: SignalR clients and servers label Feb 8, 2026
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Feb 8, 2026
@dotnet-policy-service
Copy link
Contributor

Thanks for your PR, @@BekAllaev. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the SignalR C# client’s HttpConnectionFactory shallow-copy logic so HttpConnectionOptions.WebSocketFactory is preserved for browser environments as well, instead of being dropped behind a !OperatingSystem.IsBrowser() guard.

Changes:

  • Always assign WebSocketFactory when shallow-copying HttpConnectionOptions.
  • Remove the browser-only omission of WebSocketFactory from the non-browser conditional block.

Comment on lines 90 to +93
ApplicationMaxBufferSize = options.ApplicationMaxBufferSize,
TransportMaxBufferSize = options.TransportMaxBufferSize,
UseStatefulReconnect = options.UseStatefulReconnect,
WebSocketFactory = options.WebSocketFactory,
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

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

The new behavior (copying WebSocketFactory even when OperatingSystem.IsBrowser() is true) isn’t covered by existing tests. Current unit tests for ShallowCopyHttpConnectionOptions only run on non-browser runtimes and don’t validate the browser-specific path, so a regression here could go unnoticed. Consider adding a test that runs under a browser/WASM target (or another automated browser-runtime test) to assert that WebSocketFactory flows through the shallow copy on browser platforms.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-signalr Includes: SignalR clients and servers community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable WebSocketFactory Configuration for Browser Platforms in SignalR

1 participant