From 71e9586247e91a307b1b401667c8e9f2bb42d932 Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Wed, 27 Oct 2021 18:33:41 +0200 Subject: [PATCH] fix(fastify-websocket): Handle connection and socket emitted errors --- src/__tests__/use.ts | 8 +++----- src/use/fastify-websocket.ts | 25 +++++++++++++++++++++---- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/__tests__/use.ts b/src/__tests__/use.ts index 60680118..7c7f04e0 100644 --- a/src/__tests__/use.ts +++ b/src/__tests__/use.ts @@ -28,7 +28,7 @@ afterAll(() => { console.error = consoleError; }); -for (const { tServer, itForWS, startTServer } of tServers) { +for (const { tServer, itForWS, skipUWS, startTServer } of tServers) { describe(tServer, () => { it('should allow connections with valid protocols only', async () => { const { url } = await startTServer(); @@ -405,8 +405,7 @@ for (const { tServer, itForWS, startTServer } of tServers) { }); // uWebSocket.js cannot have errors emitted on the socket - // TODO-db-211027 fastify-websocket - itForWS( + skipUWS( 'should report socket emitted errors to clients by closing the connection', async () => { const { url, waitForClient } = await startTServer(); @@ -429,8 +428,7 @@ for (const { tServer, itForWS, startTServer } of tServers) { ); // uWebSocket.js cannot have errors emitted on the socket - // TODO-db-211027 fastify-websocket - itForWS('should limit the socket emitted error message size', async () => { + skipUWS('should limit the socket emitted error message size', async () => { const { url, waitForClient } = await startTServer(); const client = await createTClient(url); diff --git a/src/use/fastify-websocket.ts b/src/use/fastify-websocket.ts index e21c907e..e3fcd990 100644 --- a/src/use/fastify-websocket.ts +++ b/src/use/fastify-websocket.ts @@ -46,12 +46,29 @@ export function makeHandler< return (connection, request) => { const { socket } = connection; - socket.on('error', (err) => + // used as listener on two streams, prevent superfluous calls on close + let emittedErrorHandled = false; + function handleEmittedError(err: Error) { + if (emittedErrorHandled) return; + emittedErrorHandled = true; + console.error( + 'Internal error emitted on a WebSocket socket. ' + + 'Please check your implementation.', + err, + ); socket.close( CloseCode.InternalServerError, - isProd ? 'Internal server error' : err.message, - ), - ); + // close reason should fit in one frame https://datatracker.ietf.org/doc/html/rfc6455#section-5.2 + isProd || err.message.length > 123 + ? 'Internal server error' + : err.message, + ); + } + + // fastify-websocket uses the WebSocket.createWebSocketStream, + // therefore errors get emitted on both the connection and the socket + connection.on('error', handleEmittedError); + socket.on('error', handleEmittedError); // keep alive through ping-pong messages let pongWait: NodeJS.Timeout | null = null;