fix(browser): prevent default entry detection and WS port conflicts#857
fix(browser): prevent default entry detection and WS port conflicts#857
Conversation
✅ Deploy Preview for rstest-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This pull request addresses WebSocket server port conflicts by implementing dynamic port allocation. Instead of using a fixed port calculation (container port + 1), the WebSocket server now uses port: 0 to let the operating system assign an available port automatically, preventing EADDRINUSE errors when the calculated port is already in use.
Key changes:
- Dynamic WebSocket port allocation using
port: 0instead ofport + 1 - Proper async handling with promise-based awaiting of 'listening'/'error' events
- Added AddressInfo type import for proper type handling
Comments suppressed due to low confidence (1)
packages/browser/src/hostController.ts:1255
- The value assigned to containerContext here is unused.
containerContext = runtime.containerContext;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await new Promise<void>((resolve, reject) => { | ||
| wss.once('listening', resolve); | ||
| wss.once('error', reject); | ||
| }); | ||
| const wsPort = (wss.address() as AddressInfo).port; |
There was a problem hiding this comment.
If the WebSocket server fails to start (error event), the devServer won't be properly cleaned up. Consider wrapping this in a try-catch block and calling await devServer.close() before re-throwing the error, similar to how errors are handled in the subsequent try-catch blocks for playwright import and browser launch.
| await new Promise<void>((resolve, reject) => { | |
| wss.once('listening', resolve); | |
| wss.once('error', reject); | |
| }); | |
| const wsPort = (wss.address() as AddressInfo).port; | |
| let wsPort: number; | |
| try { | |
| await new Promise<void>((resolve, reject) => { | |
| wss.once('listening', resolve); | |
| wss.once('error', reject); | |
| }); | |
| wsPort = (wss.address() as AddressInfo).port; | |
| } catch (_error) { | |
| wss.close(); | |
| await devServer.close(); | |
| throw _error; | |
| } |
| wss.once('listening', resolve); | ||
| wss.once('error', reject); | ||
| }); | ||
| const wsPort = (wss.address() as AddressInfo).port; |
There was a problem hiding this comment.
Although unlikely after the 'listening' event, wss.address() could potentially return null in edge cases. Consider adding a null check before accessing the port property to make the code more robust.
| const wsPort = (wss.address() as AddressInfo).port; | |
| const wsAddress = wss.address(); | |
| if (!wsAddress || typeof wsAddress === 'string') { | |
| wss.close(); | |
| await devServer.close(); | |
| throw new Error('[Browser UI] Failed to determine WebSocket server address'); | |
| } | |
| const wsPort = (wsAddress as AddressInfo).port; |
Summary
port: 0) for WebSocket server instead of fixedcontainer port + 1EADDRINUSEerror when the fixed port is already occupiedlistening/errorevent awaitingRelated Links
Checklist