search

mercurius-js / mercurius

Implement GraphQL servers and gateways with Fastify
https://mercurius.dev/
MIT License
2.34k stars 234 forks source link

Unhandled errors in subscriptions crashes server. #430

Open SkeLLLa opened 3 years ago

SkeLLLa commented 3 years ago

I've recently had some errors in logs after which the whole server got crashed.

Here's an example:

events.js:292
      throw er; // Unhandled 'error' event
      ^
RangeError: Invalid WebSocket frame: invalid status code 21021
    at Receiver.controlMessage (/opt/service/node_modules/ws/lib/receiver.js:464:18)
    at Receiver.getData (/opt/service/node_modules/ws/lib/receiver.js:350:42)
    at Receiver.startLoop (/opt/service/node_modules/ws/lib/receiver.js:143:22)
    at Receiver._write (/opt/service/node_modules/ws/lib/receiver.js:78:10)
    at writeOrBuffer (internal/streams/writable.js:358:12)
    at Receiver.Writable.write (internal/streams/writable.js:303:10)
    at Socket.socketOnData (/opt/service/node_modules/ws/lib/websocket.js:900:35)
    at Socket.emit (events.js:315:20)
    at addChunk (internal/streams/readable.js:309:12)
    at readableAddChunk (internal/streams/readable.js:284:9)
Emitted 'error' event on Duplex instance at:
    at Duplex.duplexOnError (/opt/service/node_modules/ws/lib/stream.js:37:10)
    at Duplex.emit (events.js:315:20)
    at emitErrorNT (internal/streams/destroy.js:106:8)
    at emitErrorCloseNT (internal/streams/destroy.js:74:3)
    at processTicksAndRejections (internal/process/task_queues.js:80:21) {
  [Symbol(status-code)]: 1002
}

Is there any way to add custom error handler to it, to log more details?

mcollina commented 3 years ago

Looks like a bug somewhere between mercurius and fastify-websocket.

You can get the websocket via https://github.com/websockets/ws/blob/HEAD/doc/ws.md#event-connection, you should have the websocket server exposed via https://github.com/fastify/fastify-websocket/blob/c99252b99d2ed23ca804d85a75805f8c3e199d96/index.js#L37. If you register fastify-websocket before mercurius, your instance will be used instead. Look at https://github.com/mercurius-js/mercurius/blob/6740402d9cfbde237169b0cd31d2fe348f912a28/lib/subscription.js#L63-L72 for more details.

Can you write a small repro?

SkeLLLa commented 3 years ago

Yes, I'll try to create a reproduction. So far I can tell that bug might be connected to create react app built-in proxy, because before such errors like

"path":"/sockjs-node","msg":"closed incoming websocket connection for path with no websocket handler"