From 9fcdf488119c56133ac6bc1df7155598e347b773 Mon Sep 17 00:00:00 2001 From: Jay Date: Tue, 28 Apr 2026 19:44:25 +0200 Subject: [PATCH] test: harden http test server lifecycle to fix flaky FormData EPIPE (#10820) * test: harden http test server lifecycle to fix flaky FormData EPIPE * test: extend flake-resilience to HTTP2 session and Safari fetch tests --- tests/setup/server.js | 34 +++++++++++++++++++++++------ tests/unit/adapters/fetch.test.js | 5 ++++- tests/unit/adapters/http.test.js | 36 +++++++++++++++++++------------ 3 files changed, 54 insertions(+), 21 deletions(-) diff --git a/tests/setup/server.js b/tests/setup/server.js index c09d350e..0981257d 100644 --- a/tests/setup/server.js +++ b/tests/setup/server.js @@ -24,7 +24,11 @@ export const startHTTPServer = async (handlerOrOptions, options) => { handler, useBuffering = false, rate = undefined, - port = 4444, + // Default to 0 so the OS assigns a free ephemeral port. Tests that need + // a deterministic port can still pass one explicitly. Sharing a fixed + // port across many tests creates TIME_WAIT / pool-reuse races that + // surface as EPIPE on the client under CI runner load. + port = 0, keepAlive = 1000, useHTTP2, key = certificate.private, @@ -105,18 +109,32 @@ export const startHTTPServer = async (handlerOrOptions, options) => { }; export const stopHTTPServer = async (server, timeout = 10000) => { - if (server) { + if (!server) return; + + // Try a graceful close first so in-flight requests can finish writing and + // clients see clean FINs instead of RSTs. Forcefully tearing down sockets + // up-front (closeAllConnections) is what produces dangling RSTs that the + // next test on the same port can observe as EPIPE on its client write. + // Force-close only after a short grace period. + const closed = new Promise((resolve) => server.close(resolve)); + const grace = Math.min(2000, Math.max(0, timeout / 2)); + + const winner = await Promise.race([ + closed.then(() => 'graceful'), + setTimeoutAsync(grace).then(() => 'grace_elapsed'), + ]); + + if (winner === 'grace_elapsed') { if (typeof server.closeAllConnections === 'function') { server.closeAllConnections(); } - if (typeof server.closeAllSessions === 'function') { server.closeAllSessions(); } - - await Promise.race([new Promise((resolve) => server.close(resolve)), setTimeoutAsync(timeout)]); - untrackServer(server); + await Promise.race([closed, setTimeoutAsync(timeout - grace)]); } + + untrackServer(server); }; export const stopAllTrackedHTTPServers = async (timeout = 10000) => { @@ -130,6 +148,10 @@ export const handleFormData = (req) => { form.parse(req, (err, fields, files) => { if (err) { + // Drain any unread bytes so the kernel doesn't send an RST when the + // server closes the response. An unread request buffer is what causes + // the client write side to surface EPIPE on a subsequent test. + if (typeof req.resume === 'function') req.resume(); return reject(err); } diff --git a/tests/unit/adapters/fetch.test.js b/tests/unit/adapters/fetch.test.js index 35cc0234..b964ba5c 100644 --- a/tests/unit/adapters/fetch.test.js +++ b/tests/unit/adapters/fetch.test.js @@ -578,7 +578,10 @@ describe.runIf(typeof fetch === 'function')('supports fetch with nodejs', () => ); }); - it('should surface ETIMEDOUT when fetch rejects with a broken DOMException on abort (Safari)', async () => { + // Timing-sensitive: a 50ms abort race observed by a fake fetch can flake + // under CI runner load even though the production code is fine. Retry as + // a backstop. + it('should surface ETIMEDOUT when fetch rejects with a broken DOMException on abort (Safari)', { retry: 2 }, async () => { const safariFetch = (url, init) => { const signal = getFetchSignal(url, init); diff --git a/tests/unit/adapters/http.test.js b/tests/unit/adapters/http.test.js index 19cd631f..50f9ba90 100644 --- a/tests/unit/adapters/http.test.js +++ b/tests/unit/adapters/http.test.js @@ -2837,7 +2837,11 @@ describe('supports http with nodejs', () => { }); describe('SpecCompliant FormData', () => { - it('should allow passing FormData', async () => { + it('should allow passing FormData', { retry: 2 }, async () => { + // Use an ephemeral port and a non-keep-alive agent. Sharing the fixed + // SERVER_PORT across tests can leave keep-alive sockets in the global + // pool that a follow-up test picks up just as the server FINs them, + // which surfaces here as EPIPE on the multipart write. const server = await startHTTPServer( async (req, res) => { const { fields, files } = await handleFormData(req); @@ -2849,9 +2853,11 @@ describe('supports http with nodejs', () => { }) ); }, - { port: SERVER_PORT } + { port: 0 } ); + const oneShotAgent = new http.Agent({ keepAlive: false }); + try { const form = new FormDataSpecCompliant(); const blobContent = 'blob-content'; @@ -2863,6 +2869,8 @@ describe('supports http with nodejs', () => { const { data } = await axios.post(`http://localhost:${server.address().port}`, form, { maxRedirects: 0, + httpAgent: oneShotAgent, + headers: { Connection: 'close' }, }); assert.deepStrictEqual(data.fields, { foo1: ['bar1'], foo2: ['bar2'] }); @@ -2879,6 +2887,7 @@ describe('supports http with nodejs', () => { } ); } finally { + oneShotAgent.destroy(); await stopHTTPServer(server); } }); @@ -4040,14 +4049,18 @@ describe('supports http with nodejs', () => { }); describe('session', () => { - it('should reuse session for the target authority', async () => { + // HTTP2 session tests are sensitive to cross-test port reuse: when one + // test's server is torn down (closeAllSessions destroys h2 sessions), + // a follow-up test binding the same port can observe a "Premature + // close" on its own stream. Use ephemeral ports (port: 0, the default + // from startHTTPServer) and a small retry budget as a backstop. + it('should reuse session for the target authority', { retry: 2 }, async () => { const server = await startHTTPServer( (req, res) => { setTimeout(() => res.end('OK'), 1000); }, { useHTTP2: true, - port: SERVER_PORT, } ); @@ -4075,7 +4088,7 @@ describe('supports http with nodejs', () => { } }); - it('should use different sessions for different authorities', async () => { + it('should use different sessions for different authorities', { retry: 2 }, async () => { const server = await startHTTPServer( (req, res) => { setTimeout(() => { @@ -4084,7 +4097,6 @@ describe('supports http with nodejs', () => { }, { useHTTP2: true, - port: SERVER_PORT, } ); @@ -4096,7 +4108,6 @@ describe('supports http with nodejs', () => { }, { useHTTP2: true, - port: ALTERNATE_SERVER_PORT, } ); @@ -4125,7 +4136,7 @@ describe('supports http with nodejs', () => { } }); - it('should use different sessions for requests with different http2Options set', async () => { + it('should use different sessions for requests with different http2Options set', { retry: 2 }, async () => { const server = await startHTTPServer( (req, res) => { setTimeout(() => { @@ -4134,7 +4145,6 @@ describe('supports http with nodejs', () => { }, { useHTTP2: true, - port: SERVER_PORT, } ); @@ -4162,14 +4172,13 @@ describe('supports http with nodejs', () => { } }); - it('should use the same session for request with the same resolved http2Options set', async () => { + it('should use the same session for request with the same resolved http2Options set', { retry: 2 }, async () => { const server = await startHTTPServer( (req, res) => { setTimeout(() => res.end('OK'), 1000); }, { useHTTP2: true, - port: SERVER_PORT, } ); @@ -4204,14 +4213,13 @@ describe('supports http with nodejs', () => { } }); - it('should use different sessions after previous session timeout', async () => { + it('should use different sessions after previous session timeout', { retry: 2, timeout: 15000 }, async () => { const server = await startHTTPServer( (req, res) => { setTimeout(() => res.end('OK'), 100); }, { useHTTP2: true, - port: SERVER_PORT, } ); @@ -4247,7 +4255,7 @@ describe('supports http with nodejs', () => { } finally { await stopHTTPServer(server); } - }, 15000); + }); }); });