Repository navigation
node 8.12 net,http module emit connection bug. TypeError: ParserIncomingMessage is not a constructor . #22857
Description
- Version: v8.12.0
- Platform: linux ,windows
- Subsystem:
Here is the code that can be reproduced:
const http = require('http');
const net = require('net');
const httpserver = http.createServer((req,res)=>{
console.log('~~ http request');
res.end('aaa');
})
httpserver.on('connection',(socket)=>{
console.log('httpsever connection');
})
const netserver = net.createServer((socket)=>{
console.log('netserver connection');
httpserver.emit('connection',socket);
// This listener is not important.
socket.once('data',(data)=>{
console.log('netserver data',data.toString());
// This line of code is not important.
socket.emit('data',data);
});
}).listen(30332);
The same code run in v10.9.0 and v8.10.0 has been tested without issue.
This is a bug ,because of http_event_connection.
Note: This event can also be explicitly emitted by users to inject connections into the HTTP server. In that case, any Duplex stream can be passed.
Activity
[-]node 8.12 net,http module bug. TypeError: ParserIncomingMessage is not a constructor .[/-][+]node 8.12 net,http module emit `connection` bug. TypeError: ParserIncomingMessage is not a constructor .[/+]whtiehack commentedon Sep 14, 2018
Temporary solution
socket.server = null;tonyjansan commentedon Sep 15, 2018
[2018-09-15 13:10:24.637] [ERROR] console - Caught exception: TypeError: ParserIncomingMessage is not a constructor
at HTTPParser.parserOnHeadersComplete (_http_common.js:81:21)
at socketOnData (_http_server.js:470:20)
at emitOne (events.js:116:13)
at Socket.emit (events.js:211:7)
like some problem, seems critical issue!!!
AyushG3112 commentedon Sep 15, 2018
cc @nodejs/http
Offending line:
https://github.com/nodejs/node/blob/master/lib/_http_common.js#L86
Looks like it should be
But I'm not 100% sure
Travis: Stick with node.js 8.11
apapirovski commentedon Sep 15, 2018
The problem is simply one of backporting #20029 to v8.x. I'll take care of it later today if no one gets there first. (Surprised that one slipped by.)
Travis: Stick with node.js 8.11
Travis: Stick with node.js 8.11
alexjeffburke commentedon Sep 23, 2018
Hi, sorry to nudge but is there any progress on getting a fix in - I assume that #22880 is the one to watch? Since this was a regression introduced in the active LTS release, are there plans to do an 8.12.1 release? Thanks.
lpinca commentedon Sep 25, 2018
cc: @nodejs/lts
MylesBorins commentedon Sep 25, 2018
We don't currently have a timeline for the next 8.x release but it could be escalated if this problem is serious enough. We could potentially get a smaller release out in the next 2 - 3 unless this is deemed to be a severe bug, in which case we could escalate a hotfix
papandreou commentedon Sep 25, 2018
2 - 3 days or weeks? :)
MylesBorins commentedon Sep 25, 2018
sorry for missing a word
We generally do a 2 week r.c. process for new LTS releases, and we have not yet backported stuff. So unless this is deemed a severe bug which would escalate a release we wouldn't see a release for at least 2 - 3 weeks.
whtiehack commentedon Sep 26, 2018
@MylesBorins
This problem is not a serious problem, just using the latest version of node, many libraries can not be used. For example pomelo, pinus and mitm. Therefore, using these libraries requires a rollback of the node version.
This problem has been repeated many times before and after, I think we should look for reasons to avoid recurring in the future.
Thanks.
MylesBorins commentedon Sep 26, 2018
@whtiehack we can add them to CITGM so we don't hit this edge case again
dougwilson commentedon Sep 26, 2018
FWIW this is a documented feature:
https://nodejs.org/dist/latest-v8.x/docs/api/http.html#http_event_connection
It would seem like the Node.js test suite itself should catch the issue instead of relying on CITGM to catch regressions in documented features.
Not saying that it's not worth adding to CIGTM, but maybe someone wants to contribute a test directly.
lpinca commentedon Feb 10, 2019
Fixed by e3bddeec18. Closing.