diff --git a/test/http2-request-never-settles.js b/test/http2-request-never-settles.js index daa3cec942c..60bedfc530b 100644 --- a/test/http2-request-never-settles.js +++ b/test/http2-request-never-settles.js @@ -1,7 +1,7 @@ 'use strict' const assert = require('node:assert') -const { test, after } = require('node:test') +const { test } = require('node:test') const { constants, createSecureServer } = require('node:http2') const { once } = require('node:events') const { Readable } = require('node:stream') @@ -10,6 +10,9 @@ const pem = require('@metcoder95/https-pem') const { Agent } = require('..') +const nodeMajor = Number(process.versions.node.split('.')[0]) +const skipOnAffectedNode = process.platform === 'linux' && (nodeMajor === 24 || nodeMajor === 25) + // completeRequestStream() runs on an h2 stream's 'close': // // releaseRequestStream(this) @@ -114,9 +117,11 @@ async function churningServer (rnd) { session.on('close', () => sessions.delete(session)) }) server.on('secureConnection', socket => socket.on('error', () => {})) - server.shutdown = () => { + server.shutdown = async () => { for (const session of sessions) session.destroy() - server.close() + await new Promise((resolve, reject) => { + server.close((error) => error ? reject(error) : resolve()) + }) } await once(server.listen(0), 'listening') @@ -124,7 +129,10 @@ async function churningServer (rnd) { } for (const seed of SEEDS) { - test(`every h2 request settles under connection churn (seed ${seed})`, async () => { + test(`every h2 request settles under connection churn (seed ${seed})`, { + // https://github.com/nodejs/node/issues/64841 + skip: skipOnAffectedNode && 'Node.js has an HTTP/2 memory corruption bug' + }, async (t) => { const timer = setInterval(() => {}, 1000) const rnd = makeRandom(seed) const server = await churningServer(rnd) @@ -136,10 +144,16 @@ for (const seed of SEEDS) { headersTimeout: 500, bodyTimeout: 500 }) - after(async () => { - await agent.destroy() - server.shutdown() - clearInterval(timer) + // Release each seed's resources before the next test starts. A file-level + // after hook kept every TLS server and keepalive timer until all seeds had + // finished, making this churn test sensitive to CI load. + t.after(async () => { + try { + await agent.destroy() + } finally { + clearInterval(timer) + await server.shutdown() + } }) const unsettled = [] diff --git a/test/issue-5087.js b/test/issue-5087.js index 36cf6059f70..44af141df9e 100644 --- a/test/issue-5087.js +++ b/test/issue-5087.js @@ -1,26 +1,71 @@ 'use strict' const { tspl } = require('@matteo.collina/tspl') -const { test, after } = require('node:test') +const { test } = require('node:test') const { createServer } = require('node:http2') const { once } = require('node:events') const { Client, Agent, RetryAgent, errors, request } = require('..') -test('https://github.com/nodejs/undici/issues/5087 bodyTimeout over h2 rejects with BodyTimeoutError', async (t) => { - t = tspl(t, { plan: 3 }) +function onExpectedTeardownError (error) { + if (error.code !== 'ECONNRESET' && error.code !== 'ERR_HTTP2_STREAM_ERROR') { + throw error + } +} + +function trackServerResources (server) { + const sessions = new Set() + const timers = new Set() + + server.on('error', onExpectedTeardownError) + server.on('session', (session) => { + session.on('error', onExpectedTeardownError) + session.socket?.on('error', onExpectedTeardownError) + sessions.add(session) + session.on('close', () => sessions.delete(session)) + }) + server.on('connection', (socket) => socket.on('error', onExpectedTeardownError)) + + return { + setTimer (callback, delay) { + const timer = setTimeout(() => { + timers.delete(timer) + callback() + }, delay) + timers.add(timer) + }, + clearTimers () { + for (const timer of timers) { + clearTimeout(timer) + } + timers.clear() + }, + async close () { + for (const session of sessions) { + session.destroy() + } + await new Promise((resolve, reject) => { + server.close((error) => error ? reject(error) : resolve()) + }) + } + } +} +test('https://github.com/nodejs/undici/issues/5087 bodyTimeout over h2 rejects with BodyTimeoutError', async (t) => { + const plan = tspl(t, { plan: 3 }) const server = createServer() + const resources = trackServerResources(server) + server.on('stream', (stream) => { + stream.on('error', onExpectedTeardownError) stream.respond({ ':status': 200, 'content-type': 'text/plain' }) - setTimeout(() => { + resources.setTimer(() => { try { stream.end('late') } catch {} }, 500) }) - after(() => server.close()) await once(server.listen(0), 'listening') const client = new Client(`http://localhost:${server.address().port}`, { @@ -29,7 +74,14 @@ test('https://github.com/nodejs/undici/issues/5087 bodyTimeout over h2 rejects w bodyTimeout: 50, headersTimeout: 50 }) - after(() => client.close()) + t.after(async () => { + resources.clearTimers() + try { + await client.destroy() + } finally { + await resources.close() + } + }) const res = await client.request({ path: '/', method: 'GET' }) @@ -40,26 +92,27 @@ test('https://github.com/nodejs/undici/issues/5087 bodyTimeout over h2 rejects w err = error } - t.ok(err instanceof errors.BodyTimeoutError) - t.strictEqual(err.code, 'UND_ERR_BODY_TIMEOUT') - t.strictEqual(err.message, 'HTTP/2: "stream timeout after 50"') + plan.ok(err instanceof errors.BodyTimeoutError) + plan.strictEqual(err.code, 'UND_ERR_BODY_TIMEOUT') + plan.strictEqual(err.message, 'HTTP/2: "stream timeout after 50"') - await t.completed + await plan.completed }) test('https://github.com/nodejs/undici/issues/5087 headersTimeout over h2 rejects with HeadersTimeoutError', async (t) => { - t = tspl(t, { plan: 3 }) - + const plan = tspl(t, { plan: 3 }) const server = createServer() + const resources = trackServerResources(server) + server.on('stream', (stream) => { - setTimeout(() => { + stream.on('error', onExpectedTeardownError) + resources.setTimer(() => { try { stream.close() } catch {} }, 500) }) - after(() => server.close()) await once(server.listen(0), 'listening') const client = new Client(`http://localhost:${server.address().port}`, { @@ -68,7 +121,14 @@ test('https://github.com/nodejs/undici/issues/5087 headersTimeout over h2 reject bodyTimeout: 60_000, headersTimeout: 50 }) - after(() => client.close()) + t.after(async () => { + resources.clearTimers() + try { + await client.destroy() + } finally { + await resources.close() + } + }) let err = null try { @@ -77,25 +137,27 @@ test('https://github.com/nodejs/undici/issues/5087 headersTimeout over h2 reject err = error } - t.ok(err instanceof errors.HeadersTimeoutError) - t.strictEqual(err.code, 'UND_ERR_HEADERS_TIMEOUT') - t.strictEqual(err.message, 'HTTP/2: "headers timeout after 50"') + plan.ok(err instanceof errors.HeadersTimeoutError) + plan.strictEqual(err.code, 'UND_ERR_HEADERS_TIMEOUT') + plan.strictEqual(err.message, 'HTTP/2: "headers timeout after 50"') - await t.completed + await plan.completed }) test('https://github.com/nodejs/undici/issues/5087 RetryAgent retries h2 body timeouts by default error code matching', async (t) => { - t = tspl(t, { plan: 2 }) - + const plan = tspl(t, { plan: 2 }) let hits = 0 const server = createServer() + const resources = trackServerResources(server) + server.on('stream', (stream) => { + stream.on('error', onExpectedTeardownError) hits += 1 stream.respond({ ':status': 200, 'content-type': 'text/plain' }) if (hits === 1) { - setTimeout(() => { + resources.setTimer(() => { try { stream.end('late') } catch {} @@ -106,7 +168,6 @@ test('https://github.com/nodejs/undici/issues/5087 RetryAgent retries h2 body ti stream.end(`ok after ${hits} attempt(s)`) }) - after(() => server.close()) await once(server.listen(0), 'listening') const dispatcher = new RetryAgent(new Agent({ @@ -119,15 +180,22 @@ test('https://github.com/nodejs/undici/issues/5087 RetryAgent retries h2 body ti minTimeout: 10, errorCodes: ['UND_ERR_BODY_TIMEOUT'] }) - after(() => dispatcher.close()) + t.after(async () => { + resources.clearTimers() + try { + await dispatcher.destroy() + } finally { + await resources.close() + } + }) const res = await request(`http://localhost:${server.address().port}/`, { dispatcher, method: 'GET' }) - t.strictEqual(await res.body.text(), 'ok after 2 attempt(s)') - t.strictEqual(hits, 2) + plan.strictEqual(await res.body.text(), 'ok after 2 attempt(s)') + plan.strictEqual(hits, 2) - await t.completed + await plan.completed })