feat: support reusePort on server listen #115
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,7 @@ class Master extends EventEmitter { | |
| * - {Object} [plugins] - customized plugins, for unittest | ||
| * - {Number} [workers] numbers of app workers, default to `os.cpus().length` | ||
| * - {Number} [port] listening port, default to 7001(http) or 8443(https) | ||
| * - {Boolean} [reusePort] setting `reusePort` to `true` allows multiple sockets on the same host to bind to the same port. Incoming connections are distributed by the operating system to listening sockets. This option is available only on some platforms, such as Linux 3.9+, DragonFlyBSD 3.6+, FreeBSD 12.0+, Solaris 11.4, and AIX 7.2.5+. **Default:** `false`. | ||
| * - {Number} [debugPort] listening a debug port on http protocol | ||
| * - {Object} [https] https options, { key, cert, ca }, full path | ||
| * - {Array|String} [require] will inject into worker/agent process | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ module.exports = function(options) { | |
| framework: '', | ||
| baseDir: process.cwd(), | ||
| port: options.https ? 8443 : null, | ||
| reusePort: false, | ||
| workers: null, | ||
| plugins: null, | ||
| https: false, | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -232,6 +232,35 @@ | |||||
| .expect(200); | ||||||
| }); | ||||||
|
|
||||||
| it.only('should set reusePort=true in config', async () => { | ||||||
|
Check warning on line 235 in test/app_worker.test.js
|
||||||
|
There was a problem hiding this comment. Remove .only from the test case. The use of Apply this diff to fix the issue: - it.only('should set reusePort=true in config', async () => {
+ it('should set reusePort=true in config', async () => {📝 Committable suggestion
Suggested change
🧰 Tools🪛 GitHub Check: Node.js / Test (ubuntu-latest, 23)
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 22)
🪛 GitHub Check: Node.js / Test (macos-latest, 23)
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 20)
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 18)
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 16)
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 14)
🪛 GitHub Check: Node.js / Test (macos-latest, 22)
🪛 GitHub Check: Node.js / Test (macos-latest, 20)
🪛 GitHub Check: Node.js / Test (macos-latest, 16)
🪛 GitHub Check: Node.js / Test (macos-latest, 14)
🪛 GitHub Check: Node.js / Test (macos-latest, 18)
🪛 Biome
|
||||||
| app = utils.cluster('apps/app-listen-reusePort'); | ||||||
| // app.debug(); | ||||||
| await app.ready(); | ||||||
|
|
||||||
| app.expect('code', 0); | ||||||
| app.expect('stdout', /egg started on http:\/\/127.0.0.1:17010/); | ||||||
|
|
||||||
| await request('http://0.0.0.0:17010') | ||||||
| .get('/') | ||||||
| .expect('done') | ||||||
| .expect(200); | ||||||
|
|
||||||
| await request('http://127.0.0.1:17010') | ||||||
| .get('/') | ||||||
| .expect('done') | ||||||
| .expect(200); | ||||||
|
|
||||||
| await request('http://localhost:17010') | ||||||
| .get('/') | ||||||
| .expect('done') | ||||||
| .expect(200); | ||||||
|
|
||||||
| await request('http://127.0.0.1:17010') | ||||||
| .get('/port') | ||||||
| .expect('17010') | ||||||
| .expect(200); | ||||||
| }); | ||||||
|
Comment on lines
+235
to
+262
There was a problem hiding this comment. 🛠️ Refactor suggestion Enhance test coverage for reusePort functionality. While the test verifies basic HTTP functionality, it doesn't specifically verify that the SO_REUSEPORT option is properly set. Consider adding assertions to validate the socket option. Here's a suggested enhancement to verify reusePort is actually set: it('should set reusePort=true in config', async () => {
app = utils.cluster('apps/app-listen-reusePort');
- // app.debug();
await app.ready();
app.expect('code', 0);
app.expect('stdout', /egg started on http:\/\/127.0.0.1:17010/);
+ // Verify reusePort is set in the worker process
+ await app.httpRequest()
+ .get('/get-socket-options')
+ .expect(res => {
+ assert(res.body.reusePort === true, 'Expected SO_REUSEPORT to be enabled');
+ })
+ .expect(200);
await request('http://0.0.0.0:17010')
.get('/')This requires adding a new endpoint in your test fixture that exposes the socket options. I can help implement this if needed. Would you like me to provide the implementation for the socket options endpoint in the test fixture?
🧰 Tools🪛 GitHub Check: Node.js / Test (ubuntu-latest, 23)
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 22)
🪛 GitHub Check: Node.js / Test (macos-latest, 23)
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 20)
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 18)
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 16)
🪛 GitHub Check: Node.js / Test (ubuntu-latest, 14)
🪛 GitHub Check: Node.js / Test (macos-latest, 22)
🪛 GitHub Check: Node.js / Test (macos-latest, 20)
🪛 GitHub Check: Node.js / Test (macos-latest, 16)
🪛 GitHub Check: Node.js / Test (macos-latest, 14)
🪛 GitHub Check: Node.js / Test (macos-latest, 18)
🪛 Biome
|
||||||
|
|
||||||
| it('should use hostname in config', async () => { | ||||||
| const url = address.ip() + ':17010'; | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| 'use strict'; | ||
|
|
||
| module.exports = app => { | ||
| // don't use the port that egg-mock defined | ||
| app._options.port = undefined; | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| module.exports = app => { | ||
| app.get('/', ctx => { | ||
| ctx.body = 'done'; | ||
| }); | ||
|
|
||
| app.get('/port', ctx => { | ||
| ctx.body = ctx.app._options.port; | ||
| }); | ||
| }; |
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -0,0 +1,11 @@ | ||||
| 'use strict'; | ||||
|
|
||||
|
Comment on lines
+1
to
+2
There was a problem hiding this comment. Remove redundant 'use strict' directive. The 'use strict' directive is unnecessary in ES modules as they are automatically in strict mode. Apply this diff: -'use strict';
-📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome
|
||||
| module.exports = { | ||||
| keys: '123', | ||||
| cluster: { | ||||
| listen: { | ||||
| port: 17010, | ||||
| reusePort: true, | ||||
| }, | ||||
| }, | ||||
| }; | ||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| { | ||
| "name": "app-listen-reusePort" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,17 @@ describe('test/master.test.js', () => { | |
| .end(done); | ||
| }); | ||
|
|
||
| it('start success with reusePort=true', done => { | ||
| mm.env('local'); | ||
| app = utils.cluster('apps/master-worker-started', { reusePort: true }); | ||
|
|
||
| app.expect('stdout', /egg start/) | ||
| .expect('stdout', /egg started/) | ||
| .notExpect('stdout', /\[master\] agent_worker#1:\d+ start with clusterPort:\d+/) | ||
| .expect('code', 0) | ||
| .end(done); | ||
| }); | ||
|
|
||
| it('start success in prod env', done => { | ||
| mm.env('prod'); | ||
| app = utils.cluster('apps/mock-production-app').debug(false); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve code clarity by avoiding assignment in expression.
The current initialization pattern can make the code harder to read. Consider making the assignments more explicit.
📝 Committable suggestion
🧰 Tools
🪛 Biome