kIncomingMessage undefined for socket.server

[leiyangyou]

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)

[gireeshpunathil]

do you have the full code to reproduce?

[leiyangyou]

Hi, please refer to

https://github.com/NetEase/pomelo/blob/5f7bec1c9be5fa2f88a366b15085cf64d4e786d7/lib/connectors/hybrid/wsprocessor.js#L30

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);
  }
};

[gireeshpunathil]

/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?

[leiyangyou]

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 ***@***.***> 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 <#19231 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGXzfrgaiIV20oPtXQLTRhTnITu10Ovks5tmzzdgaJpZM4Si3ca> .

[apapirovski]

@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.