Handle ws operations in a safe manner

Summary:
After doing some tests, any error thrown during ws events can result in flipper-server shutting down.

To avoid this, effectively, errors throughout should be properly handled.

The problem is that there's no guarantees this is or will be the case.

Instead, wrap the event handlers in a safe function execution wrapper. Any errors will be caught and logged.

Reviewed By: aigoncharov

Differential Revision: D37206923

fbshipit-source-id: 6f7cadc297ac39768030962c6eaadde55048fd21
This commit is contained in:
Lorenzo Blasa
2022-06-16 10:35:17 -07:00
committed by Facebook GitHub Bot
parent 2712ea3481
commit 8a1e484d0c

View File

@@ -19,13 +19,23 @@ import {
CompanionEventWebSocketMessage, CompanionEventWebSocketMessage,
} from 'flipper-common'; } from 'flipper-common';
import {FlipperServerImpl} from '../FlipperServerImpl'; import {FlipperServerImpl} from '../FlipperServerImpl';
import {WebSocketServer} from 'ws'; import {RawData, WebSocketServer} from 'ws';
import { import {
FlipperServerCompanion, FlipperServerCompanion,
FlipperServerCompanionEnv, FlipperServerCompanionEnv,
} from 'flipper-server-companion'; } from 'flipper-server-companion';
import {URLSearchParams} from 'url'; import {URLSearchParams} from 'url';
const safe = (f: () => void) => {
try {
f();
} catch (error) {
if (error instanceof Error) {
console.error(error.name, error.stack);
}
}
};
/** /**
* Attach and handle incoming messages from clients. * Attach and handle incoming messages from clients.
* @param flipperServer A FlipperServer instance. * @param flipperServer A FlipperServer instance.
@@ -42,7 +52,7 @@ export function attachSocketServer(
` ${req.socket.remoteAddress}:${req.socket.remotePort}`) || ` ${req.socket.remoteAddress}:${req.socket.remotePort}`) ||
''; '';
console.log(`Client connected${clientAddress}`); console.log('Client connected', clientAddress);
let connected = true; let connected = true;
@@ -117,7 +127,7 @@ export function attachSocketServer(
flipperServerCompanion?.onAny(onServerCompanionEvent); flipperServerCompanion?.onAny(onServerCompanionEvent);
client.on('message', (data) => { async function onClientMessage(data: RawData) {
let [event, payload]: [event: string | null, payload: any | null] = [ let [event, payload]: [event: string | null, payload: any | null] = [
null, null,
null, null,
@@ -205,20 +215,30 @@ export function attachSocketServer(
}); });
} }
} }
}
client.on('message', (data) => {
safe(() => onClientMessage(data));
}); });
async function onClientClose(error: Error | undefined = undefined) {
if (error) {
console.error(`Client disconnected ${clientAddress} with error`, error);
} else {
console.log(`Client disconnected ${clientAddress}`);
}
connected = false;
flipperServer.offAny(onServerEvent);
flipperServerCompanion?.destroyAll();
}
client.on('close', () => { client.on('close', () => {
console.log(`Client disconnected ${clientAddress}`); safe(() => onClientClose());
connected = false;
flipperServer.offAny(onServerEvent);
flipperServerCompanion?.destroyAll();
}); });
client.on('error', (e) => { client.on('error', (error) => {
console.error(`Socket error ${clientAddress}`, e); safe(() => onClientClose(error));
connected = false;
flipperServer.offAny(onServerEvent);
flipperServerCompanion?.destroyAll();
}); });
}); });
} }