Skip to content

Commit 2d168f8

Browse files
authored
feat(cluster): make the separate HTTP debug server opt-in (#6073)
Configuring debugPort for a plain HTTP application calls listen() twice on the same HTTP server, preventing both configured endpoints from becoming usable. Add config.cluster.separateDebugServer as an opt-in to create a separate HTTP debug listener. The option defaults to false, preserving existing HTTP behavior as requested in review. HTTPS already uses a separate HTTP server; this option leaves that server selection unchanged. Document the option and add its EggAppConfig type. Tests cover omitted/false/true configuration, with and without debugPort, plus the framework's false default. Real cluster tests cover HTTP with default and explicit hostnames and HTTPS; both endpoints respond and both ports are released after shutdown. Validation on Windows, Node.js 22.23.2: - Focused cluster tests: debug-port.test.ts, worker-protocol.test.ts, and https.test.ts — 26 passed. - Framework configuration tests — 2 passed. - Restoring unconditional separate-server creation causes the two default/false compatibility regressions to fail, with the remaining 18 protocol tests passing. - Cluster tsc --noEmit passed. Final touched-file oxlint --type-aware --type-check passed with three pre-existing redundant-union warnings in packages/egg/src/lib/types.ts. All changed files were formatted with oxfmt; git diff --check passed. The runs use the package Vitest entry points and original configurations with the existing focused dependency environment; this is not a full monorepo installation or suite. The first sandbox-user attempts failed before applications started because os.userInfo() returned uv_os_get_passwd ENOMEM; the same tests passed under the normal Windows user. Two final test typing annotations were checked by type-aware lint after the successful runtime runs. No coverage percentage is claimed; the updated CI/Codecov result is still required. AI-assisted follow-up implementation and validation summary (GPT-6). Follow-up validation for `8ab7b0b`: independent HTTP and HTTPS debug listeners now route bind errors through the existing startup log/worker-exit handler. The default shared HTTP listener retains exactly one handler. Native occupied-port regressions live in a separate file from the mocked protocol tests. On Windows with Node.js 24.15.0 and the existing focused dependencies, all 28 tests in worker-protocol, debug-port, https and debug-port-error pass. Restoring only the preceding production source makes both occupied-port regressions fail with real EADDRINUSE errors; the final source was restored byte-for-byte. Cluster typecheck and final three-file type-aware lint pass with no warnings. The earlier two configuration tests are unchanged and were not repeated for this handler-only follow-up. This remains focused local validation; updated upstream CI/Codecov is pending. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * HTTP applications can use a separate server for the debug port by enabling `separateDebugServer`. The setting defaults to `false`, so the debug port uses the application server unless configured otherwise. * **Bug Fixes** * Errors from the application server and any separate debug server are now logged and trigger process exit. * **Documentation** * Documented the setting and its behavior for HTTP and HTTPS applications. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 7f69524 commit 2d168f8

11 files changed

Lines changed: 253 additions & 4 deletions

File tree

‎packages/cluster/README.md‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,25 @@ startCluster({
7171
| ports | `Array` | startup port of each app worker, such as: [7001, 7002, 7003], only effects when the startMode is 'worker_threads' |
7272
| env | `String` | custom env, default is process.env.EGG_SERVER_ENV |
7373

74+
### Separate HTTP Debug Server
75+
76+
For an HTTP application, set `config.cluster.separateDebugServer` to `true` to
77+
serve the configured `debugPort` on a separate HTTP server:
78+
79+
```js
80+
// config/config.default.js
81+
exports.cluster = {
82+
separateDebugServer: true,
83+
};
84+
```
85+
86+
Pass `port` and `debugPort` to `startCluster` as usual. Both ports serve the same
87+
application callback. The option defaults to `false`, preserving the existing
88+
HTTP behavior of reusing the application server for `debugPort`. Enable it when
89+
an HTTP application needs both listeners, since a Node.js server cannot listen
90+
on two ports at once. HTTPS applications already use a separate HTTP server for
91+
`debugPort` and are unaffected by this option.
92+
7493
## Env
7594

7695
`EGG_APP_CLOSE_TIMEOUT`: app worker boot timeout value

‎packages/cluster/src/worker_protocol/app.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,14 +132,18 @@ export function startAppWorker(
132132
} else {
133133
server = createHttpServer(app.callback());
134134
if (debugPort) {
135-
debugPortServer = server;
135+
debugPortServer = clusterConfig.separateDebugServer ? createHttpServer(app.callback()) : server;
136136
}
137137
}
138138

139-
server.once('error', (err: any) => {
139+
const serverErrorHandler = (err: Error & { code?: string }) => {
140140
consoleLogger.error('[app_worker] server got error: %s, code: %s', err.message, err.code);
141141
exitProcess();
142-
});
142+
};
143+
server.once('error', serverErrorHandler);
144+
if (debugPortServer && debugPortServer !== server) {
145+
debugPortServer.once('error', serverErrorHandler);
146+
}
143147

144148
// emit `server` event in app
145149
app.emit('server', server);
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import { strict as assert } from 'node:assert';
2+
import { EventEmitter, once } from 'node:events';
3+
import { createServer, type Server } from 'node:http';
4+
import { fileURLToPath } from 'node:url';
5+
6+
import { describe, it, vi } from 'vitest';
7+
8+
import { startAppWorker } from '../src/worker_protocol/app.ts';
9+
10+
describe('test/debug-port-error.test.ts', () => {
11+
it.each(['http', 'https'])('handles an occupied separate debug port for %s', async (protocol) => {
12+
const blocker = createServer();
13+
let server: Server | undefined;
14+
let timeout: ReturnType<typeof setTimeout> | undefined;
15+
try {
16+
// Match debugPortServer.listen(debugPort), including its default address family.
17+
blocker.listen(0);
18+
await once(blocker, 'listening');
19+
const address = blocker.address();
20+
assert.ok(address && typeof address !== 'string');
21+
22+
const app = new EventEmitter() as any;
23+
app.config = { cluster: { listen: { port: 0 }, separateDebugServer: true } };
24+
app.options = {};
25+
app.ready = (callback: (err?: Error) => void) => callback();
26+
app.callback = () => (_request: unknown, response: { end(): void }) => response.end();
27+
app.close = () => {};
28+
app.once('server', (value: Server) => {
29+
server = value;
30+
});
31+
let resolveKilled: (() => void) | undefined;
32+
const killed = new Promise<void>((resolve, reject) => {
33+
resolveKilled = resolve;
34+
timeout = setTimeout(() => reject(new Error('Debug-port bind failure did not request worker exit')), 2000);
35+
});
36+
const kill = vi.fn(() => resolveKilled!());
37+
const logger = { error: vi.fn() } as any;
38+
startAppWorker(
39+
app,
40+
{
41+
debugPort: address.port,
42+
...(protocol === 'https'
43+
? {
44+
https: {
45+
key: fileURLToPath(new URL('./fixtures/server.key', import.meta.url)),
46+
cert: fileURLToPath(new URL('./fixtures/server.cert', import.meta.url)),
47+
},
48+
}
49+
: {}),
50+
},
51+
{ workerId: 1, send() {}, kill, gracefulExit() {}, on() {} },
52+
logger,
53+
);
54+
await killed;
55+
56+
assert.ok(server);
57+
assert.equal(server.listenerCount('error'), 1);
58+
assert.equal(kill.mock.calls.length, 1);
59+
assert.equal(logger.error.mock.calls.length, 1);
60+
assert.equal(logger.error.mock.calls[0][0], '[app_worker] server got error: %s, code: %s');
61+
assert.match(logger.error.mock.calls[0][1], /EADDRINUSE/);
62+
assert.equal(logger.error.mock.calls[0][2], 'EADDRINUSE');
63+
} finally {
64+
clearTimeout(timeout);
65+
await Promise.all(
66+
[server, blocker]
67+
.filter((value): value is Server => value !== undefined)
68+
.map((value) => new Promise<void>((resolve) => value.close(() => resolve()))),
69+
);
70+
}
71+
});
72+
});
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import { strict as assert } from 'node:assert';
2+
import { createConnection } from 'node:net';
3+
4+
import { mm } from '@eggjs/mock';
5+
import { detectPort } from 'detect-port';
6+
import { HttpClient } from 'urllib';
7+
import { afterEach, describe, it } from 'vitest';
8+
9+
import { cluster, getFilepath } from './utils.ts';
10+
11+
const httpclient = new HttpClient({ connect: { rejectUnauthorized: false } });
12+
13+
function isPortOpen(port: number): Promise<boolean> {
14+
return new Promise((resolve, reject) => {
15+
const socket = createConnection({ port, host: '127.0.0.1' });
16+
socket.once('connect', () => {
17+
socket.destroy();
18+
resolve(true);
19+
});
20+
socket.once('error', (err: NodeJS.ErrnoException) => {
21+
if (err.code === 'ECONNREFUSED') {
22+
resolve(false);
23+
} else {
24+
reject(err);
25+
}
26+
});
27+
socket.setTimeout(2000, () => socket.destroy(new Error('Timed out checking the closed listener')));
28+
});
29+
}
30+
31+
describe('test/debug-port.test.ts', () => {
32+
let app: ReturnType<typeof cluster> | undefined;
33+
34+
afterEach(async () => {
35+
try {
36+
await app?.close();
37+
} finally {
38+
app = undefined;
39+
await mm.restore();
40+
}
41+
});
42+
43+
it.each([
44+
{ name: 'HTTP without a hostname', protocol: 'http', hostname: '' },
45+
{ name: 'HTTP with an explicit hostname', protocol: 'http', hostname: '127.0.0.1' },
46+
{ name: 'HTTPS', protocol: 'https', hostname: '' },
47+
])('serves both ports and closes them for $name', async ({ protocol, hostname }) => {
48+
const port = await detectPort();
49+
const debugPort = await detectPort();
50+
assert.notEqual(port, debugPort);
51+
mm(process.env, 'EGG_TEST_CLUSTER_DEBUG_HOSTNAME', hostname);
52+
mm(process.env, 'EGG_TEST_CLUSTER_SEPARATE_DEBUG_SERVER', String(protocol === 'http'));
53+
54+
const options = {
55+
workers: 1,
56+
port,
57+
debugPort,
58+
...(protocol === 'https'
59+
? {
60+
https: {
61+
key: getFilepath('server.key'),
62+
cert: getFilepath('server.cert'),
63+
ca: getFilepath('server.ca'),
64+
},
65+
}
66+
: {}),
67+
};
68+
app = cluster('apps/http-debug-server', options);
69+
await app.ready();
70+
71+
const primary = await httpclient.request(`${protocol}://127.0.0.1:${port}`, { dataType: 'text' });
72+
const debug = await httpclient.request(`http://127.0.0.1:${debugPort}`, { dataType: 'text' });
73+
assert.equal(primary.status, 200);
74+
assert.equal(debug.status, 200);
75+
assert.equal(primary.data, 'debug-port server');
76+
assert.equal(debug.data, primary.data);
77+
78+
await app.close();
79+
app = undefined;
80+
assert.equal(await isPortOpen(port), false);
81+
assert.equal(await isPortOpen(debugPort), false);
82+
});
83+
});
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
module.exports = (app) => {
2+
app.get('/', (ctx) => {
3+
ctx.body = 'debug-port server';
4+
});
5+
};
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
'use strict';
2+
3+
exports.keys = 'debug-port-test';
4+
exports.cluster = {
5+
separateDebugServer: process.env.EGG_TEST_CLUSTER_SEPARATE_DEBUG_SERVER === 'true',
6+
listen: {
7+
hostname: process.env.EGG_TEST_CLUSTER_DEBUG_HOSTNAME || undefined,
8+
},
9+
};
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"name": "http-debug-server"
3+
}

‎packages/cluster/test/worker-protocol.test.ts‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type { Server } from 'node:http';
44
import os from 'node:os';
55
import { Worker } from 'node:worker_threads';
66

7-
import { describe, it, vi } from 'vitest';
7+
import { describe, it, vi, type MockInstance } from 'vitest';
88

99
import { startAgentWorker } from '../src/worker_protocol/agent.ts';
1010
import { startAppWorker } from '../src/worker_protocol/app.ts';
@@ -35,6 +35,50 @@ vi.mock('node:http', async (importOriginal) => {
3535
});
3636

3737
describe('test/worker-protocol.test.ts', () => {
38+
describe.each([undefined, false, true])('separateDebugServer: %s', (separateDebugServer) => {
39+
it.each([undefined, 7002])('preserves the HTTP listener behavior with debugPort: %s', async (debugPort) => {
40+
const app = new EventEmitter() as any;
41+
app.config = {
42+
cluster: {
43+
listen: { port: 7001 },
44+
...(separateDebugServer === undefined ? {} : { separateDebugServer }),
45+
},
46+
};
47+
app.options = {};
48+
app.ready = (callback: (err?: Error) => void) => callback();
49+
app.callback = vi.fn(() => (_request: unknown, response: { end(): void }) => response.end());
50+
app.close = () => {};
51+
52+
let server: Server | undefined;
53+
let listen: MockInstance<Server['listen']> | undefined;
54+
app.once('server', (value: Server) => {
55+
server = value;
56+
listen = vi.spyOn(value, 'listen');
57+
});
58+
startAppWorker(
59+
app,
60+
{ debugPort },
61+
{
62+
workerId: 1,
63+
send() {},
64+
kill() {
65+
assert.fail('HTTP app should not be killed');
66+
},
67+
gracefulExit() {},
68+
on() {},
69+
},
70+
);
71+
await Promise.resolve();
72+
73+
assert.ok(server);
74+
assert.ok(listen);
75+
assert.equal(server.listenerCount('error'), 1);
76+
assert.deepEqual(listen.mock.calls, debugPort && !separateDebugServer ? [[7001], [debugPort]] : [[7001]]);
77+
assert.equal(app.callback.mock.calls.length, debugPort && separateDebugServer ? 2 : 1);
78+
server.close();
79+
});
80+
});
81+
3882
it('uses parentPort without terminating the main process', async () => {
3983
const worker = new Worker(new URL('./fixtures/worker-thread-io.mjs', import.meta.url));
4084
const workerId = worker.threadId;

‎packages/egg/src/config/config.default.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,7 @@ const factory: EggConfigFactory = defineConfigFactory((appInfo): PartialEggConfi
355355
/**
356356
* The options of cluster
357357
* @member {Object} Config#cluster
358+
* @property {Boolean} separateDebugServer - use a separate HTTP server for debugPort on HTTP applications, default is `false`.
358359
* @property {Object} listen - listen options, see {@link https://nodejs.org/api/http.html#http_server_listen_port_hostname_backlog_callback}
359360
* @property {String} listen.path - set a unix sock path when server listen
360361
* @property {Number} listen.port - set a port when server listen
@@ -363,6 +364,7 @@ const factory: EggConfigFactory = defineConfigFactory((appInfo): PartialEggConfi
363364
* Only available on Linux 3.9+, DragonFlyBSD 3.6+, FreeBSD 12.0+, Solaris 11.4, and AIX 7.2.5+.
364365
*/
365366
config.cluster = {
367+
separateDebugServer: false,
366368
listen: {
367369
path: '',
368370
port: 7001,

‎packages/egg/src/lib/types.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,8 @@ export interface EggAppConfig extends EggCoreAppConfig {
278278
serverTimeout: number | null;
279279

280280
cluster: {
281+
/** Use a separate HTTP server for debugPort on HTTP applications, defaults to `false`. */
282+
separateDebugServer?: boolean;
281283
listen: {
282284
path: string;
283285
port: number;

0 commit comments

Comments
 (0)