Server crashes with Invalid WebSocket frame

[pimvanderheijden]

Duplicate of closed issue: #1777

Description

The server crashes on an invalid WS frame. I have the impression this is not intended behaviour. I've been reading the code at Receiver.controlMessage and Receiver.receiverOnError but can't seem to find a reason for it to throw an Error besides emitting it. If I would know how to catch this error I could work around the problem.

Reproducible in:

• Node.js version(s): 12.18.3
• OS version(s): Ubuntu 20

Steps to reproduce:

Send a 1006 frame to the server

Expected result:

Server emits error

Actual result:

Error thrown and app crashes

Attachments:

events.js:291
      throw er; // Unhandled 'error' event
      ^

RangeError: Invalid WebSocket frame: invalid status code 1006
    at Receiver.controlMessage (/app/node_modules/ws/lib/receiver.js:464:18)
    at Receiver.getData (/app/node_modules/ws/lib/receiver.js:350:42)
    at Receiver.startLoop (/app/node_modules/ws/lib/receiver.js:143:22)
    at Receiver._write (/app/node_modules/ws/lib/receiver.js:78:10)
    at doWrite (_stream_writable.js:403:12)
    at writeOrBuffer (_stream_writable.js:387:5)
    at Receiver.Writable.write (_stream_writable.js:318:11)
    at Socket.socketOnData (/app/node_modules/ws/lib/websocket.js:900:35)
    at Socket.emit (events.js:314:20)
    at addChunk (_stream_readable.js:297:12)
    at readableAddChunk (_stream_readable.js:272:9)
    at Socket.Readable.push (_stream_readable.js:213:10)
    at TCP.onStreamRead (internal/stream_base_commons.js:188:23)

Emitted 'error' event on WebSocket instance at:
    at Receiver.receiverOnError (/app/node_modules/ws/lib/websocket.js:805:13)
    at Receiver.emit (events.js:314:20)
    at errorOrDestroy (internal/streams/destroy.js:108:12)
    at onwriteError (_stream_writable.js:418:5)
    at onwrite (_stream_writable.js:445:5)
    at Receiver.startLoop (/app/node_modules/ws/lib/receiver.js:152:5)
    at Receiver._write (/app/node_modules/ws/lib/receiver.js:78:10)
    [... lines matching original stack trace ...]
 {
  [Symbol(status-code)]: 1002
}

[lpinca]

This is expected. Add a listener for the 'error' event as per #1777 (comment).

[lpinca]

FYI, it's not the sever that emits the error but the server client (the WebSocket object received in the 'connection' event).

[pimvanderheijden]

@lpinca Thanks for having a look. Some context:

The problem was indeed in our application code. The event listeners — including the one for "error" — got removed as part of closing the WebSocket and cleaning up. The problem with that approach, however, is the frame the connection receives in response to the close frame 1001. If that frame causes an error, for example when it is 1006, the WebSocket is going to throw because the listener got removed already...

I solved it by cleaning up listeners only on "close" event. If the WebSocket does not get a "close" event somehow, then that can be a slight problem. The client could not close the connection (and keep doing ping / pong). Then it will stay open, even though the server has called .close() on the socket. I think the right approach is to stop doing ping/pong for that connection when calling .close(), all so that the socket will be terminated eventually after a ping / pong timeout is reached in application code.

[jamesopti]

This is expected. Add a listener for the 'error' event as per #1777 (comment).

@lpinca Is it necessary to remove this event listener via ws.off('error', cb)? Or is it safe to let the instance get destroyed with the listener attached?

[lpinca]

Or is it safe to let the instance get destroyed with the listener attached?

It is ok to keep it attached.

[jamesopti]

@lpinca Relatedly, do I need to ensure that the ws instance is destroyed when I receive this event? Or will it get destroyed automatically?

I looked through the README but couldnt find any documentation about the 'error' event.

[lpinca]

It is documented here: https://github.com/websockets/ws/blob/8.8.1/doc/ws.md#event-error-1.