diff --git a/desktop/flipper-server-core/src/comms/ServerWebSocket.tsx b/desktop/flipper-server-core/src/comms/ServerWebSocket.tsx index 49b4a0b55..6960191eb 100644 --- a/desktop/flipper-server-core/src/comms/ServerWebSocket.tsx +++ b/desktop/flipper-server-core/src/comms/ServerWebSocket.tsx @@ -170,15 +170,17 @@ class ServerWebSocket extends ServerAdapter { this.handleConnectionAttempt(ctx); ws.on('message', async (message: WebSocket.RawData) => { + const messageString = message.toString(); try { - const messageString = message.toString(); const parsedMessage = this.handleMessageDeserialization(messageString); // Successful deserialization is a proof that the message is a string this.handleMessage(ctx, parsedMessage, messageString); } catch (error) { - // See the reasoning in the error handler for a `connection` event - ws.emit('error', error); - ws.close(WSCloseCode.InternalError); + // If handling an individual message failes, we don't necessarily kill the connection, + // all other plugins might still be working correctly. So let's just report it. + // This avoids ping-ponging connections if an individual plugin sends garbage (e.g. T129428800) + // or throws an error when handling messages + console.error('Failed to handle message', messageString, error); } }); } diff --git a/desktop/flipper-server-core/src/comms/__tests__/ServerWebSocket.node.tsx b/desktop/flipper-server-core/src/comms/__tests__/ServerWebSocket.node.tsx index 2a244bd75..8b2c57a6e 100644 --- a/desktop/flipper-server-core/src/comms/__tests__/ServerWebSocket.node.tsx +++ b/desktop/flipper-server-core/src/comms/__tests__/ServerWebSocket.node.tsx @@ -100,7 +100,7 @@ describe('ServerWebSocket', () => { expect(mockSEListener.onError).toBeCalledTimes(0); // no onError triggered, as start throws already }); - test('calls listener onError if a message handling fails', async () => { + test('does NOT call listener onError if individual message handling fails', async () => { const mockSEListener = createMockSEListener(); server = new ServerWebSocket(mockSEListener); @@ -117,9 +117,8 @@ describe('ServerWebSocket', () => { // Sending invalid JSON to cause a parsing error wsClient.send(`{{{{`); // Server must close the connection on error - await new Promise((resolve) => { - wsClient!.onclose = resolve; - }); - expect(mockSEListener.onError).toBeCalledTimes(1); + + expect(mockSEListener.onError).toBeCalledTimes(0); + expect(mockSEListener.onListening).toBeCalledTimes(1); }); });