after upgrading to node 9.7 some existing code started crashing when calling socket.emit('data', data)
the socket was created by net/Server, and later added to an httpServer through httpServer.emit('connection', socket)
I understand that the usage may be a bit hacky, but this was how some third party library implemented a hybrid server (one that takes both tcp & ws connections)
do you have the full code to reproduce?
Hi, please refer to
the socket was initially created by a net/Server & bound to the server, which does not have a kIncomingMessage defined
Processor.prototype.add = function(socket, data) {
if(this.state !== ST_STARTED) {
return;
}
this.httpServer.emit('connection', socket);
if(typeof socket.ondata === 'function') {
// compatible with stream2
socket.ondata(data, 0, data.length);
} else {
// compatible with old stream
socket.emit('data', data);
}
};
my current hacky fix involves doing this
Processor.prototype.add = function(socket, data) {
if(this.state !== ST_STARTED) {
return;
}
socket.server = this.httpServer
this.httpServer.emit('connection', socket);
if(typeof socket.ondata === 'function') {
// compatible with stream2
socket.ondata(data, 0, data.length);
} else {
// compatible with old stream
socket.emit('data', data);
}
};
/cc @nodejs/http - is the hybrid use case (mixing net' socket to be fused into the http server and the problems arises there on) supported?
Yes that’s the case. A net socket is reused for http when it detects that
the incoming data looks like http
On Mon, 9 Apr 2018 at 18:50, Gireesh Punathil notifications@github.com
wrote:
/cc @nodejs/http https://github.com/orgs/nodejs/teams/http - is the
hybrid use case (mixing net' socket to be fused into the http server and
the problems arises there on) supported?—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
https://github.com/nodejs/node/issues/19231#issuecomment-379711676, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAGXzfrgaiIV20oPtXQLTRhTnITu10Ovks5tmzzdgaJpZM4Si3ca
.
@gireeshpunathil I don't think it's meant to be supported in the way that it's used here but it did work until now... We should probably resolve.
Most helpful comment
@gireeshpunathil I don't think it's meant to be supported in the way that it's used here but it did work until now... We should probably resolve.