Skip to content

Commit f7b53d0

Browse files
lpincarichardlau
authored andcommitted
http: remove misleading warning
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 <[email protected]> Reviewed-By: Paolo Insogna <[email protected]>
1 parent 61d1535 commit f7b53d0

3 files changed

+56
-18
lines changed

lib/_http_server.js

-15
Original file line numberDiff line numberDiff line change
@@ -862,27 +862,12 @@ const requestChunkExtensionsTooLargeResponse = Buffer.from(
862862
'Connection: close\r\n\r\n', 'ascii',
863863
);
864864

865-
function warnUnclosedSocket() {
866-
if (warnUnclosedSocket.emitted) {
867-
return;
868-
}
869-
870-
warnUnclosedSocket.emitted = true;
871-
process.emitWarning(
872-
'An error event has already been emitted on the socket. ' +
873-
'Please use the destroy method on the socket while handling ' +
874-
"a 'clientError' event.",
875-
);
876-
}
877-
878865
function socketOnError(e) {
879866
// Ignore further errors
880867
this.removeListener('error', socketOnError);
881868

882869
if (this.listenerCount('error', noop) === 0) {
883870
this.on('error', noop);
884-
} else {
885-
warnUnclosedSocket();
886871
}
887872

888873
if (!this.server.emit('clientError', e, this)) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
'use strict';
2+
const common = require('../common');
3+
4+
// Test that the `'clientError'` event can be emitted multiple times even if the
5+
// socket is correctly destroyed and that no warning is raised.
6+
7+
const assert = require('assert');
8+
const http = require('http');
9+
const net = require('net');
10+
11+
process.on('warning', common.mustNotCall());
12+
13+
function socketListener(socket) {
14+
const firstByte = socket.read(1);
15+
if (firstByte === null) {
16+
socket.once('readable', () => {
17+
socketListener(socket);
18+
});
19+
return;
20+
}
21+
22+
socket.unshift(firstByte);
23+
httpServer.emit('connection', socket);
24+
}
25+
26+
const netServer = net.createServer(socketListener);
27+
const httpServer = http.createServer(common.mustNotCall());
28+
29+
httpServer.once('clientError', common.mustCall((err, socket) => {
30+
assert.strictEqual(err.code, 'HPE_INVALID_METHOD');
31+
assert.strictEqual(err.rawPacket.toString(), 'Q');
32+
socket.destroy();
33+
34+
httpServer.once('clientError', common.mustCall((err) => {
35+
assert.strictEqual(err.code, 'HPE_INVALID_METHOD');
36+
assert.strictEqual(
37+
err.rawPacket.toString(),
38+
'WE http://example.com HTTP/1.1\r\n\r\n'
39+
);
40+
}));
41+
}));
42+
43+
netServer.listen(0, common.mustCall(() => {
44+
const socket = net.createConnection(netServer.address().port);
45+
46+
socket.on('connect', common.mustCall(() => {
47+
socket.end('QWE http://example.com HTTP/1.1\r\n\r\n');
48+
}));
49+
50+
socket.on('close', () => {
51+
netServer.close();
52+
});
53+
54+
socket.resume();
55+
}));

test/parallel/test-http-socket-error-listeners.js

+1-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
// Flags: --no-warnings
2-
31
'use strict';
42

53
const common = require('../common');
@@ -17,7 +15,7 @@ const net = require('net');
1715
{
1816
let i = 0;
1917
let socket;
20-
process.on('warning', common.mustCall());
18+
process.on('warning', common.mustNotCall());
2119

2220
const server = http.createServer(common.mustNotCall());
2321

0 commit comments

Comments
 (0)