fix(proxy): match the HMR upgrade path exactly like the ws server#5678
Conversation
…e configured path Adds trailing slash, case variations, percent-encoded path, and leading double slash to the existing HMR/proxy isolation tests so that all URL shapes recognized as the HMR upgrade are treated consistently.
Tighten the HMR-vs-proxy upgrade dispatch so URL variants of the
configured HMR path are consistently recognized as the HMR socket:
- lowercase comparison (case-insensitive)
- decode percent-encoding before comparison
- strip trailing slashes
- collapse leading multiple slashes so //foo is treated as /foo
instead of being parsed as a scheme-relative URL
- wrap URL parsing in try/catch so a malformed req.url does not
raise an uncaught exception
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5678 +/- ##
=======================================
Coverage 83.60% 83.61%
=======================================
Files 13 13
Lines 2086 2087 +1
Branches 773 774 +1
=======================================
+ Hits 1744 1745 +1
Misses 306 306
Partials 36 36 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
/cc @bjohansebas can you take a look, thanks |
|
Okay, I removed the normalization/decoding step so that it follows the ws implementation and avoids any unexpected edge cases. I'm also going to add tests for SockJS since it's still used in this version. It can be removed in the next major release. |
bjohansebas
changed the title
There was a problem hiding this comment.
Pull request overview
Adds a more faithful HMR WebSocket upgrade-path check when deciding whether to dispatch an upgrade event to user-defined WebSocket proxies, aligning the proxy “skip” behavior with how the ws server matches paths.
Changes:
- Updated
Serverupgrade handling to compare the raw request target (with query stripped) against the configured HMR path. - Added a comprehensive regression test suite ensuring HMR upgrades are served locally while non-HMR upgrades are forwarded to user proxies (including
ws,sockjs, custom paths, and disabled WS server cases).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
lib/Server.js |
Adjusts HMR upgrade-path matching to avoid URL normalization and match ws path handling semantics. |
test/server/proxy-option.test.js |
Adds new tests covering HMR vs non-HMR upgrade dispatch behavior across WS server types and configurations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Interesting, Node 24 doesn't work because of Chrome-related issues. |
No description provided.