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 - 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. - 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. PR-URL: https://github.com/nodejs/node/pull/57586 Fixes: https://github.com/nodejs/node/issues/57611 Refs: https://datatracker.ietf.org/doc/html/rfc7540#section-9.1 Refs: https://nodejs.org/api/http.html#serverclosecallback Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
152 lines
3.4 KiB
JavaScript
152 lines
3.4 KiB
JavaScript
'use strict';
|
|
|
|
const common = require('../common');
|
|
if (!common.hasCrypto)
|
|
common.skip('missing crypto');
|
|
|
|
const assert = require('assert');
|
|
const events = require('events');
|
|
const { createServer, connect } = require('http2');
|
|
|
|
events.captureRejections = true;
|
|
|
|
{
|
|
// Test error thrown in the server 'stream' event,
|
|
// after a respond()
|
|
|
|
const server = createServer();
|
|
server.on('stream', common.mustCall(async (stream) => {
|
|
server.close();
|
|
|
|
stream.respond({ ':status': 200 });
|
|
|
|
const _err = new Error('kaboom');
|
|
stream.on('error', common.mustCall((err) => {
|
|
assert.strictEqual(err, _err);
|
|
}));
|
|
throw _err;
|
|
}));
|
|
|
|
server.listen(0, common.mustCall(() => {
|
|
const { port } = server.address();
|
|
const session = connect(`http://localhost:${port}`);
|
|
|
|
const req = session.request();
|
|
|
|
req.on('error', common.mustCall((err) => {
|
|
assert.strictEqual(err.code, 'ERR_HTTP2_STREAM_ERROR');
|
|
}));
|
|
|
|
req.on('close', common.mustCall(() => {
|
|
session.close();
|
|
}));
|
|
}));
|
|
}
|
|
|
|
{
|
|
// Test error thrown in the server 'stream' event,
|
|
// before a respond().
|
|
|
|
const server = createServer();
|
|
server.on('stream', common.mustCall(async (stream) => {
|
|
server.close();
|
|
|
|
stream.on('error', common.mustNotCall());
|
|
|
|
throw new Error('kaboom');
|
|
}));
|
|
|
|
server.listen(0, common.mustCall(() => {
|
|
const { port } = server.address();
|
|
const session = connect(`http://localhost:${port}`);
|
|
|
|
const req = session.request();
|
|
|
|
req.on('response', common.mustCall((headers) => {
|
|
assert.strictEqual(headers[':status'], 500);
|
|
}));
|
|
|
|
req.on('close', common.mustCall(() => {
|
|
session.close();
|
|
}));
|
|
}));
|
|
}
|
|
|
|
{
|
|
// Test error thrown in 'request' event
|
|
|
|
const server = createServer(common.mustCall(async (req, res) => {
|
|
server.close();
|
|
res.setHeader('content-type', 'application/json');
|
|
const _err = new Error('kaboom');
|
|
throw _err;
|
|
}));
|
|
|
|
server.listen(0, common.mustCall(() => {
|
|
const { port } = server.address();
|
|
const session = connect(`http://localhost:${port}`);
|
|
|
|
const req = session.request();
|
|
|
|
req.on('response', common.mustCall((headers) => {
|
|
assert.strictEqual(headers[':status'], 500);
|
|
assert.strictEqual(Object.hasOwn(headers, 'content-type'), false);
|
|
}));
|
|
|
|
req.on('close', common.mustCall(() => {
|
|
session.close();
|
|
}));
|
|
|
|
req.resume();
|
|
}));
|
|
}
|
|
|
|
{
|
|
// Test error thrown in the client 'stream' event
|
|
|
|
const server = createServer();
|
|
server.on('stream', common.mustCall(async (stream) => {
|
|
const { port } = server.address();
|
|
|
|
stream.pushStream({
|
|
':scheme': 'http',
|
|
':path': '/foobar',
|
|
':authority': `localhost:${port}`,
|
|
}, common.mustCall((err, push) => {
|
|
push.respond({
|
|
'content-type': 'text/html',
|
|
':status': 200
|
|
});
|
|
push.end('pushed by the server');
|
|
|
|
stream.end('test');
|
|
}));
|
|
|
|
stream.respond({
|
|
':status': 200
|
|
});
|
|
|
|
server.close();
|
|
}));
|
|
|
|
server.listen(0, common.mustCall(() => {
|
|
const { port } = server.address();
|
|
const session = connect(`http://localhost:${port}`);
|
|
|
|
const req = session.request();
|
|
req.resume();
|
|
|
|
session.on('stream', common.mustCall(async (stream) => {
|
|
session.close();
|
|
|
|
const _err = new Error('kaboom');
|
|
stream.on('error', common.mustCall((err) => {
|
|
assert.strictEqual(err, _err);
|
|
}));
|
|
throw _err;
|
|
}));
|
|
|
|
req.end();
|
|
}));
|
|
}
|