From 8c67b049abc883f599a5420358039ca70d781df5 Mon Sep 17 00:00:00 2001 From: Lorenzo Blasa Date: Tue, 21 Jun 2022 12:48:43 -0700 Subject: [PATCH] Attach connection handler earlier Summary: This change attaches our event handlers as soon as the ws is created. As a consequence, we need to wait until the server has created any necessary instances required to process incoming requests. To achieve this, I created a type called `Lazy`. This type wraps around a value and a promise to that value. Callers can check if the value is set. If not, callers can wait for it. Ultimately, the value can be set outside of the promise itself. Reviewed By: passy Differential Revision: D37284939 fbshipit-source-id: 17dec548d7155a3d65440c9584cec07cbb826c37 --- desktop/app/src/init.tsx | 11 ++-- .../src/server/attachSocketServer.tsx | 11 ++-- .../src/server/startServer.tsx | 54 ++++++++++++++----- desktop/flipper-server/src/index.tsx | 10 ++-- 4 files changed, 53 insertions(+), 33 deletions(-) diff --git a/desktop/app/src/init.tsx b/desktop/app/src/init.tsx index a18b45359..403ddec91 100644 --- a/desktop/app/src/init.tsx +++ b/desktop/app/src/init.tsx @@ -16,7 +16,6 @@ import { } from 'flipper-plugin'; import {createFlipperServer, FlipperServerState} from 'flipper-frontend-core'; import { - attachSocketServer, FlipperServerImpl, getEnvironmentInfo, getGatekeepers, @@ -93,13 +92,13 @@ async function getFlipperServer( if (!(await checkSocketInUse(socketPath))) { console.info('flipper-server: not running/listening, start'); - const {socket} = await startServer({ + const {readyForIncomingConnections} = await startServer({ port: 52342, staticDir: staticPath, entry: 'index.web.dev.html', }); - const flipperServer = await startFlipperServer( + const server = await startFlipperServer( appPath, staticPath, '', @@ -107,10 +106,10 @@ async function getFlipperServer( keytar, ); - const companionEnv = await initCompanionEnv(flipperServer); - await flipperServer.connect(); + const companionEnv = await initCompanionEnv(server); + await server.connect(); - attachSocketServer(flipperServer, socket, companionEnv); + await readyForIncomingConnections(server, companionEnv); } else { console.info('flipper-server: already running'); const loggerOutputFile = 'flipper-server-log.out'; diff --git a/desktop/flipper-server-core/src/server/attachSocketServer.tsx b/desktop/flipper-server-core/src/server/attachSocketServer.tsx index ec59e3156..3f91b81b8 100644 --- a/desktop/flipper-server-core/src/server/attachSocketServer.tsx +++ b/desktop/flipper-server-core/src/server/attachSocketServer.tsx @@ -42,8 +42,8 @@ const safe = (f: () => void) => { * @param socket A ws socket on which to listen for events. */ export function attachSocketServer( - flipperServer: FlipperServerImpl, socket: WebSocketServer, + server: FlipperServerImpl, companionEnv: FlipperServerCompanionEnv, ) { socket.on('connection', (client, req) => { @@ -53,7 +53,6 @@ export function attachSocketServer( ''; console.log('Client connected', clientAddress); - let connected = true; let flipperServerCompanion: FlipperServerCompanion | undefined; @@ -62,7 +61,7 @@ export function attachSocketServer( if (params.get('server_companion')) { flipperServerCompanion = new FlipperServerCompanion( - flipperServer, + server, getLogger(), companionEnv.pluginInitializer.initialPlugins, ); @@ -112,7 +111,7 @@ export function attachSocketServer( client.send(JSON.stringify(message)); } - flipperServer.onAny(onServerEvent); + server.onAny(onServerEvent); async function onServerCompanionEvent(event: string, payload: any) { const message = { @@ -167,7 +166,7 @@ export function attachSocketServer( const execRes = flipperServerCompanion?.canHandleCommand(command) ? flipperServerCompanion.exec(command, ...args) - : flipperServer.exec(command, ...args); + : server.exec(command, ...args); execRes .then((result: any) => { @@ -229,7 +228,7 @@ export function attachSocketServer( } connected = false; - flipperServer.offAny(onServerEvent); + server.offAny(onServerEvent); flipperServerCompanion?.destroyAll(); } diff --git a/desktop/flipper-server-core/src/server/startServer.tsx b/desktop/flipper-server-core/src/server/startServer.tsx index 876c98b2f..d14eb6eef 100644 --- a/desktop/flipper-server-core/src/server/startServer.tsx +++ b/desktop/flipper-server-core/src/server/startServer.tsx @@ -20,6 +20,9 @@ import {makeSocketPath, checkSocketInUse} from './utilities'; import proxy from 'http-proxy'; import exitHook from 'exit-hook'; +import {attachSocketServer} from './attachSocketServer'; +import {FlipperServerImpl} from '../FlipperServerImpl'; +import {FlipperServerCompanionEnv} from 'flipper-server-companion'; type Config = { port: number; @@ -27,6 +30,11 @@ type Config = { entry: string; }; +type ReadyForConnections = ( + server: FlipperServerImpl, + companionEnv: FlipperServerCompanionEnv, +) => Promise; + /** * Orchestrates the creation of the HTTP server, proxy, and web socket. * @param config Server configuration. @@ -36,14 +44,9 @@ export async function startServer(config: Config): Promise<{ app: Express; server: http.Server; socket: WebSocketServer; + readyForIncomingConnections: ReadyForConnections; }> { - const {app, server} = await startHTTPServer(config); - const socket = addWebsocket(server, config); - return { - app, - server, - socket, - }; + return await startHTTPServer(config); } /** @@ -52,9 +55,12 @@ export async function startServer(config: Config): Promise<{ * @param config Server configuration. * @returns A promise to both app and HTTP server. */ -async function startHTTPServer( - config: Config, -): Promise<{app: Express; server: http.Server}> { +async function startHTTPServer(config: Config): Promise<{ + app: Express; + server: http.Server; + socket: WebSocketServer; + readyForIncomingConnections: ReadyForConnections; +}> { const app = express(); app.use((_req, res, next) => { @@ -89,8 +95,14 @@ async function startHTTPServer( async function startProxyServer( config: Config, app: Express, -): Promise<{app: Express; server: http.Server}> { +): Promise<{ + app: Express; + server: http.Server; + socket: WebSocketServer; + readyForIncomingConnections: ReadyForConnections; +}> { const server = http.createServer(app); + const socket = addWebsocket(server, config); // For now, we only support domain socket access on POSIX-like systems. // On Windows, a proxy is not created and the server starts @@ -98,7 +110,12 @@ async function startProxyServer( if (os.platform() === 'win32') { return new Promise((resolve) => { console.log(`Starting server on http://localhost:${config.port}`); - server.listen(config.port, undefined, () => resolve({app, server})); + const readyForIncomingConnections = (): Promise => { + return new Promise((resolve) => { + server.listen(config.port, undefined, () => resolve()); + }); + }; + resolve({app, server, socket, readyForIncomingConnections}); }); } @@ -140,8 +157,17 @@ async function startProxyServer( }); return new Promise((resolve) => { - proxyServer.listen(config.port); - server.listen(socketPath, undefined, () => resolve({app, server})); + const readyForIncomingConnections = ( + serverImpl: FlipperServerImpl, + companionEnv: FlipperServerCompanionEnv, + ): Promise => { + attachSocketServer(socket, serverImpl, companionEnv); + return new Promise((resolve) => { + proxyServer.listen(config.port); + server.listen(socketPath, undefined, () => resolve()); + }); + }; + resolve({app, server, socket, readyForIncomingConnections}); }); } diff --git a/desktop/flipper-server/src/index.tsx b/desktop/flipper-server/src/index.tsx index 3ea4e3281..76db80518 100644 --- a/desktop/flipper-server/src/index.tsx +++ b/desktop/flipper-server/src/index.tsx @@ -16,11 +16,7 @@ import fs from 'fs-extra'; import yargs from 'yargs'; import open from 'open'; import {initCompanionEnv} from 'flipper-server-companion'; -import { - attachSocketServer, - startFlipperServer, - startServer, -} from 'flipper-server-core'; +import {startFlipperServer, startServer} from 'flipper-server-core'; import {isTest} from 'flipper-common'; const argv = yargs @@ -97,7 +93,7 @@ async function start() { console.error('Failed to load keytar:', e); } - const {app, server, socket} = await startServer({ + const {app, server, socket, readyForIncomingConnections} = await startServer({ port: argv.port, staticDir, entry: 'index.web.dev.html', @@ -123,7 +119,7 @@ async function start() { if (argv.bundler) { await attachDevServer(app, server, socket, rootDir); } - attachSocketServer(flipperServer, socket, companionEnv); + await readyForIncomingConnections(flipperServer, companionEnv); } start()