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

File tree

3 files changed

+56
-18
lines changed

3 files changed

+56
-18
lines changed

‎lib/_http_server.js‎

Lines changed: 0 additions & 15 deletions
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-
functionwarnUnclosedSocket(){
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
functionsocketOnError(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)){
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
'use strict';
2+
constcommon=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+
constassert=require('assert');
8+
consthttp=require('http');
9+
constnet=require('net');
10+
11+
process.on('warning',common.mustNotCall());
12+
13+
functionsocketListener(socket){
14+
constfirstByte=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+
constnetServer=net.createServer(socketListener);
27+
consthttpServer=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+
constsocket=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‎

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

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

2220
constserver=http.createServer(common.mustNotCall());
2321

0 commit comments

Comments
(0)