From 76847bbef197fdd2b644e842ad607ffd60995aba Mon Sep 17 00:00:00 2001 From: Lorenzo Blasa Date: Wed, 16 Mar 2022 02:26:42 -0700 Subject: [PATCH] Do not emit errors when unable to extract client query Summary: ^ Usually, when launching Chrome, this error will take place a significat amount of times. By looking at the HTTP request, it is difficult to determine the origin other than being a Chrome extension. There's no actionable items in Flipper, so do not emit the error. Changelog: Do not emit errors when unable to extract client query Reviewed By: antonk52 Differential Revision: D34898603 fbshipit-source-id: fbeb2db7ec2914669192cbebc2e6b15464e31827 --- desktop/flipper-common/src/index.tsx | 1 + desktop/flipper-common/src/utils/errors.tsx | 8 ++++++++ .../src/comms/ServerWebSocket.tsx | 18 +++++++++++++----- .../comms/__tests__/ServerWebSocket.node.tsx | 19 ------------------- 4 files changed, 22 insertions(+), 24 deletions(-) diff --git a/desktop/flipper-common/src/index.tsx b/desktop/flipper-common/src/index.tsx index e5e1e1214..e014558f9 100644 --- a/desktop/flipper-common/src/index.tsx +++ b/desktop/flipper-common/src/index.tsx @@ -37,6 +37,7 @@ export { export { ConnectivityError, CancelledPromiseError, + UnableToExtractClientQueryError, UserUnauthorizedError, UserNotSignedInError, NoLongerConnectedToClientError, diff --git a/desktop/flipper-common/src/utils/errors.tsx b/desktop/flipper-common/src/utils/errors.tsx index 3eb9be44e..05f83e306 100644 --- a/desktop/flipper-common/src/utils/errors.tsx +++ b/desktop/flipper-common/src/utils/errors.tsx @@ -30,6 +30,14 @@ export function isConnectivityOrAuthError( ); } +export class UnableToExtractClientQueryError extends Error { + constructor(msg: string) { + super(msg); + this.name = 'UnableToExtractClientQueryError'; + } + name: 'UnableToExtractClientQueryError'; +} + export class CancelledPromiseError extends Error { constructor(msg: string) { super(msg); diff --git a/desktop/flipper-server-core/src/comms/ServerWebSocket.tsx b/desktop/flipper-server-core/src/comms/ServerWebSocket.tsx index 94cdf4d93..17a21bcdc 100644 --- a/desktop/flipper-server-core/src/comms/ServerWebSocket.tsx +++ b/desktop/flipper-server-core/src/comms/ServerWebSocket.tsx @@ -17,7 +17,7 @@ import WebSocket, { import {createServer as createHttpsServer} from 'https'; import {createServer as createHttpServer} from 'http'; import querystring from 'querystring'; -import {ClientQuery} from 'flipper-common'; +import {ClientQuery, UnableToExtractClientQueryError} from 'flipper-common'; import { assertNotNull, parseClientQuery, @@ -101,14 +101,22 @@ class ServerWebSocket extends ServerAdapter { try { this.onConnection(ws, request); } catch (error) { + // TODO: Investigate if we need to close the socket in the `error` listener + // DRI: @aigoncharov + ws.close(WSCloseCode.InternalError); + + if (error instanceof UnableToExtractClientQueryError) { + // If we are unable to extract the client query, do not emit an error. + // It cannot be determined if the client is legitimately trying to establish + // a connection with Flipper or some other process is trying to connect to + // the port currently used by Flipper. + return; + } // If an exception is thrown, an `error` event is not emitted automatically. // We need to explicitly handle the error and emit an error manually. // If we leave it unhanled, the process just dies // https://replit.com/@aigoncharov/WS-error-handling#index.js ws.emit('error', error); - // TODO: Investigate if we need to close the socket in the `error` listener - // DRI: @aigoncharov - ws.close(WSCloseCode.InternalError); } }, ); @@ -185,7 +193,7 @@ class ServerWebSocket extends ServerAdapter { console.warn( '[conn] Unable to extract the client query from the request URL.', ); - throw new Error( + throw new UnableToExtractClientQueryError( '[conn] Unable to extract the client query from the request URL.', ); } 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 3eaa85e6c..2a244bd75 100644 --- a/desktop/flipper-server-core/src/comms/__tests__/ServerWebSocket.node.tsx +++ b/desktop/flipper-server-core/src/comms/__tests__/ServerWebSocket.node.tsx @@ -100,25 +100,6 @@ describe('ServerWebSocket', () => { expect(mockSEListener.onError).toBeCalledTimes(0); // no onError triggered, as start throws already }); - test('calls listener onError if a connection attempt fails', async () => { - const mockSEListener = createMockSEListener(); - - server = new ServerWebSocket(mockSEListener); - - const port = await server.start(0); - - expect(mockSEListener.onError).toBeCalledTimes(0); - // We pass a conection URL without required params hoping that it is going to fail - wsClient = new WebSocket(`ws://localhost:${port}`); - await new Promise((resolve) => { - wsClient!.onclose = () => { - resolve(); - }; - }); - wsClient = undefined; - expect(mockSEListener.onError).toBeCalledTimes(1); - }); - test('calls listener onError if a message handling fails', async () => { const mockSEListener = createMockSEListener();