Skip to content

Commit

Permalink
http: remove misleading warning
Browse files Browse the repository at this point in the history
There are cases where the `'clientError'` event can be emitted multiple
times, even if the socket is correctly destroyed.

Fixes: #51073
PR-URL: #51204
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
  • Loading branch information
lpinca authored and richardlau committed Mar 25, 2024
1 parent 61d1535 commit f7b53d0
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 18 deletions.
15 changes: 0 additions & 15 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -862,27 +862,12 @@ const requestChunkExtensionsTooLargeResponse = 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)) {
Expand Down
55 changes: 55 additions & 0 deletions test/parallel/test-http-server-multiple-client-error.js
Original file line number Diff line number Diff line change
@@ -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();
}));
4 changes: 1 addition & 3 deletions test/parallel/test-http-socket-error-listeners.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// Flags: --no-warnings

'use strict';

const common = require('../common');
Expand All @@ -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());

Expand Down

0 comments on commit f7b53d0

Please sign in to comment.