Streamline web socket config, KestrelWebSocketServer impl#53108
Open
tmat wants to merge 3 commits intodotnet:release/10.0.3xxfrom
Open
Streamline web socket config, KestrelWebSocketServer impl#53108tmat wants to merge 3 commits intodotnet:release/10.0.3xxfrom
tmat wants to merge 3 commits intodotnet:release/10.0.3xxfrom
Conversation
- Added DOTNET_WATCH_AUTO_RELOAD_WSS_PORT environment variable - Added AutoReloadWebSocketSecurePort property to EnvironmentOptions - Updated BrowserRefreshServer to use separate ports for HTTP and HTTPS - This allows users to avoid port conflicts when TLS is supported When TLS is available, users can now set: - DOTNET_WATCH_AUTO_RELOAD_WS_PORT for HTTP (e.g., 8081) - DOTNET_WATCH_AUTO_RELOAD_WSS_PORT for HTTPS (e.g., 8082) Co-authored-by: dbreshears <3432571+dbreshears@users.noreply.github.com>
Renamed properties and variables as requested: - AutoReloadWebSocketHostName → BrowserWebSocketHostName - AutoReloadWebSocketPort → BrowserWebSocketPort - AutoReloadWebSocketSecurePort → BrowserWebSocketSecurePort Updated in: - EnvironmentOptions.cs (property definitions and FromEnvironment) - BrowserRefreshServer.cs (constructor parameters and local variables) - WebApplicationAppModel.cs (property usage) Co-authored-by: tmat <41759+tmat@users.noreply.github.com>
8 tasks
tmat
commented
Feb 21, 2026
| env[AgentEnvironmentVariables.DotNetWatchHotReloadWebSocketKey] = _handler.SharedSecretProvider.GetPublicKey(); | ||
| } | ||
|
|
||
| private async Task HandleRequestAsync(HttpContext context) |
Member
Author
There was a problem hiding this comment.
Moved to RequestHandler type to reduce mutable state.
Member
Author
|
@jonathanpeppers ptal |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical bug where dotnet watch fails when DOTNET_WATCH_AUTO_RELOAD_WS_PORT is set and TLS is supported, by implementing separate port configuration for HTTP and HTTPS WebSocket endpoints.
Changes:
- Introduced
WebSocketConfigstruct to encapsulate WebSocket server configuration with separate HTTP and HTTPS ports - Added support for
DOTNET_WATCH_AUTO_RELOAD_WSS_PORTenvironment variable to specify the secure WebSocket port independently - Refactored
KestrelWebSocketServerto use the new configuration structure and return WebSocket URLs directly - Refactored
WebSocketClientTransportto use a nestedRequestHandlerclass for better separation of concerns
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/BuiltInTools/HotReloadClient/Web/WebSocketConfig.cs | New struct encapsulating WebSocket configuration with separate HTTP and HTTPS ports to prevent binding conflicts |
| src/BuiltInTools/HotReloadClient/Web/KestrelWebSocketServer.cs | Refactored to use WebSocketConfig, simplified URL conversion, and made StartServerAsync static factory method |
| src/BuiltInTools/HotReloadClient/Web/BrowserRefreshServer.cs | Updated to use WebSocketConfig and disable secure port when TLS is not supported |
| src/BuiltInTools/Watch/Context/EnvironmentVariables.cs | Added BrowserWebSocketSecurePort property and renamed properties for clarity |
| src/BuiltInTools/Watch/Context/EnvironmentOptions.cs | Replaced individual WebSocket parameters with WebSocketConfig instances for browser and agent |
| src/BuiltInTools/Watch/AppModels/WebApplicationAppModel.cs | Updated to pass WebSocketConfig instead of individual parameters |
| src/BuiltInTools/Watch/AppModels/MobileAppModel.cs | Updated to pass WebSocketConfig to CreateAsync method |
| src/BuiltInTools/HotReloadClient/WebSocketClientTransport.cs | Refactored with nested RequestHandler class and updated to use WebSocketConfig |
| test/dotnet-watch.Tests/Web/KestrelWebSocketServerTests.cs | Added tests for GetWebSocketUrl method covering various URL transformations |
Comment on lines
39
to
45
| @@ -83,68 +43,71 @@ await process | |||
| /// <param name="port">HTTP port to bind to (0 for auto-assign)</param> | |||
| /// <param name="securePort">HTTPS port to bind to in addition to HTTP port. Null to skip HTTPS.</param> | |||
| /// <param name="cancellationToken">Cancellation token</param> | |||
There was a problem hiding this comment.
The XML documentation parameters do not match the actual method signature. The method now takes WebSocketConfig config and RequestDelegate requestHandler parameters instead of hostName, port, and securePort. Update the parameter documentation to reflect the actual parameters.
Comment on lines
+27
to
+39
| public IEnumerable<string> GetHttpUrls() | ||
| { | ||
| yield return $"http://{HostName}:{Port}"; | ||
|
|
||
| if (SecurePort.HasValue) | ||
| { | ||
| yield return $"https://{HostName}:{SecurePort.Value}"; | ||
| } | ||
| } | ||
|
|
||
| public WebSocketConfig WithSecurePort(int? value) | ||
| => new(port, value, hostName); | ||
| } |
There was a problem hiding this comment.
The new WebSocketConfig struct lacks test coverage for its public methods GetHttpUrls() and WithSecurePort(). Consider adding unit tests to verify:
- GetHttpUrls() correctly yields HTTP URL when SecurePort is null
- GetHttpUrls() correctly yields both HTTP and HTTPS URLs when SecurePort has a value
- GetHttpUrls() correctly uses the provided hostName or defaults to "127.0.0.1"
- WithSecurePort() correctly creates a new config with the updated SecurePort value while preserving other properties
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
DOTNET_WATCH_AUTO_RELOAD_WSS_PORTto control browser refresh wss port.Wraps port and host web socket info to
WebSocketConfiguration.Reduces mutable state of Kestrel server and WebSocketClientTransport.
Fixes #52908