From c7a135962d5e9eb43b516349c4e298dd356c945f Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Thu, 21 Dec 2023 21:09:01 +0100 Subject: [PATCH] http: remove misleading warning There are cases where the `'clientError'` event can be emitted multiple times, even if the socket is correctly destroyed. Fixes: https://github.com/nodejs/node/issues/51073 PR-URL: https://github.com/nodejs/node/pull/51204 Reviewed-By: Matteo Collina Reviewed-By: Paolo Insogna --- lib/_http_server.js | 15 ----- .../test-http-server-multiple-client-error.js | 55 +++++++++++++++++++ .../test-http-socket-error-listeners.js | 4 +- 3 files changed, 56 insertions(+), 18 deletions(-) create mode 100644 test/parallel/test-http-server-multiple-client-error.js diff --git a/lib/_http_server.js b/lib/_http_server.js index 3e19f1ba78e7cc..2591ceb4194137 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -857,27 +857,12 @@ const requestHeaderFieldsTooLargeResponse = Buffer.from( 'Connection: close\r\n\r\n', 'ascii', ); -function warnUnclosedSocket() { - if (warnUnclosedSocket.emitted) { - return; - } - - warnUnclosedSocket.emitted = true; - process.emitWarning( - 'An error event has already been emitted on the socket. ' + - 'Please use the destroy method on the socket while handling ' + - "a 'clientError' event.", - ); -} - function socketOnError(e) { // Ignore further errors this.removeListener('error', socketOnError); if (this.listenerCount('error', noop) === 0) { this.on('error', noop); - } else { - warnUnclosedSocket(); } if (!this.server.emit('clientError', e, this)) { diff --git a/test/parallel/test-http-server-multiple-client-error.js b/test/parallel/test-http-server-multiple-client-error.js new file mode 100644 index 00000000000000..4ac41d295e3d3e --- /dev/null +++ b/test/parallel/test-http-server-multiple-client-error.js @@ -0,0 +1,55 @@ +'use strict'; +const common = require('../common'); + +// Test that the `'clientError'` event can be emitted multiple times even if the +// socket is correctly destroyed and that no warning is raised. + +const assert = require('assert'); +const http = require('http'); +const net = require('net'); + +process.on('warning', common.mustNotCall()); + +function socketListener(socket) { + const firstByte = socket.read(1); + if (firstByte === null) { + socket.once('readable', () => { + socketListener(socket); + }); + return; + } + + socket.unshift(firstByte); + httpServer.emit('connection', socket); +} + +const netServer = net.createServer(socketListener); +const httpServer = http.createServer(common.mustNotCall()); + +httpServer.once('clientError', common.mustCall((err, socket) => { + assert.strictEqual(err.code, 'HPE_INVALID_METHOD'); + assert.strictEqual(err.rawPacket.toString(), 'Q'); + socket.destroy(); + + httpServer.once('clientError', common.mustCall((err) => { + assert.strictEqual(err.code, 'HPE_INVALID_METHOD'); + assert.strictEqual( + err.rawPacket.toString(), + 'WE http://example.com HTTP/1.1\r\n\r\n' + ); + })); +})); + +netServer.listen(0, common.mustCall(() => { + const socket = net.createConnection(netServer.address().port); + + socket.on('connect', common.mustCall(() => { + socket.end('QWE http://example.com HTTP/1.1\r\n\r\n'); + })); + + socket.on('close', () => { + netServer.close(); + }); + + socket.resume(); +})); diff --git a/test/parallel/test-http-socket-error-listeners.js b/test/parallel/test-http-socket-error-listeners.js index f0d220a5d9337a..558bb35882b6ad 100644 --- a/test/parallel/test-http-socket-error-listeners.js +++ b/test/parallel/test-http-socket-error-listeners.js @@ -1,5 +1,3 @@ -// Flags: --no-warnings - 'use strict'; const common = require('../common'); @@ -17,7 +15,7 @@ const net = require('net'); { let i = 0; let socket; - process.on('warning', common.mustCall()); + process.on('warning', common.mustNotCall()); const server = http.createServer(common.mustNotCall());