scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-10-11 03:38:07 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

fix(test): prevent Docker proxy port reuse and retry timing races

Browse source 
Reserve the upstream port while choosing and starting the proxy so it cannot forward requests to itself. Start delayed upstream binding after observing ECONNREFUSED, and require the POST test to exercise a real retry.

Verified all 59 Docker integration tests, forced port reuse, five slow-start cases, ESLint and Prettier.
This commit is contained in:
Gergő Magyar 2026-10-04 04:45:55 +00:00 • committed by GitHub
parent 1915460ae7
commit c3437c2bf7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
1 changed files with 25 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

45
docker-server.test.mjs
View file

@ -331,8 +331,8 @@ const respondOk = (_req, res) => {
// //
// upstream request handler, replaceable mid-test via `ctx.handler`; // upstream request handler, replaceable mid-test via `ctx.handler`;
// null points the proxy at a port nothing ever listens on // null points the proxy at a port nothing ever listens on
// listenAfterMs bind the upstream this late, so the first attempt(s) hit // listenAfterMs bind the upstream this late after the first refused attempt
// ECONNREFUSED (a single-instance restart window) // (a single-instance restart window)
// schemeless drop http:// from GITNEXUS_UPSTREAM_URL, the way Render's // schemeless drop http:// from GITNEXUS_UPSTREAM_URL, the way Render's
// `fromService: { property: hostport }` yields it // `fromService: { property: hostport }` yields it
// env extra environment for docker-server.mjs // env extra environment for docker-server.mjs
@ -366,18 +366,15 @@ async function withProxy(
}) })
: null; : null;
// A late (or never) bind needs its port reserved up front; otherwise let the // Keep the upstream port bound until the proxy port is chosen. Releasing it
// OS assign one at listen time. // sooner lets the OS assign both services the same port and proxy to itself.
const upstreamPort = const reservation = server ?? createServer();
server && listenAfterMs === 0 const upstreamPort = await new Promise((r) =>
? await new Promise((r) => server.listen(0, '127.0.0.1', () => r(server.address().port))) reservation.listen(0, '127.0.0.1', () => r(reservation.address().port)),
: await getFreePort(); );
const bindTimer =
server && listenAfterMs > 0
? setTimeout(() => server.listen(upstreamPort, '127.0.0.1'), listenAfterMs)
: null;
const port = await getFreePort(); const port = await getFreePort();
let bindTimer = null;
const target = `127.0.0.1:${upstreamPort}`; const target = `127.0.0.1:${upstreamPort}`;
const proc = spawnServerWithEnv(dir, port, { const proc = spawnServerWithEnv(dir, port, {
GITNEXUS_UPSTREAM_URL: schemeless ? target : `http://${target}`, GITNEXUS_UPSTREAM_URL: schemeless ? target : `http://${target}`,
@ -390,18 +387,25 @@ async function withProxy(
...env, ...env,
}); });
proc.stderr.setEncoding('utf8'); proc.stderr.setEncoding('utf8');
proc.stderr.on('data', (chunk) => { const collectStderr = (chunk) => {
ctx.stderr += chunk; ctx.stderr += chunk;
}); // Process startup must not consume the restart window or skip the retry.
if (server && listenAfterMs > 0 && !bindTimer && ctx.stderr.includes('ECONNREFUSED; retry')) {
bindTimer = setTimeout(() => server.listen(upstreamPort, '127.0.0.1'), listenAfterMs);
}
};
proc.stderr.on('data', collectStderr);
try { try {
await waitForServer(port); await waitForServer(port);
if (!server || listenAfterMs > 0) await new Promise((r) => reservation.close(r));
await fn(port, ctx); await fn(port, ctx);
} finally { } finally {
proc.stderr.off('data', collectStderr);
if (bindTimer) clearTimeout(bindTimer); if (bindTimer) clearTimeout(bindTimer);
await killAndWait(proc); await killAndWait(proc);
if (server?.listening) { if (reservation.listening) {
server.closeAllConnections?.(); reservation.closeAllConnections?.();
await new Promise((r) => server.close(r)); await new Promise((r) => reservation.close(r));
} }
await rm(dir, { recursive: true, force: true }); await rm(dir, { recursive: true, force: true });
} }
@ -661,8 +665,8 @@ it('returns 502 when the upstream is unreachable', async () => {
// -- Connection-retry across an upstream restart window --------------------- // -- Connection-retry across an upstream restart window ---------------------
// //
// `listenAfterMs: 400` binds the upstream late, so the first attempt hits // `listenAfterMs: 400` binds the upstream 400ms after the first ECONNREFUSED,
// ECONNREFUSED and must be retried — a single-instance restart. The default 3 // so the request must be retried — a single-instance restart. The default 3
// attempts (backoff 250ms, 500ms) span ~750ms, so a retry lands after the bind. // attempts (backoff 250ms, 500ms) span ~750ms, so a retry lands after the bind.
it('retries a connection-refused POST and succeeds once the upstream is up', async () => { it('retries a connection-refused POST and succeeds once the upstream is up', async () => {
@ -676,6 +680,7 @@ it('retries a connection-refused POST and succeeds once the upstream is up', asy
assert.match(res.body, /"ok":true/); assert.match(res.body, /"ok":true/);
assert.equal(ctx.calls, 1, 'upstream must run the job exactly once (no double-execute)'); assert.equal(ctx.calls, 1, 'upstream must run the job exactly once (no double-execute)');
assert.equal(ctx.body, '{"repo":"x"}', 'buffered body replayed intact'); assert.equal(ctx.body, '{"repo":"x"}', 'buffered body replayed intact');
assert.match(ctx.stderr, /ECONNREFUSED; retry/, 'the restart gap must exercise a retry');
}); });
}); });