Node: kIncomingMessage undefined for socket.server

Created on 8 Mar 2018  Â·  5Comments  Â·  Source: nodejs/node

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)

confirmed-bug http

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.

All 5 comments

do you have the full code to reproduce?

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

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

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ksushilmaurya picture ksushilmaurya  Â·  3Comments

fanjunzhi picture fanjunzhi  Â·  3Comments

sandeepks1 picture sandeepks1  Â·  3Comments

vsemozhetbyt picture vsemozhetbyt  Â·  3Comments

seishun picture seishun  Â·  3Comments