fix(fastify-websocket): Handle connection and socket emitted errors

ilijaNL · Oct 27, 2021 · 71e9586 · 71e9586
1 parent 62d0b3d
commit 71e9586
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 9 deletions.
diff --git 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
@@ -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;