Decouple Metro device handling from Flipper core

Summary: Decoupled metro 'device' from Redux store. Extracting some commonalities with Android device management up into FlipperServer

Reviewed By: timur-valiev

Differential Revision: D30309256

fbshipit-source-id: 1a9ac01e3f21d2d08761554d3644a7ae8d00a93e

desktop/app/src/dispatcher/flipperServer.tsx

@@ -17,7 +17,7 @@ import {notification} from 'antd';
 
 export default async (store: Store, logger: Logger) => {
   const {enableAndroid, androidHome} = store.getState().settingsState;
-  const server = await startFlipperServer(
+  const server = startFlipperServer(
     {
       enableAndroid,
       androidHome,
@@ -40,7 +40,7 @@ export default async (store: Store, logger: Logger) => {
     });
   });
 
-  server.on('server-start-error', (err) => {
+  server.on('server-error', (err) => {
     notification.error({
       message: 'Failed to start connection server',
       description:
@@ -58,7 +58,7 @@ export default async (store: Store, logger: Logger) => {
             {'' + err}
           </>
         ) : (
-          <>Failed to start connection server: ${err.message}</>
+          <>Failed to start Flipper server: ${err.message}</>
         ),
       duration: null,
     });
@@ -87,7 +87,7 @@ export default async (store: Store, logger: Logger) => {
       os: device.os,
       serial: device.serial,
     });
-    // N.B.: note that we don't remove the device, we keep it in offline mode!
+    // N.B.: note that we don't remove the device, we keep it in offline
   });
 
   server.on('client-connected', (payload) =>
@@ -100,6 +100,21 @@ export default async (store: Store, logger: Logger) => {
     });
   }
 
+  server
+    .waitForServerStarted()
+    .then(() => {
+      console.log(
+        'Flipper server started and accepting device / client connections',
+      );
+    })
+    .catch((e) => {
+      console.error('Failed to start Flipper server', e);
+      notification.error({
+        message: 'Failed to start Flipper server',
+        description: 'error: ' + e,
+      });
+    });
+
   return () => {
     server.close();
   };

desktop/app/src/sandy-chrome/appinspect/__tests__/PluginList.spec.tsx

@@ -17,7 +17,6 @@ import BaseDevice from '../../../server/devices/BaseDevice';
 import {_SandyPluginDefinition} from 'flipper-plugin';
 import {TestUtils} from 'flipper-plugin';
 import {selectPlugin} from '../../../reducers/connections';
-import {registerMetroDevice} from '../../../server/devices/metro/metroDeviceManager';
 import {
   addGatekeepedPlugins,
   registerMarketplacePlugins,
@@ -78,7 +77,10 @@ describe('basic getActiveDevice with metro present', () => {
     };
     testDevice = flipper.device;
     // flipper.store.dispatch(registerPlugins([LogsPlugin]))
-    await registerMetroDevice(undefined, flipper.store, flipper.logger);
+    flipper.store.dispatch({
+      type: 'REGISTER_DEVICE',
+      payload: new MetroDevice('http://localhost:8081', undefined),
+    });
     metro = getMetroDevice(flipper.store.getState())!;
     metro.supportsPlugin = (p) => {
       return p.id !== 'unsupportedDevicePlugin';

desktop/app/src/server/FlipperServer.tsx

@@ -30,7 +30,8 @@ import desktopDevice from './devices/desktop/desktopDeviceManager';
 import BaseDevice from './devices/BaseDevice';
 
 type FlipperServerEvents = {
-  'server-start-error': any;
+  'server-state': {state: ServerState; error?: Error};
+  'server-error': any;
   notification: {
     type: 'error';
     title: string;
@@ -47,17 +48,18 @@ export interface FlipperServerConfig {
   serverPorts: ServerPorts;
 }
 
-export async function startFlipperServer(
+export function startFlipperServer(
   config: FlipperServerConfig,
   store: Store,
   logger: Logger,
-): Promise<FlipperServer> {
+): FlipperServer {
   const server = new FlipperServer(config, store, logger);
-
+  server.start();
-  await server.start();
   return server;
 }
 
+type ServerState = 'pending' | 'starting' | 'started' | 'error' | 'closed';
+
 /**
  * FlipperServer takes care of all incoming device & client connections.
  * It will set up managers per device type, and create the incoming
@@ -70,7 +72,8 @@ export class FlipperServer {
   private readonly events = new EventEmitter();
   readonly server: ServerController;
   readonly disposers: ((() => void) | void)[] = [];
-
+  private readonly devices = new Map<string, BaseDevice>();
+  state: ServerState = 'pending';
   android: AndroidDeviceManager;
 
   // TODO: remove store argument
@@ -84,8 +87,50 @@ export class FlipperServer {
     this.android = new AndroidDeviceManager(this);
   }
 
+  setServerState(state: ServerState, error?: Error) {
+    this.state = state;
+    this.emit('server-state', {state, error});
+  }
+
+  async waitForServerStarted() {
+    return new Promise<void>((resolve, reject) => {
+      switch (this.state) {
+        case 'closed':
+          return reject(new Error('Server was closed already'));
+        case 'error':
+          return reject(new Error('Server has errored already'));
+        case 'started':
+          return resolve();
+        default: {
+          const listener = ({
+            state,
+            error,
+          }: {
+            state: ServerState;
+            error: Error;
+          }) => {
+            switch (state) {
+              case 'error':
+                return reject(error);
+              case 'started':
+                return resolve();
+              case 'closed':
+                return reject(new Error('Server closed'));
+            }
+            this.events.off('server-state', listener);
+          };
+          this.events.on('server-state', listener);
+        }
+      }
+    });
+  }
+
   /** @private */
   async start() {
+    if (this.state !== 'pending') {
+      throw new Error('Server already started');
+    }
+    this.setServerState('starting');
     const server = this.server;
 
     server.addListener('new-client', (client: Client) => {
@@ -93,7 +138,7 @@ export class FlipperServer {
     });
 
     server.addListener('error', (err) => {
-      this.emit('server-start-error', err);
+      this.emit('server-error', err);
     });
 
     server.addListener('start-client-setup', (client: UninitializedClient) => {
@@ -170,15 +215,21 @@ export class FlipperServer {
       },
     );
 
-    await server.init();
+    try {
-    await this.startDeviceListeners();
+      await server.init();
+      await this.startDeviceListeners();
+      this.setServerState('started');
+    } catch (e) {
+      console.error('Failed to start FlipperServer', e);
+      this.setServerState('error', e);
+    }
   }
 
   async startDeviceListeners() {
     this.disposers.push(
       await this.android.watchAndroidDevices(),
       iOSDevice(this.store, this.logger),
-      metroDevice(this.store, this.logger),
+      metroDevice(this),
       desktopDevice(this),
     );
   }
@@ -200,8 +251,60 @@ export class FlipperServer {
     this.events.emit(event, payload);
   }
 
+  registerDevice(device: BaseDevice) {
+    // destroy existing device
+    const existing = this.devices.get(device.serial);
+    if (existing) {
+      // assert different kind of devices aren't accidentally reusing the same serial
+      if (Object.getPrototypeOf(existing) !== Object.getPrototypeOf(device)) {
+        throw new Error(
+          `Tried to register a new device type for existing serial '${
+            device.serial
+          }': Trying to replace existing '${
+            Object.getPrototypeOf(existing).constructor.name
+          }' with a new '${Object.getPrototypeOf(device).constructor.name}`,
+        );
+      }
+      // devices should be recycled, unless they have lost connection
+      if (existing.connected.get()) {
+        throw new Error(
+          `Tried to replace still connected device '${device.serial}' with a new instance`,
+        );
+      }
+      existing.destroy();
+    }
+    // register new device
+    this.devices.set(device.serial, device);
+    this.emit('device-connected', device);
+  }
+
+  unregisterDevice(serial: string) {
+    const device = this.devices.get(serial);
+    if (!device) {
+      return;
+    }
+    device.disconnect(); // we'll only destroy upon replacement
+    this.emit('device-disconnected', device);
+  }
+
+  getDevice(serial: string): BaseDevice {
+    const device = this.devices.get(serial);
+    if (!device) {
+      throw new Error('No device with serial: ' + serial);
+    }
+    return device;
+  }
+
+  getDeviceSerials(): string[] {
+    return Array.from(this.devices.keys());
+  }
+
   public async close() {
     this.server.close();
+    for (const device of this.devices.values()) {
+      device.destroy();
+    }
     this.disposers.forEach((f) => f?.());
+    this.setServerState('closed');
   }
 }

desktop/app/src/server/devices/android/androidDeviceManager.tsx

@@ -230,20 +230,12 @@ export class AndroidDeviceManager {
       return;
     }
 
-    // remove offline devices with same serial as the connected.
+    this.flipperServer.registerDevice(androidDevice);
-    this.devices.get(androidDevice.serial)?.destroy();
-    // register new device
-    this.devices.set(androidDevice.serial, androidDevice);
-    this.flipperServer.emit('device-connected', androidDevice);
   }
 
   unregisterDevices(serials: Array<string>) {
     serials.forEach((serial) => {
-      const device = this.devices.get(serial);
+      this.flipperServer.unregisterDevice(serial);
-      if (device?.connected?.get()) {
-        device.disconnect();
-        this.flipperServer.emit('device-disconnected', device);
-      }
     });
   }
 }

desktop/app/src/server/devices/desktop/desktopDeviceManager.tsx

@@ -20,5 +20,5 @@ export default (flipperServer: FlipperServer) => {
   } else {
     return;
   }
-  flipperServer.emit('device-connected', device);
+  flipperServer.registerDevice(device);
 };

desktop/app/src/server/devices/metro/metroDeviceManager.tsx

@@ -7,13 +7,10 @@
  * @format
  */
 
-import {Store} from '../../../reducers/index';
-import {Logger} from '../../../fb-interfaces/Logger';
 import MetroDevice from './MetroDevice';
 import http from 'http';
-import {addErrorNotification} from '../../../reducers/notifications';
-import {destroyDevice} from '../../../reducers/connections';
 import {parseEnvironmentVariableAsNumber} from '../../utils/environmentVariables';
+import {FlipperServer} from '../../FlipperServer';
 
 const METRO_HOST = 'localhost';
 const METRO_PORT = parseEnvironmentVariableAsNumber('METRO_SERVER_PORT', 8081);
@@ -47,29 +44,15 @@ async function isMetroRunning(): Promise<boolean> {
   });
 }
 
-export async function registerMetroDevice(
+async function registerMetroDevice(
   ws: WebSocket | undefined,
-  store: Store,
+  flipperServer: FlipperServer,
-  logger: Logger,
 ) {
   const metroDevice = new MetroDevice(METRO_URL, ws);
-  logger.track('usage', 'register-device', {
+  flipperServer.registerDevice(metroDevice);
-    os: 'Metro',
-    name: metroDevice.title,
-  });
-
-  metroDevice.loadDevicePlugins(
-    store.getState().plugins.devicePlugins,
-    store.getState().connections.enabledDevicePlugins,
-  );
-  store.dispatch({
-    type: 'REGISTER_DEVICE',
-    payload: metroDevice,
-    serial: METRO_URL,
-  });
 }
 
-export default (store: Store, logger: Logger) => {
+export default (flipperServer: FlipperServer) => {
   let timeoutHandle: NodeJS.Timeout;
   let ws: WebSocket | undefined;
   let unregistered = false;
@@ -86,7 +69,7 @@ export default (store: Store, logger: Logger) => {
         _ws.onopen = () => {
           clearTimeout(guard);
           ws = _ws;
-          registerMetroDevice(ws, store, logger);
+          registerMetroDevice(ws, flipperServer);
         };
 
         _ws.onclose = _ws.onerror = function (event?: any) {
@@ -100,20 +83,19 @@ export default (store: Store, logger: Logger) => {
             unregistered = true;
             clearTimeout(guard);
             ws = undefined;
-            destroyDevice(store, logger, METRO_URL);
+            flipperServer.unregisterDevice(METRO_URL);
             scheduleNext();
           }
         };
 
         const guard = setTimeout(() => {
           // Metro is running, but didn't respond to /events endpoint
-          store.dispatch(
+          flipperServer.emit('notification', {
-            addErrorNotification(
+            type: 'error',
-              'Failed to connect to Metro',
+            title: 'Failed to connect to Metro',
-              `Flipper did find a running Metro instance, but couldn't connect to the logs. Probably your React Native version is too old to support Flipper. Cause: Failed to get a connection to ${METRO_LOGS_ENDPOINT} in a timely fashion`,
+            description: `Flipper did find a running Metro instance, but couldn't connect to the logs. Probably your React Native version is too old to support Flipper. Cause: Failed to get a connection to ${METRO_LOGS_ENDPOINT} in a timely fashion`,
-            ),
+          });
-          );
+          registerMetroDevice(undefined, flipperServer);
-          registerMetroDevice(undefined, store, logger);
           // Note: no scheduleNext, we won't retry until restart
         }, 5000);
       } catch (e) {