Random port retry logic #1692
Random port retry logic #1692
Conversation
This comment has been minimized.
This comment has been minimized.
Codecov Report
@@ Coverage Diff @@
## master #1692 +/- ##
==========================================
- Coverage 84.39% 83.76% -0.63%
==========================================
Files 7 8 +1
Lines 519 536 +17
Branches 159 161 +2
==========================================
+ Hits 438 449 +11
- Misses 64 70 +6
Partials 17 17
Continue to review full report at Codecov.
|
|
/cc @hiroppy |
| // Because NaN == null is false, defaultTo fails if parseInt returns NaN | ||
| // so the tryParseInt function is introduced to handle NaN | ||
| const defaultPortRetry = defaultTo( | ||
| tryParseInt(process.env.DEFAULT_PORT_RETRY), |
There was a problem hiding this comment.
IMO: I think we should leave process.env.DEFAULT_PORT_RETRY as a document.
There was a problem hiding this comment.
@hiroppy can you create issue in webpack docs repo?
There was a problem hiding this comment.
I think DEFAULT_PORT_RETRY should be added in devServer because currently, we don' use process.env. see https://webpack.js.org/configuration/dev-server/
@evilebottnawi what do you think?
There was a problem hiding this comment.
@hiroppy yep, after merge we need open issue here https://github.com/webpack/webpack.js.org 👍
|
Thanks for doing this. I must apologize for putting this aside for too long. Got bombarded with work right after our Chinese New Year holidays, and I totally forgot to put this into my reminders. |
For Bugs and Features; did you add new tests?
Some find port tests were added.
Motivation / Use-Case
Closes #1572.
The main change is to delay port finding to just before trying to run server.listen. Just this change alone reduced the time window between determining the available port and actually listening to it, as previously the compiler is initiated between this time frame. This naturally let most instances attempt to get available port in different moments and reducing the risk of trying to use the same port.
Another change is to listen to error event when port finding is used, when 'EADDRINUSE' error is encountered, retry the port finding again for up to 10 times. Not the most elegant solution, but should be sufficient for most prototyping scenarios.
If retried for 10 times and still cannot find available pot to use, the dev-server will error out like before.
Breaking Changes
Unless there are people relying on the fact that attempting to start multiple instances without specifying port could result in errors, this should not have breaking changes.
Additional Info
This is recreation of #1577 which seems to have any activity. As this is an important feature, this PR presents the original work with some refactoring and also conflict free state after merged against last master. The original commits were cherry picked and most of the credits should be given to the original author of the PR.
A manual test of the feature was also performed.
/CC @evilebottnawi