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
This commit is contained in:
committed by
Facebook GitHub Bot
parent
cac6f85d17
commit
76847bbef1
@@ -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.',
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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<void>((resolve) => {
|
||||
wsClient!.onclose = () => {
|
||||
resolve();
|
||||
};
|
||||
});
|
||||
wsClient = undefined;
|
||||
expect(mockSEListener.onError).toBeCalledTimes(1);
|
||||
});
|
||||
|
||||
test('calls listener onError if a message handling fails', async () => {
|
||||
const mockSEListener = createMockSEListener();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user