From bb71a11bd006c36a0570a442016e63a525de3bf8 Mon Sep 17 00:00:00 2001 From: Kushagra Pandey Date: Sat, 22 Mar 2025 03:12:25 +0530 Subject: [PATCH 1/7] http2: session tracking and graceful server close This change adds proper tracking of HTTP / 2 server sessions to ensure they are gracefully closed when the server is shut down.It implements: - A new kSessions symbol for tracking active sessions - Adding/removing sessions from a SafeSet in the server - A closeAllSessions helper function to close active sessions - Updates to Http2Server and Http2SecureServer close methods Breaking Change: any client trying to create new requests on existing connections will not be able to do so once server close is initiated Refs: https://datatracker.ietf.org/doc/html/rfc7540\#section-9.1 Refs: https://nodejs.org/api/http.html\#serverclosecallback --- lib/internal/http2/core.js | 29 +++ test/parallel/test-http2-capture-rejection.js | 4 +- ...rverresponse-statusmessage-property-set.js | 4 +- ...t-serverresponse-statusmessage-property.js | 4 +- .../test-http2-request-after-server-close.js | 217 ++++++++++++++++ ...test-http2-server-close-client-behavior.js | 231 ++++++++++++++++++ 6 files changed, 485 insertions(+), 4 deletions(-) create mode 100644 test/parallel/test-http2-request-after-server-close.js create mode 100644 test/parallel/test-http2-server-close-client-behavior.js diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 554221ac614636..2071ed50b0ac78 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -251,6 +251,7 @@ const kServer = Symbol('server'); const kState = Symbol('state'); const kType = Symbol('type'); const kWriteGeneric = Symbol('write-generic'); +const kSessions = Symbol('sessions'); const { kBitfield, @@ -1125,9 +1126,13 @@ function emitClose(self, error) { function cleanupSession(session) { const socket = session[kSocket]; const handle = session[kHandle]; + const server = session[kServer]; session[kProxySocket] = undefined; session[kSocket] = undefined; session[kHandle] = undefined; + if (server) { + server[kSessions].delete(session); + } session[kNativeFields] = trackAssignmentsTypedArray( new Uint8Array(kSessionUint8FieldCount)); if (handle) @@ -1644,6 +1649,9 @@ class ServerHttp2Session extends Http2Session { constructor(options, socket, server) { super(NGHTTP2_SESSION_SERVER, options, socket); this[kServer] = server; + if (server) { + server[kSessions].add(this); + } // This is a bit inaccurate because it does not reflect changes to // number of listeners made after the session was created. This should // not be an issue in practice. Additionally, the 'priority' event on @@ -3168,11 +3176,25 @@ function onErrorSecureServerSession(err, socket) { socket.destroy(err); } +/** + * This function closes all active sessions gracefully. + * @param {*} server the underlying server whose sessions to be closed + */ +function closeAllSessions(server) { + const sessions = server[kSessions]; + if (sessions.size > 0) { + sessions.forEach((session) => { + session.close(); + }); + } +} + class Http2SecureServer extends TLSServer { constructor(options, requestListener) { options = initializeTLSOptions(options); super(options, connectionListener); this[kOptions] = options; + this[kSessions] = new SafeSet(); this.timeout = 0; this.on('newListener', setupCompat); if (options.allowHTTP1 === true) { @@ -3205,6 +3227,7 @@ class Http2SecureServer extends TLSServer { if (this[kOptions].allowHTTP1 === true) { httpServerPreClose(this); } + closeAllSessions(this); ReflectApply(TLSServer.prototype.close, this, arguments); } @@ -3220,6 +3243,7 @@ class Http2Server extends NETServer { options = initializeOptions(options); super(options, connectionListener); this[kOptions] = options; + this[kSessions] = new SafeSet(); this.timeout = 0; this.on('newListener', setupCompat); if (typeof requestListener === 'function') @@ -3241,6 +3265,11 @@ class Http2Server extends NETServer { this[kOptions].settings = { ...this[kOptions].settings, ...settings }; } + close() { + closeAllSessions(this); + ReflectApply(NETServer.prototype.close, this, arguments); + } + async [SymbolAsyncDispose]() { return promisify(super.close).call(this); } diff --git a/test/parallel/test-http2-capture-rejection.js b/test/parallel/test-http2-capture-rejection.js index 6d8cb224ce7919..39feec784f3d4a 100644 --- a/test/parallel/test-http2-capture-rejection.js +++ b/test/parallel/test-http2-capture-rejection.js @@ -108,8 +108,6 @@ events.captureRejections = true; server.on('stream', common.mustCall(async (stream) => { const { port } = server.address(); - server.close(); - stream.pushStream({ ':scheme': 'http', ':path': '/foobar', @@ -127,6 +125,8 @@ events.captureRejections = true; stream.respond({ ':status': 200 }); + + server.close(); })); server.listen(0, common.mustCall(() => { diff --git a/test/parallel/test-http2-compat-serverresponse-statusmessage-property-set.js b/test/parallel/test-http2-compat-serverresponse-statusmessage-property-set.js index 87e172402899f2..778600775eb45f 100644 --- a/test/parallel/test-http2-compat-serverresponse-statusmessage-property-set.js +++ b/test/parallel/test-http2-compat-serverresponse-statusmessage-property-set.js @@ -24,7 +24,6 @@ server.listen(0, common.mustCall(function() { response.statusMessage = 'test'; response.statusMessage = 'test'; // only warn once assert.strictEqual(response.statusMessage, ''); // no change - server.close(); })); response.end(); })); @@ -44,6 +43,9 @@ server.listen(0, common.mustCall(function() { request.on('end', common.mustCall(function() { client.close(); })); + request.on('close', common.mustCall(function() { + server.close(); + })); request.end(); request.resume(); })); diff --git a/test/parallel/test-http2-compat-serverresponse-statusmessage-property.js b/test/parallel/test-http2-compat-serverresponse-statusmessage-property.js index 8a083cf3ba1638..eaffcc11cd3175 100644 --- a/test/parallel/test-http2-compat-serverresponse-statusmessage-property.js +++ b/test/parallel/test-http2-compat-serverresponse-statusmessage-property.js @@ -23,7 +23,6 @@ server.listen(0, common.mustCall(function() { response.on('finish', common.mustCall(function() { assert.strictEqual(response.statusMessage, ''); assert.strictEqual(response.statusMessage, ''); // only warn once - server.close(); })); response.end(); })); @@ -43,6 +42,9 @@ server.listen(0, common.mustCall(function() { request.on('end', common.mustCall(function() { client.close(); })); + request.on('close', common.mustCall(function() { + server.close(); + })); request.end(); request.resume(); })); diff --git a/test/parallel/test-http2-request-after-server-close.js b/test/parallel/test-http2-request-after-server-close.js new file mode 100644 index 00000000000000..4b6b2b4508e5ac --- /dev/null +++ b/test/parallel/test-http2-request-after-server-close.js @@ -0,0 +1,217 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) { common.skip('missing crypto'); }; +const fixtures = require('../common/fixtures'); +// This test ensure that the server will not accept any new request +// after server close is called. +const assert = require('assert'); +const http2 = require('http2'); + +const { test } = require('node:test'); + +/** + * Create and manage an HTTP/2 client stream with controlled write patterns + * @param {http2.ClientHttp2Session} client - The HTTP/2 client session + * @param {string} clientId - Identifier for the client (e.g., '1', '2') + * @param {number} writeCount - Number of writes to perform + * @param {number} writeInterval - Interval between writes in ms + * @returns {object} - Object containing stream, status tracking, and functions + */ +function createClientStream(client, clientId, writeCount, writeInterval = 100) { + let currentWriteCount = 0; + let intervalId = null; + let streamClosed = false; + + // Create the request + const req = client.request({ + ':path': `/client${clientId}`, + ':method': 'POST', + 'client-id': clientId, + 'content-type': 'text/plain' + }); + + // Set up event handlers + req.on('response', (_) => {}); + + req.on('data', (_) => {}); + + req.on('end', () => { + streamClosed = true; + }); + + req.on('close', () => { + streamClosed = true; + if (intervalId) { + clearInterval(intervalId); + intervalId = null; + } + }); + + req.on('error', (err) => { + if (intervalId) { + clearInterval(intervalId); + intervalId = null; + } + }); + + // Start the write interval + intervalId = setInterval(() => { + currentWriteCount++; + if (currentWriteCount > writeCount) { + if (intervalId) { + clearInterval(intervalId); + intervalId = null; + } + req.close(); + return; + } + + req.write(`Client ${clientId} write #${currentWriteCount}\n`); + }, writeInterval); + + // Return object with stream, status tracking, and cleanup function + return { + stream: req, + getWriteCount: () => currentWriteCount, + isActive: () => !streamClosed && !req.destroyed && !req.closed, + }; +} + +// This test start a server and create a client. Client open a request and +// send 20 writes at interval of 100ms and then close at 2000ms from server start. +// Server close is fired after 1000ms from server start. +// Same client open another request after 1500ms from server start and tries to +// send 10 writes at interval of 100ms but failed to connect as server close is already fired at 1000ms. +// Request 1 from client is gracefully closed after accepting all 20 writes as it started before server close fired. +// server successfully closes gracefully after receiving all 20 writes from client and also server refused to accept any new request. +test('HTTP/2 server close with existing and new requests', async () => { + + // Server setup + const server = http2.createSecureServer({ + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem') + }); + + // Track server events + let serverStart = 0; + let serverCloseTime = 0; + let requestsReceived = 0; + let writesReceived = 0; + let req1Complete = false; + let req2Error = null; + + // Handle streams on the server + server.on('stream', (stream, headers) => { + requestsReceived++; + + stream.respond({ + ':status': 200, + 'content-type': 'text/plain' + }); + + // Count writes from clients + stream.on('data', (chunk) => { + writesReceived++; + stream.write(`Echo: ${chunk.toString().trim()}`); + }); + + stream.on('end', () => { + stream.end('Server: Stream closed'); + }); + }); + + // Start the server + await new Promise((resolve) => server.listen(0, () => { + serverStart = Date.now(); + resolve(); + })); + const port = server.address().port; + + // Create client + const client = http2.connect(`https://localhost:${port}`, { + rejectUnauthorized: false + }); + + // Create first request that will start immediately and write 20 times eache write at interval of 100ms + // The request will be closed at 2000ms after 20 writes + const request1 = createClientStream(client, '1', 20, 100); + + // wait 1000ms before closing the server + await new Promise((resolve) => setTimeout(resolve, common.platformTimeout(1000))); + + // close the server + await new Promise((resolve) => { + server.close(() => { + serverCloseTime = Date.now(); + resolve(); + }); + }); + + // Wait 500ms before creating the second request + await new Promise((resolve) => setTimeout(resolve, common.platformTimeout(500))); + + // Try to create the second request after 1500ms of server start - should fail + try { + const request2 = createClientStream(client, '2', 10, 100); + // If we get here without error, wait to see if an error event happens + request2.stream.on('error', (err) => { + req2Error = err; + }); + + } catch (err) { + // Should fail synchronously with ERR_HTTP2_INVALID_SESSION + req2Error = err; + } + + // Wait for request 1 to complete gracefully (should be around 2000ms) + await new Promise((resolve) => { + const checkComplete = () => { + if (!request1.isActive()) { + req1Complete = true; + resolve(); + } else { + // Check again in 100ms + setTimeout(checkComplete, common.platformTimeout(100)); + } + }; + + // Set a timeout to prevent hanging if request never completes + setTimeout(() => { + resolve(); + }, common.platformTimeout(1500)); + + checkComplete(); + }); + + // Ensure client is closed + client.close(); + + // Wait for cleanup + await new Promise((resolve) => setTimeout(resolve, common.platformTimeout(200))); + + // Verify test expectations + + // Request 1 should have completed + assert.ok(req1Complete, 'Request 1 should complete gracefully'); + assert.ok(request1.getWriteCount() > 0, 'Request 1 should have written data'); + // Request 1 should have written 20 times and request 2 written 0 times + assert.strictEqual(writesReceived, 20); + + // Request 2 fails with ERR_HTTP2_INVALID_SESSION because the server + // fired close at 1000ms which stops accepting any new request. + // Since Request 2 starts at 1500ms, it fails. + assert.ok(req2Error, 'Request 2 should have an error'); + // Request 2 should fail with ERR_HTTP2_INVALID_SESSION + assert.strictEqual(req2Error.code, 'ERR_HTTP2_INVALID_SESSION'); + + // Server should have received only the first request as 2nd request received after server close fired. + assert.strictEqual(requestsReceived, 1); + assert.ok( + serverCloseTime - serverStart >= 2000, + `Server should fully close after 2000ms of server start when all streams complete (actual: ${serverCloseTime - serverStart}ms)` + ); + assert.ok( + (serverCloseTime - serverStart) - 2000 < 200, + `Server should fully close just after all streams complete` + ); +}); diff --git a/test/parallel/test-http2-server-close-client-behavior.js b/test/parallel/test-http2-server-close-client-behavior.js new file mode 100644 index 00000000000000..280dc2f35cf853 --- /dev/null +++ b/test/parallel/test-http2-server-close-client-behavior.js @@ -0,0 +1,231 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) { common.skip('missing crypto'); }; +const fixtures = require('../common/fixtures'); +// This test ensure that server will close grcaefully after completing all active streams. +// It also ensures that idle connections will close immediately after server close is called. +const assert = require('assert'); +const http2 = require('http2'); + +const { test } = require('node:test'); + +/** + * Create and manage an HTTP/2 client with a single stream + * @param {http2.ClientHttp2Session} client - The HTTP/2 client session + * @param {string} clientId - Identifier for the client (e.g., '1', '2') + * @param {number} writeCount - Number of writes to perform + * @param {number} writeInterval - Interval between writes in ms + * @returns {object} - Object containing stream, writeTracker, and cleanup function + */ +function createClientStream(client, clientId, writeCount, writeInterval = 100) { + let currentWriteCount = 0; + let intervalId = null; + let streamClosed = false; + + // Create the request + const req = client.request({ + ':path': `/client${clientId}`, + ':method': 'POST', + 'client-id': clientId, + 'content-type': 'text/plain' + }); + + // Set up event handlers + req.on('response', (_) => {}); + + req.on('data', (_) => {}); + + req.on('end', () => { + streamClosed = true; + }); + + req.on('close', () => { + streamClosed = true; + if (intervalId) { + clearInterval(intervalId); + intervalId = null; + } + }); + + req.on('error', (err) => { + if (intervalId) { + clearInterval(intervalId); + intervalId = null; + } + }); + + // Start the write interval + intervalId = setInterval(() => { + currentWriteCount++; + if (currentWriteCount > writeCount) { + if (intervalId) { + clearInterval(intervalId); + intervalId = null; + } + req.close(); + return; + } + + req.write(`Client ${clientId} write #${currentWriteCount}\n`); + }, writeInterval); + + // Return object with stream, status tracking, and cleanup function + return { + stream: req, + getWriteCount: () => currentWriteCount, + isActive: () => !streamClosed && !req.destroyed && !req.closed, + }; +} + +// This test init a server and connect two clients, client1 and client2. +// Client1 sends 10 writes at interval of 100ms and then close at 1000ms from server start. +// Client2 sends 20 writes at interval of 100ms and then close at 2000ms from server start. +// Server close is fired after 1500ms from server start. +// This test verifies that server closes all active sessions gracefully and +// client1 session terminates immediately after server close is fired as server immediately closes all idle sessions. +// client2 session terminates approximately 500ms after server close as client2 write till 2000ms from server start. +test('HTTP/2 server close behavior with different client behavior', async () => { + const server = http2.createSecureServer({ + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), + }); + + let serverStart = 0; + let streamCount = 0; + let writeCount1 = 0; + let writeCount2 = 0; + let serverCloseTime = 0; + + // Track server-side session termination + const sessionTerminationTimes = new Map(); + + server.on('session', (session) => { + + // Store the client ID in the session for identification + session.clientId = null; + + session.on('close', () => { + const terminationTime = Date.now(); + if (session.clientId) { + sessionTerminationTimes.set(session.clientId, terminationTime); + } + }); + + session.on('error', (err) => { + console.error(`Session error:`, err); + }); + }); + + // Server stream handler + server.on('stream', (stream, headers) => { + const clientId = headers['client-id']; + streamCount++; + + // Identify the session this stream belongs to + if (stream.session && !stream.session.clientId) { + stream.session.clientId = clientId; + } + + stream.respond({ + ':status': 200, + 'content-type': 'text/plain' + }); + + // Handle incoming data + stream.on('data', (chunk) => { + const data = chunk.toString().trim(); + + if (clientId === '1') { + writeCount1++; + } else if (clientId === '2') { + writeCount2++; + } + + stream.write(`Echo: ${data}`); + }); + + stream.on('end', () => {}); + + stream.on('error', (err) => { + console.error(`Stream ${stream.id} error:`, err); + }); + }); + + // Start the server + await new Promise((resolve) => server.listen(0, () => { + serverStart = Date.now(); + resolve(); + })); + + const port = server.address().port; + + // This client will send 10 writes at interval of 100ms and then close at 1000ms from server start. + const client1 = http2.connect(`https://localhost:${port}`, { + rejectUnauthorized: false + }); + + // This client will send 20 writes at interval of 100ms and then close at 2000ms from server start. + const client2 = http2.connect(`https://localhost:${port}`, { + rejectUnauthorized: false + }); + + // Create streams for both clients + createClientStream(client1, '1', 10, 100); + createClientStream(client2, '2', 20, 100); + + + // Wait for 1500ms before server close + await new Promise((resolve) => setTimeout(resolve, common.platformTimeout(1500))); + const serverCloseFireTime = Date.now(); + + // Fire server close after 1500ms of server start + await new Promise((resolve) => { + server.close(() => { + serverCloseTime = Date.now(); + resolve(); + }); + }); + + + // Wait for all server-side session terminations to complete + await new Promise((resolve) => setTimeout(resolve, common.platformTimeout(500))); + + // Verify test expectations + // Should have exactly 2 streams (one per client)' + assert.strictEqual(streamCount, 2); + // Server should have received 10 writes from client 1 + assert.strictEqual(writeCount1, 10); + // Server should have received 20 writes from client 2 + assert.strictEqual(writeCount2, 20); + + // Verify session termination times from server's perspective + assert.ok(sessionTerminationTimes.has('1'), 'Server should record termination of client 1 session'); + assert.ok(sessionTerminationTimes.has('2'), 'Server should record termination of client 2 session'); + + const client1TerminationTime = sessionTerminationTimes.get('1'); + const client2TerminationTime = sessionTerminationTimes.get('2'); + + // Client 1 session should terminate immediately after + // server close is fired as server immediately closes all idle sessions. + assert.ok( + client1TerminationTime - serverCloseTime < 100, + `Client 1 session should terminate shortly after server close (actual delay: ${client1TerminationTime - serverCloseTime}ms)` + ); + + // Client 2 session should terminate approximately 500ms after + // server close as client write till 2000ms from server start. + const client2Delay = client2TerminationTime - serverCloseFireTime; + assert.ok( + client2Delay >= 450 && client2Delay <= 650, + `Client 2 session should terminate ~500ms after server close (actual delay: ${client2Delay}ms)` + ); + assert.ok( + serverCloseTime - serverStart >= 2000, + `Server should fully close after 2000ms of server start when all streams complete (actual: ${serverCloseTime - serverStart}ms)` + ); + assert.ok( + (serverCloseTime - serverStart) - 2000 < 200, + `Server should fully close just after all streams complete` + ); + +}); From 7a852db48479d691305ce3be3641a50af4437634 Mon Sep 17 00:00:00 2001 From: Kushagra Pandey Date: Fri, 28 Mar 2025 23:46:38 +0530 Subject: [PATCH 2/7] fix: improve HTTP/2 server shutdown to prevent race conditions 1. Fix server shutdown race condition - Stop listening for new connections before closing existing ones - Ensure server.close() properly completes in all scenarios 2. Improve HTTP/2 tests - Replace setTimeout with event-based flow control - Simplify test logic for better readability - Add clear state tracking for event ordering - Improve assertions to verify correct shutdown sequence This eliminates a race condition where new sessions could connect between the time existing sessions are closed and the server stops listening, potentially preventing the server from fully shutting down. --- lib/internal/http2/core.js | 4 +- test/parallel/test-http2-graceful-close.js | 103 ++++++++ .../test-http2-request-after-server-close.js | 217 ---------------- ...test-http2-server-close-client-behavior.js | 231 ------------------ ...test-http2-server-close-idle-connection.js | 100 ++++++++ 5 files changed, 205 insertions(+), 450 deletions(-) create mode 100644 test/parallel/test-http2-graceful-close.js delete mode 100644 test/parallel/test-http2-request-after-server-close.js delete mode 100644 test/parallel/test-http2-server-close-client-behavior.js create mode 100644 test/parallel/test-http2-server-close-idle-connection.js diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 2071ed50b0ac78..0cc4ead811716a 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -3224,11 +3224,11 @@ class Http2SecureServer extends TLSServer { } close() { + ReflectApply(TLSServer.prototype.close, this, arguments); if (this[kOptions].allowHTTP1 === true) { httpServerPreClose(this); } closeAllSessions(this); - ReflectApply(TLSServer.prototype.close, this, arguments); } closeIdleConnections() { @@ -3266,8 +3266,8 @@ class Http2Server extends NETServer { } close() { - closeAllSessions(this); ReflectApply(NETServer.prototype.close, this, arguments); + closeAllSessions(this); } async [SymbolAsyncDispose]() { diff --git a/test/parallel/test-http2-graceful-close.js b/test/parallel/test-http2-graceful-close.js new file mode 100644 index 00000000000000..753f65d5fd7ea4 --- /dev/null +++ b/test/parallel/test-http2-graceful-close.js @@ -0,0 +1,103 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const assert = require('assert'); +const http2 = require('http2'); + +// This test verifies that the server waits for active connections/sessions +// to complete and all data to be sent before fully closing + +// Keep track of the test flow with these flags +const states = { + serverListening: false, + responseStarted: false, + dataFullySent: false, + streamClosed: false, + serverClosed: false +}; + +// Create a server that will send a large response in chunks +const server = http2.createServer(); + +// Track server events +server.on('stream', common.mustCall((stream, headers) => { + assert.strictEqual(states.serverListening, true); + + // Setup the response + stream.respond({ + 'content-type': 'text/plain', + ':status': 200 + }); + + // Initiate the server close before client data is sent, this will + // test if the server properly waits for the stream to finish + server.close(common.mustCall(() => { + // Stream should be closed before server close callback + // Should be called only after the stream has closed + assert.strictEqual(states.streamClosed, true); + states.serverClosed = true; + })); + + // Mark response as started + states.responseStarted = true; + + // Create a large response (1MB) to ensure it takes some time to send + const chunk = Buffer.alloc(64 * 1024, 'x'); + + // Send 16 chunks (1MB total) to simulate a large response + for (let i = 0; i < 16; i++) { + stream.write(chunk); + } + + + // Stream end should happen after data is written + stream.end(); + states.dataFullySent = true; + + // When stream closes, we can verify order of events + stream.on('close', common.mustCall(() => { + // Data should be fully sent before stream closes + assert.strictEqual(states.dataFullySent, true); + states.streamClosed = true; + })); +})); + +// Start the server +server.listen(0, common.mustCall(() => { + states.serverListening = true; + + // Create client and request + const client = http2.connect(`http://localhost:${server.address().port}`); + const req = client.request({ ':path': '/' }); + + // Track received data + let receivedData = 0; + req.on('data', (chunk) => { + receivedData += chunk.length; + }); + + // When the request completes + req.on('end', common.mustCall(() => { + // All data should be sent before request ends + assert.strictEqual(states.dataFullySent, true); + })); + + // When request closes + req.on('close', common.mustCall(() => { + // Check final state + assert.strictEqual(states.streamClosed, true); + // Should receive all data + assert.strictEqual(receivedData, 64 * 1024 * 16); + + // Wait for the server close confirmation + process.on('exit', () => { + // Server should be fully closed + assert.strictEqual(states.serverClosed, true); + }); + })); + + // Start the request + req.end(); +})); diff --git a/test/parallel/test-http2-request-after-server-close.js b/test/parallel/test-http2-request-after-server-close.js deleted file mode 100644 index 4b6b2b4508e5ac..00000000000000 --- a/test/parallel/test-http2-request-after-server-close.js +++ /dev/null @@ -1,217 +0,0 @@ -'use strict'; -const common = require('../common'); -if (!common.hasCrypto) { common.skip('missing crypto'); }; -const fixtures = require('../common/fixtures'); -// This test ensure that the server will not accept any new request -// after server close is called. -const assert = require('assert'); -const http2 = require('http2'); - -const { test } = require('node:test'); - -/** - * Create and manage an HTTP/2 client stream with controlled write patterns - * @param {http2.ClientHttp2Session} client - The HTTP/2 client session - * @param {string} clientId - Identifier for the client (e.g., '1', '2') - * @param {number} writeCount - Number of writes to perform - * @param {number} writeInterval - Interval between writes in ms - * @returns {object} - Object containing stream, status tracking, and functions - */ -function createClientStream(client, clientId, writeCount, writeInterval = 100) { - let currentWriteCount = 0; - let intervalId = null; - let streamClosed = false; - - // Create the request - const req = client.request({ - ':path': `/client${clientId}`, - ':method': 'POST', - 'client-id': clientId, - 'content-type': 'text/plain' - }); - - // Set up event handlers - req.on('response', (_) => {}); - - req.on('data', (_) => {}); - - req.on('end', () => { - streamClosed = true; - }); - - req.on('close', () => { - streamClosed = true; - if (intervalId) { - clearInterval(intervalId); - intervalId = null; - } - }); - - req.on('error', (err) => { - if (intervalId) { - clearInterval(intervalId); - intervalId = null; - } - }); - - // Start the write interval - intervalId = setInterval(() => { - currentWriteCount++; - if (currentWriteCount > writeCount) { - if (intervalId) { - clearInterval(intervalId); - intervalId = null; - } - req.close(); - return; - } - - req.write(`Client ${clientId} write #${currentWriteCount}\n`); - }, writeInterval); - - // Return object with stream, status tracking, and cleanup function - return { - stream: req, - getWriteCount: () => currentWriteCount, - isActive: () => !streamClosed && !req.destroyed && !req.closed, - }; -} - -// This test start a server and create a client. Client open a request and -// send 20 writes at interval of 100ms and then close at 2000ms from server start. -// Server close is fired after 1000ms from server start. -// Same client open another request after 1500ms from server start and tries to -// send 10 writes at interval of 100ms but failed to connect as server close is already fired at 1000ms. -// Request 1 from client is gracefully closed after accepting all 20 writes as it started before server close fired. -// server successfully closes gracefully after receiving all 20 writes from client and also server refused to accept any new request. -test('HTTP/2 server close with existing and new requests', async () => { - - // Server setup - const server = http2.createSecureServer({ - key: fixtures.readKey('agent1-key.pem'), - cert: fixtures.readKey('agent1-cert.pem') - }); - - // Track server events - let serverStart = 0; - let serverCloseTime = 0; - let requestsReceived = 0; - let writesReceived = 0; - let req1Complete = false; - let req2Error = null; - - // Handle streams on the server - server.on('stream', (stream, headers) => { - requestsReceived++; - - stream.respond({ - ':status': 200, - 'content-type': 'text/plain' - }); - - // Count writes from clients - stream.on('data', (chunk) => { - writesReceived++; - stream.write(`Echo: ${chunk.toString().trim()}`); - }); - - stream.on('end', () => { - stream.end('Server: Stream closed'); - }); - }); - - // Start the server - await new Promise((resolve) => server.listen(0, () => { - serverStart = Date.now(); - resolve(); - })); - const port = server.address().port; - - // Create client - const client = http2.connect(`https://localhost:${port}`, { - rejectUnauthorized: false - }); - - // Create first request that will start immediately and write 20 times eache write at interval of 100ms - // The request will be closed at 2000ms after 20 writes - const request1 = createClientStream(client, '1', 20, 100); - - // wait 1000ms before closing the server - await new Promise((resolve) => setTimeout(resolve, common.platformTimeout(1000))); - - // close the server - await new Promise((resolve) => { - server.close(() => { - serverCloseTime = Date.now(); - resolve(); - }); - }); - - // Wait 500ms before creating the second request - await new Promise((resolve) => setTimeout(resolve, common.platformTimeout(500))); - - // Try to create the second request after 1500ms of server start - should fail - try { - const request2 = createClientStream(client, '2', 10, 100); - // If we get here without error, wait to see if an error event happens - request2.stream.on('error', (err) => { - req2Error = err; - }); - - } catch (err) { - // Should fail synchronously with ERR_HTTP2_INVALID_SESSION - req2Error = err; - } - - // Wait for request 1 to complete gracefully (should be around 2000ms) - await new Promise((resolve) => { - const checkComplete = () => { - if (!request1.isActive()) { - req1Complete = true; - resolve(); - } else { - // Check again in 100ms - setTimeout(checkComplete, common.platformTimeout(100)); - } - }; - - // Set a timeout to prevent hanging if request never completes - setTimeout(() => { - resolve(); - }, common.platformTimeout(1500)); - - checkComplete(); - }); - - // Ensure client is closed - client.close(); - - // Wait for cleanup - await new Promise((resolve) => setTimeout(resolve, common.platformTimeout(200))); - - // Verify test expectations - - // Request 1 should have completed - assert.ok(req1Complete, 'Request 1 should complete gracefully'); - assert.ok(request1.getWriteCount() > 0, 'Request 1 should have written data'); - // Request 1 should have written 20 times and request 2 written 0 times - assert.strictEqual(writesReceived, 20); - - // Request 2 fails with ERR_HTTP2_INVALID_SESSION because the server - // fired close at 1000ms which stops accepting any new request. - // Since Request 2 starts at 1500ms, it fails. - assert.ok(req2Error, 'Request 2 should have an error'); - // Request 2 should fail with ERR_HTTP2_INVALID_SESSION - assert.strictEqual(req2Error.code, 'ERR_HTTP2_INVALID_SESSION'); - - // Server should have received only the first request as 2nd request received after server close fired. - assert.strictEqual(requestsReceived, 1); - assert.ok( - serverCloseTime - serverStart >= 2000, - `Server should fully close after 2000ms of server start when all streams complete (actual: ${serverCloseTime - serverStart}ms)` - ); - assert.ok( - (serverCloseTime - serverStart) - 2000 < 200, - `Server should fully close just after all streams complete` - ); -}); diff --git a/test/parallel/test-http2-server-close-client-behavior.js b/test/parallel/test-http2-server-close-client-behavior.js deleted file mode 100644 index 280dc2f35cf853..00000000000000 --- a/test/parallel/test-http2-server-close-client-behavior.js +++ /dev/null @@ -1,231 +0,0 @@ -'use strict'; -const common = require('../common'); -if (!common.hasCrypto) { common.skip('missing crypto'); }; -const fixtures = require('../common/fixtures'); -// This test ensure that server will close grcaefully after completing all active streams. -// It also ensures that idle connections will close immediately after server close is called. -const assert = require('assert'); -const http2 = require('http2'); - -const { test } = require('node:test'); - -/** - * Create and manage an HTTP/2 client with a single stream - * @param {http2.ClientHttp2Session} client - The HTTP/2 client session - * @param {string} clientId - Identifier for the client (e.g., '1', '2') - * @param {number} writeCount - Number of writes to perform - * @param {number} writeInterval - Interval between writes in ms - * @returns {object} - Object containing stream, writeTracker, and cleanup function - */ -function createClientStream(client, clientId, writeCount, writeInterval = 100) { - let currentWriteCount = 0; - let intervalId = null; - let streamClosed = false; - - // Create the request - const req = client.request({ - ':path': `/client${clientId}`, - ':method': 'POST', - 'client-id': clientId, - 'content-type': 'text/plain' - }); - - // Set up event handlers - req.on('response', (_) => {}); - - req.on('data', (_) => {}); - - req.on('end', () => { - streamClosed = true; - }); - - req.on('close', () => { - streamClosed = true; - if (intervalId) { - clearInterval(intervalId); - intervalId = null; - } - }); - - req.on('error', (err) => { - if (intervalId) { - clearInterval(intervalId); - intervalId = null; - } - }); - - // Start the write interval - intervalId = setInterval(() => { - currentWriteCount++; - if (currentWriteCount > writeCount) { - if (intervalId) { - clearInterval(intervalId); - intervalId = null; - } - req.close(); - return; - } - - req.write(`Client ${clientId} write #${currentWriteCount}\n`); - }, writeInterval); - - // Return object with stream, status tracking, and cleanup function - return { - stream: req, - getWriteCount: () => currentWriteCount, - isActive: () => !streamClosed && !req.destroyed && !req.closed, - }; -} - -// This test init a server and connect two clients, client1 and client2. -// Client1 sends 10 writes at interval of 100ms and then close at 1000ms from server start. -// Client2 sends 20 writes at interval of 100ms and then close at 2000ms from server start. -// Server close is fired after 1500ms from server start. -// This test verifies that server closes all active sessions gracefully and -// client1 session terminates immediately after server close is fired as server immediately closes all idle sessions. -// client2 session terminates approximately 500ms after server close as client2 write till 2000ms from server start. -test('HTTP/2 server close behavior with different client behavior', async () => { - const server = http2.createSecureServer({ - key: fixtures.readKey('agent1-key.pem'), - cert: fixtures.readKey('agent1-cert.pem'), - }); - - let serverStart = 0; - let streamCount = 0; - let writeCount1 = 0; - let writeCount2 = 0; - let serverCloseTime = 0; - - // Track server-side session termination - const sessionTerminationTimes = new Map(); - - server.on('session', (session) => { - - // Store the client ID in the session for identification - session.clientId = null; - - session.on('close', () => { - const terminationTime = Date.now(); - if (session.clientId) { - sessionTerminationTimes.set(session.clientId, terminationTime); - } - }); - - session.on('error', (err) => { - console.error(`Session error:`, err); - }); - }); - - // Server stream handler - server.on('stream', (stream, headers) => { - const clientId = headers['client-id']; - streamCount++; - - // Identify the session this stream belongs to - if (stream.session && !stream.session.clientId) { - stream.session.clientId = clientId; - } - - stream.respond({ - ':status': 200, - 'content-type': 'text/plain' - }); - - // Handle incoming data - stream.on('data', (chunk) => { - const data = chunk.toString().trim(); - - if (clientId === '1') { - writeCount1++; - } else if (clientId === '2') { - writeCount2++; - } - - stream.write(`Echo: ${data}`); - }); - - stream.on('end', () => {}); - - stream.on('error', (err) => { - console.error(`Stream ${stream.id} error:`, err); - }); - }); - - // Start the server - await new Promise((resolve) => server.listen(0, () => { - serverStart = Date.now(); - resolve(); - })); - - const port = server.address().port; - - // This client will send 10 writes at interval of 100ms and then close at 1000ms from server start. - const client1 = http2.connect(`https://localhost:${port}`, { - rejectUnauthorized: false - }); - - // This client will send 20 writes at interval of 100ms and then close at 2000ms from server start. - const client2 = http2.connect(`https://localhost:${port}`, { - rejectUnauthorized: false - }); - - // Create streams for both clients - createClientStream(client1, '1', 10, 100); - createClientStream(client2, '2', 20, 100); - - - // Wait for 1500ms before server close - await new Promise((resolve) => setTimeout(resolve, common.platformTimeout(1500))); - const serverCloseFireTime = Date.now(); - - // Fire server close after 1500ms of server start - await new Promise((resolve) => { - server.close(() => { - serverCloseTime = Date.now(); - resolve(); - }); - }); - - - // Wait for all server-side session terminations to complete - await new Promise((resolve) => setTimeout(resolve, common.platformTimeout(500))); - - // Verify test expectations - // Should have exactly 2 streams (one per client)' - assert.strictEqual(streamCount, 2); - // Server should have received 10 writes from client 1 - assert.strictEqual(writeCount1, 10); - // Server should have received 20 writes from client 2 - assert.strictEqual(writeCount2, 20); - - // Verify session termination times from server's perspective - assert.ok(sessionTerminationTimes.has('1'), 'Server should record termination of client 1 session'); - assert.ok(sessionTerminationTimes.has('2'), 'Server should record termination of client 2 session'); - - const client1TerminationTime = sessionTerminationTimes.get('1'); - const client2TerminationTime = sessionTerminationTimes.get('2'); - - // Client 1 session should terminate immediately after - // server close is fired as server immediately closes all idle sessions. - assert.ok( - client1TerminationTime - serverCloseTime < 100, - `Client 1 session should terminate shortly after server close (actual delay: ${client1TerminationTime - serverCloseTime}ms)` - ); - - // Client 2 session should terminate approximately 500ms after - // server close as client write till 2000ms from server start. - const client2Delay = client2TerminationTime - serverCloseFireTime; - assert.ok( - client2Delay >= 450 && client2Delay <= 650, - `Client 2 session should terminate ~500ms after server close (actual delay: ${client2Delay}ms)` - ); - assert.ok( - serverCloseTime - serverStart >= 2000, - `Server should fully close after 2000ms of server start when all streams complete (actual: ${serverCloseTime - serverStart}ms)` - ); - assert.ok( - (serverCloseTime - serverStart) - 2000 < 200, - `Server should fully close just after all streams complete` - ); - -}); diff --git a/test/parallel/test-http2-server-close-idle-connection.js b/test/parallel/test-http2-server-close-idle-connection.js new file mode 100644 index 00000000000000..7bf30cb049ce76 --- /dev/null +++ b/test/parallel/test-http2-server-close-idle-connection.js @@ -0,0 +1,100 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const assert = require('assert'); +const http2 = require('http2'); + +// This test verifies that the server closes idle connections + +// Track test state with flags +const states = { + serverListening: false, + initialRequestCompleted: false, + connectionBecameIdle: false, + serverClosedIdleConnection: false +}; + +const server = http2.createServer(); + +// Track server events +server.on('stream', common.mustCall((stream, headers) => { + assert.strictEqual(states.serverListening, true); + + // Respond to the request with a small payload + stream.respond({ + 'content-type': 'text/plain', + ':status': 200 + }); + stream.end('hello'); + + // After the request is done, the connection should become idle + stream.on('close', () => { + states.initialRequestCompleted = true; + // Mark connection as idle after the request completes + states.connectionBecameIdle = true; + }); +})); + +// Track session closure events +server.on('session', common.mustCall((session) => { + session.on('stream', common.mustCall((stream) => { + stream.on('close', common.mustCall(() => { + // This should only happen after the connection became idle + assert.strictEqual(states.connectionBecameIdle, true); + states.serverClosedIdleConnection = true; + + // Test is successful, close the server + server.close(common.mustCall(() => { + console.log('server closed'); + })); + })); + })); + session.on('close', common.mustCall(() => { + console.log('session closed'); + })); +})); + +// Start the server +server.listen(0, common.mustCall(() => { + states.serverListening = true; + + // Create client and initial request + const client = http2.connect(`http://localhost:${server.address().port}`); + + // Track client session events + client.on('close', common.mustCall(() => { + // Verify server closed the connection after it became idle + assert.strictEqual(states.serverClosedIdleConnection, true); + })); + + // Make an initial request + const req = client.request({ ':path': '/' }); + + req.on('response', common.mustCall()); + + // Process the response data + req.setEncoding('utf8'); + let data = ''; + req.on('data', (chunk) => { + data += chunk; + }); + + // When initial request ends + req.on('end', common.mustCall(() => { + assert.strictEqual(data, 'hello'); + + // Don't explicitly close the client - keep it idle + // Wait for the server to close the idle connection + })); + + client.on('close', common.mustCall()); + + req.end(); +})); + +// Final verification on exit +process.on('exit', () => { + assert.strictEqual(states.serverClosedIdleConnection, true); +}); From 6bc96f18d0b2388ef258d9279a7738ac29505d0e Mon Sep 17 00:00:00 2001 From: Kushagra Pandey Date: Sat, 29 Mar 2025 16:51:27 +0530 Subject: [PATCH 3/7] http2: fix cross-platform test timing issues Fix test-http2-server-http1-client.js failure on Ubuntu by deferring server.close() to next event loop cycle. The issue only affected Ubuntu where session close occurs before error emission, causing the test to miss errors when HTTP/1 clients connect to HTTP/2 servers. Using setImmediate() ensures error events fire before server close across all platforms while maintaining recent session handling improvements. --- test/parallel/test-http2-server-http1-client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-http2-server-http1-client.js b/test/parallel/test-http2-server-http1-client.js index 40e97f04a4e888..1a699c3194b59b 100644 --- a/test/parallel/test-http2-server-http1-client.js +++ b/test/parallel/test-http2-server-http1-client.js @@ -23,5 +23,5 @@ server.on('session', common.mustCall((session) => { server.listen(0, common.mustCall(() => { const req = http.get(`http://localhost:${server.address().port}`); - req.on('error', (error) => server.close()); + req.on('error', (error) => setImmediate(() => server.close())); })); From dd751ae3ccd4887b3ec65ec940108011817c70c7 Mon Sep 17 00:00:00 2001 From: Kushagra Pandey Date: Mon, 7 Apr 2025 01:22:19 +0530 Subject: [PATCH 4/7] resolved pr comments on test-http2-graceful-close.js --- test/parallel/test-http2-graceful-close.js | 70 ++++++---------------- 1 file changed, 17 insertions(+), 53 deletions(-) diff --git a/test/parallel/test-http2-graceful-close.js b/test/parallel/test-http2-graceful-close.js index 753f65d5fd7ea4..f611639cb8714b 100644 --- a/test/parallel/test-http2-graceful-close.js +++ b/test/parallel/test-http2-graceful-close.js @@ -9,65 +9,40 @@ const http2 = require('http2'); // This test verifies that the server waits for active connections/sessions // to complete and all data to be sent before fully closing -// Keep track of the test flow with these flags -const states = { - serverListening: false, - responseStarted: false, - dataFullySent: false, - streamClosed: false, - serverClosed: false -}; - // Create a server that will send a large response in chunks const server = http2.createServer(); // Track server events server.on('stream', common.mustCall((stream, headers) => { - assert.strictEqual(states.serverListening, true); - - // Setup the response - stream.respond({ - 'content-type': 'text/plain', - ':status': 200 - }); // Initiate the server close before client data is sent, this will // test if the server properly waits for the stream to finish - server.close(common.mustCall(() => { - // Stream should be closed before server close callback - // Should be called only after the stream has closed - assert.strictEqual(states.streamClosed, true); - states.serverClosed = true; - })); - - // Mark response as started - states.responseStarted = true; - - // Create a large response (1MB) to ensure it takes some time to send - const chunk = Buffer.alloc(64 * 1024, 'x'); + server.close(common.mustCall()); + setImmediate(() => { + stream.respond({ + 'content-type': 'text/plain', + ':status': 200 + }); - // Send 16 chunks (1MB total) to simulate a large response - for (let i = 0; i < 16; i++) { - stream.write(chunk); - } + // Create a large response (1MB) to ensure it takes some time to send + const chunk = Buffer.alloc(64 * 1024, 'x'); + // Send 16 chunks (1MB total) to simulate a large response + for (let i = 0; i < 16; i++) { + stream.write(chunk); + } - // Stream end should happen after data is written - stream.end(); - states.dataFullySent = true; + // Stream end should happen after data is written + stream.end(); + }); - // When stream closes, we can verify order of events stream.on('close', common.mustCall(() => { - // Data should be fully sent before stream closes - assert.strictEqual(states.dataFullySent, true); - states.streamClosed = true; + assert.strictEqual(stream.writableFinished, true); })); })); // Start the server server.listen(0, common.mustCall(() => { - states.serverListening = true; - // Create client and request const client = http2.connect(`http://localhost:${server.address().port}`); const req = client.request({ ':path': '/' }); @@ -79,23 +54,12 @@ server.listen(0, common.mustCall(() => { }); // When the request completes - req.on('end', common.mustCall(() => { - // All data should be sent before request ends - assert.strictEqual(states.dataFullySent, true); - })); + req.on('end', common.mustCall()); // When request closes req.on('close', common.mustCall(() => { - // Check final state - assert.strictEqual(states.streamClosed, true); // Should receive all data assert.strictEqual(receivedData, 64 * 1024 * 16); - - // Wait for the server close confirmation - process.on('exit', () => { - // Server should be fully closed - assert.strictEqual(states.serverClosed, true); - }); })); // Start the request From be55b6d47bd8d90f267025cc558714aefb252ad2 Mon Sep 17 00:00:00 2001 From: Kushagra Pandey Date: Mon, 7 Apr 2025 02:30:21 +0530 Subject: [PATCH 5/7] resolved pr comments on test-http2-server-close-idle-connection.js and strengthen test --- ...test-http2-server-close-idle-connection.js | 74 +++++-------------- 1 file changed, 17 insertions(+), 57 deletions(-) diff --git a/test/parallel/test-http2-server-close-idle-connection.js b/test/parallel/test-http2-server-close-idle-connection.js index 7bf30cb049ce76..87b040a987c8de 100644 --- a/test/parallel/test-http2-server-close-idle-connection.js +++ b/test/parallel/test-http2-server-close-idle-connection.js @@ -8,66 +8,30 @@ const http2 = require('http2'); // This test verifies that the server closes idle connections -// Track test state with flags -const states = { - serverListening: false, - initialRequestCompleted: false, - connectionBecameIdle: false, - serverClosedIdleConnection: false -}; - const server = http2.createServer(); -// Track server events -server.on('stream', common.mustCall((stream, headers) => { - assert.strictEqual(states.serverListening, true); - - // Respond to the request with a small payload - stream.respond({ - 'content-type': 'text/plain', - ':status': 200 - }); - stream.end('hello'); - - // After the request is done, the connection should become idle - stream.on('close', () => { - states.initialRequestCompleted = true; - // Mark connection as idle after the request completes - states.connectionBecameIdle = true; - }); -})); - -// Track session closure events server.on('session', common.mustCall((session) => { session.on('stream', common.mustCall((stream) => { + // Respond to the request with a small payload + stream.respond({ + 'content-type': 'text/plain', + ':status': 200 + }); + stream.end('hello'); stream.on('close', common.mustCall(() => { - // This should only happen after the connection became idle - assert.strictEqual(states.connectionBecameIdle, true); - states.serverClosedIdleConnection = true; - - // Test is successful, close the server - server.close(common.mustCall(() => { - console.log('server closed'); - })); + assert.strictEqual(stream.writableFinished, true); })); })); - session.on('close', common.mustCall(() => { - console.log('session closed'); - })); + session.on('close', common.mustCall()); })); // Start the server server.listen(0, common.mustCall(() => { - states.serverListening = true; - // Create client and initial request const client = http2.connect(`http://localhost:${server.address().port}`); - // Track client session events - client.on('close', common.mustCall(() => { - // Verify server closed the connection after it became idle - assert.strictEqual(states.serverClosedIdleConnection, true); - })); + // This will ensure that server closed the idle connection + client.on('close', common.mustCall()); // Make an initial request const req = client.request({ ':path': '/' }); @@ -84,17 +48,13 @@ server.listen(0, common.mustCall(() => { // When initial request ends req.on('end', common.mustCall(() => { assert.strictEqual(data, 'hello'); - - // Don't explicitly close the client - keep it idle - // Wait for the server to close the idle connection + // Close the server as client is idle now + setImmediate(() => { + server.close(common.mustCall()); + }); })); - client.on('close', common.mustCall()); - - req.end(); + // Don't explicitly close the client, we want to keep it + // idle and check if the server closes the idle connection. + // As this is what we want to test here. })); - -// Final verification on exit -process.on('exit', () => { - assert.strictEqual(states.serverClosedIdleConnection, true); -}); From 8ee2e3e2b094b74fbaa37f5798c36058fa5d19e6 Mon Sep 17 00:00:00 2001 From: Kushagra Pandey Date: Mon, 7 Apr 2025 23:46:30 +0530 Subject: [PATCH 6/7] resolved pr comments: done suggested changes from lpinca --- test/parallel/test-http2-graceful-close.js | 8 ++++---- test/parallel/test-http2-server-close-idle-connection.js | 4 +--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/test/parallel/test-http2-graceful-close.js b/test/parallel/test-http2-graceful-close.js index f611639cb8714b..6e518080f6aa71 100644 --- a/test/parallel/test-http2-graceful-close.js +++ b/test/parallel/test-http2-graceful-close.js @@ -17,7 +17,7 @@ server.on('stream', common.mustCall((stream, headers) => { // Initiate the server close before client data is sent, this will // test if the server properly waits for the stream to finish - server.close(common.mustCall()); + server.close(); setImmediate(() => { stream.respond({ 'content-type': 'text/plain', @@ -37,6 +37,7 @@ server.on('stream', common.mustCall((stream, headers) => { }); stream.on('close', common.mustCall(() => { + assert.strictEqual(stream.readableEnded, true); assert.strictEqual(stream.writableFinished, true); })); })); @@ -53,13 +54,12 @@ server.listen(0, common.mustCall(() => { receivedData += chunk.length; }); - // When the request completes - req.on('end', common.mustCall()); - // When request closes req.on('close', common.mustCall(() => { // Should receive all data + assert.strictEqual(req.readableEnded, true); assert.strictEqual(receivedData, 64 * 1024 * 16); + assert.strictEqual(req.writableFinished, true); })); // Start the request diff --git a/test/parallel/test-http2-server-close-idle-connection.js b/test/parallel/test-http2-server-close-idle-connection.js index 87b040a987c8de..56a94f2786b20e 100644 --- a/test/parallel/test-http2-server-close-idle-connection.js +++ b/test/parallel/test-http2-server-close-idle-connection.js @@ -36,8 +36,6 @@ server.listen(0, common.mustCall(() => { // Make an initial request const req = client.request({ ':path': '/' }); - req.on('response', common.mustCall()); - // Process the response data req.setEncoding('utf8'); let data = ''; @@ -50,7 +48,7 @@ server.listen(0, common.mustCall(() => { assert.strictEqual(data, 'hello'); // Close the server as client is idle now setImmediate(() => { - server.close(common.mustCall()); + server.close(); }); })); From 90acf086c184724566706037978f2fd63f58b039 Mon Sep 17 00:00:00 2001 From: Kushagra Pandey Date: Wed, 9 Apr 2025 16:33:55 +0530 Subject: [PATCH 7/7] resolved pr comment: updated for each loop to for...of loop --- lib/internal/http2/core.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 0cc4ead811716a..19b99995258bd2 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -3183,9 +3183,9 @@ function onErrorSecureServerSession(err, socket) { function closeAllSessions(server) { const sessions = server[kSessions]; if (sessions.size > 0) { - sessions.forEach((session) => { + for (const session of sessions) { session.close(); - }); + } } }