Defer initialization of Android and iOS device managers

Summary: Remove hidden async initialization of adb and idb. Make it explicit. Remove nullable fields in Android and iOS device managers.

Reviewed By: lawrencelomax

Differential Revision: D33915177

fbshipit-source-id: 882f79310410e0dfde6169abf343ab808644e4a2

desktop/flipper-server-core/src/FlipperServerImpl.tsx

@@ -46,6 +46,9 @@ import {promises} from 'fs';
 // Electron 11 runs on Node 12 which does not support fs.promises.rm
 import rm from 'rimraf';
 import assert from 'assert';
+import {setAdbClient} from './devices/android/adbClient';
+import {setIdbConfig} from './devices/ios/idbConfig';
+import {assertNotNull} from './comms/Utilities';
 
 const {access, copyFile, mkdir, unlink, stat, readlink, readFile, writeFile} =
   promises;
@@ -66,8 +69,8 @@ export class FlipperServerImpl implements FlipperServer {
   private readonly devices = new Map<string, ServerDevice>();
   state: FlipperServerState = 'pending';
   stateError: string | undefined = undefined;
-  android: AndroidDeviceManager;
+  android?: AndroidDeviceManager;
-  ios: IOSDeviceManager;
+  ios?: IOSDeviceManager;
   keytarManager: KeytarManager;
   pluginManager: PluginManager;
 
@@ -81,8 +84,6 @@ export class FlipperServerImpl implements FlipperServer {
       'Loaded flipper config, paths: ' + JSON.stringify(config.paths, null, 2),
     );
     const server = (this.server = new ServerController(this));
-    this.android = new AndroidDeviceManager(this);
-    this.ios = new IOSDeviceManager(this);
     this.keytarManager = new KeytarManager(keytarModule);
     // given flipper-dump, it might make more sense to have the plugin command
     // handling (like download, install, etc) moved to flipper-server & app,
@@ -161,10 +162,34 @@ export class FlipperServerImpl implements FlipperServer {
   async startDeviceListeners() {
     const asyncDeviceListenersPromises: Array<Promise<void>> = [];
     if (this.config.settings.enableAndroid) {
-      asyncDeviceListenersPromises.push(this.android.watchAndroidDevices());
+      asyncDeviceListenersPromises.push(
+        setAdbClient(this.config.settings)
+          .then((adbClient) => {
+            if (!adbClient) {
+              return;
+            }
+            this.android = new AndroidDeviceManager(this, adbClient);
+            return this.android.watchAndroidDevices();
+          })
+          .catch((e) => {
+            console.error(
+              'FlipperServerImpl.startDeviceListeners.watchAndroidDevices -> unexpected error',
+              e,
+            );
+          }),
+      );
     }
     if (this.config.settings.enableIOS) {
-      asyncDeviceListenersPromises.push(this.ios.watchIOSDevices());
+      const idbConfig = setIdbConfig(this.config.settings);
+      this.ios = new IOSDeviceManager(this, idbConfig);
+      asyncDeviceListenersPromises.push(
+        this.ios.watchIOSDevices().catch((e) => {
+          console.error(
+            'FlipperServerImpl.startDeviceListeners.watchIOSDevices -> unexpected error',
+            e,
+          );
+        }),
+      );
     }
     const asyncDeviceListeners = await Promise.all(
       asyncDeviceListenersPromises,
@@ -346,13 +371,20 @@ export class FlipperServerImpl implements FlipperServer {
         },
       };
     },
-    'android-get-emulators': async () => this.android.getAndroidEmulators(),
+    'android-get-emulators': async () => {
+      assertNotNull(this.android);
+      return this.android.getAndroidEmulators();
+    },
     'android-launch-emulator': async (name, coldBoot) =>
       launchEmulator(this.config.settings.androidHome, name, coldBoot),
-    'ios-get-simulators': async (bootedOnly) =>
+    'ios-get-simulators': async (bootedOnly) => {
-      this.ios.getSimulators(bootedOnly),
+      assertNotNull(this.ios);
-    'ios-launch-simulator': async (udid) =>
+      return this.ios.getSimulators(bootedOnly);
-      this.ios.simctlBridge.launchSimulator(udid),
+    },
+    'ios-launch-simulator': async (udid) => {
+      assertNotNull(this.ios);
+      return this.ios.simctlBridge.launchSimulator(udid);
+    },
     'persist-settings': async (settings) => saveSettings(settings),
     'persist-launcher-settings': async (settings) =>
       saveLauncherSettings(settings),

desktop/flipper-server-core/src/comms/ServerController.tsx

@@ -24,6 +24,7 @@ import invariant from 'invariant';
 import DummyDevice from '../devices/DummyDevice';
 import {
   appNameWithUpdateHint,
+  assertNotNull,
   cloneClientQuerySafeForLogging,
   transformCertificateExchangeMediumToType,
 } from './Utilities';
@@ -281,10 +282,12 @@ class ServerController extends EventEmitter implements ServerEventsListener {
     let certificateProvider: CertificateProvider;
     switch (clientQuery.os) {
       case 'Android': {
+        assertNotNull(this.flipperServer.android);
         certificateProvider = this.flipperServer.android.certificateProvider;
         break;
       }
       case 'iOS': {
+        assertNotNull(this.flipperServer.ios);
         certificateProvider = this.flipperServer.ios.certificateProvider;
 
         if (medium === 'WWW') {
@@ -377,6 +380,7 @@ class ServerController extends EventEmitter implements ServerEventsListener {
     // For Android, device id might change
     if (csr_path && csr && query.os === 'Android') {
       const app_name = await extractAppNameFromCSR(csr);
+      assertNotNull(this.flipperServer.android);
       // TODO: allocate new object, kept now as is to keep changes minimal
       (query as any).device_id =
         await this.flipperServer.android.certificateProvider.getTargetDeviceId(

desktop/flipper-server-core/src/devices/android/androidDeviceManager.tsx

@@ -10,28 +10,20 @@
 import AndroidDevice from './AndroidDevice';
 import KaiOSDevice from './KaiOSDevice';
 import child_process from 'child_process';
-import {setAdbClient} from './adbClient';
 import {Client as ADBClient, Device} from 'adbkit';
 import {join} from 'path';
 import {FlipperServerImpl} from '../../FlipperServerImpl';
 import {notNull} from '../../utils/typeUtils';
-import {
+import {getServerPortsConfig} from '../../FlipperServerConfig';
-  getServerPortsConfig,
-  getFlipperServerConfig,
-} from '../../FlipperServerConfig';
 import AndroidCertificateProvider from './AndroidCertificateProvider';
-import {assertNotNull} from '../../comms/Utilities';
 
 export class AndroidDeviceManager {
-  private adbClient?: ADBClient;
+  readonly certificateProvider: AndroidCertificateProvider;
-  constructor(public flipperServer: FlipperServerImpl) {}
+  constructor(
-
+    private readonly flipperServer: FlipperServerImpl,
-  public get certificateProvider() {
+    private readonly adbClient: ADBClient,
-    assertNotNull(
+  ) {
-      this.adbClient,
+    this.certificateProvider = new AndroidCertificateProvider(this.adbClient);
-      'AndroidDeviceManager.certificateProvider -> missing adbClient',
-    );
-    return new AndroidCertificateProvider(this.adbClient);
   }
 
   private createDevice(
@@ -181,16 +173,7 @@ export class AndroidDeviceManager {
 
   async watchAndroidDevices() {
     try {
-      const client = await setAdbClient(getFlipperServerConfig().settings);
+      this.adbClient
-      if (!client) {
-        throw new Error(
-          'AndroidDeviceManager.watchAndroidDevices -> adb not initialized',
-        );
-      }
-
-      this.adbClient = client;
-
-      client
         .trackDevices()
         .then((tracker) => {
           tracker.on('error', (err) => {
@@ -212,7 +195,7 @@ export class AndroidDeviceManager {
 
           tracker.on('add', async (device) => {
             if (device.type !== 'offline') {
-              this.registerDevice(client, device);
+              this.registerDevice(this.adbClient, device);
             } else {
               console.warn(
                 `[conn] Found device ${device.id}, but it has status offline. If this concerns an emulator and the problem persists, try these solutins: https://stackoverflow.com/a/21330228/1983583, https://stackoverflow.com/a/56053223/1983583`,
@@ -224,7 +207,7 @@ export class AndroidDeviceManager {
             if (device.type === 'offline') {
               this.flipperServer.unregisterDevice(device.id);
             } else {
-              this.registerDevice(client, device);
+              this.registerDevice(this.adbClient, device);
             }
           });
 

desktop/flipper-server-core/src/devices/ios/__tests__/iOSDevice.node.tsx

@@ -88,8 +88,10 @@ test('test checkXcodeVersionMismatch with an incorrect Simulator.app', () => {
 });
 
 test('test queryDevices when simctl used', async () => {
-  const ios = new IOSDeviceManager(fakeFlipperServer);
+  const ios = new IOSDeviceManager(
-  (ios as any).idbConfig = getFlipperServerConfig().settings;
+    fakeFlipperServer,
+    getFlipperServerConfig().settings,
+  );
   ios.simctlBridge = fakeSimctlBridge;
 
   await ios.queryDevices(fakeSimctlBridge);
@@ -109,8 +111,10 @@ test('test queryDevices when simctl used', async () => {
 });
 
 test('test queryDevices when idb used', async () => {
-  const ios = new IOSDeviceManager(fakeFlipperServer);
+  const ios = new IOSDeviceManager(
-  (ios as any).idbConfig = getFlipperServerConfig().settings;
+    fakeFlipperServer,
+    getFlipperServerConfig().settings,
+  );
   ios.simctlBridge = fakeSimctlBridge;
 
   await ios.queryDevices(fakeIDBBridge);

desktop/flipper-server-core/src/devices/ios/iOSDeviceManager.tsx

@@ -22,14 +22,11 @@ import {
 } from './IOSBridge';
 import {FlipperServerImpl} from '../../FlipperServerImpl';
 import {getFlipperServerConfig} from '../../FlipperServerConfig';
-import {IdbConfig, setIdbConfig} from './idbConfig';
+import {IdbConfig} from './idbConfig';
-import {assertNotNull} from '../../comms/Utilities';
 import iOSCertificateProvider from './iOSCertificateProvider';
 
 export class IOSDeviceManager {
   private portForwarders: Array<ChildProcess> = [];
-  private idbConfig?: IdbConfig;
-
   private portforwardingClient = path.join(
     getFlipperServerConfig().paths.staticPath,
     'PortForwardingMacApp.app',
@@ -39,14 +36,13 @@ export class IOSDeviceManager {
   );
   simctlBridge: SimctlBridge = new SimctlBridge();
 
-  constructor(private flipperServer: FlipperServerImpl) {}
+  readonly certificateProvider: iOSCertificateProvider;
 
-  public get certificateProvider() {
+  constructor(
-    assertNotNull(
+    private readonly flipperServer: FlipperServerImpl,
-      this.idbConfig,
+    private readonly idbConfig: IdbConfig,
-      'IOSDeviceManager.certificateProvider -> missing idbConfig',
+  ) {
-    );
+    this.certificateProvider = new iOSCertificateProvider(this.idbConfig);
-    return new iOSCertificateProvider(this.idbConfig);
   }
 
   private forwardPort(port: number, multiplexChannelPort: number) {
@@ -137,11 +133,9 @@ export class IOSDeviceManager {
   }
 
   public async watchIOSDevices() {
-    const settings = getFlipperServerConfig().settings;
-    this.idbConfig = setIdbConfig(settings);
     try {
       const isDetected = await iosUtil.isXcodeDetected();
-      if (settings.enablePhysicalIOS) {
+      if (this.idbConfig.enablePhysicalIOS) {
         this.startDevicePortForwarders();
       }
       try {
@@ -149,9 +143,9 @@ export class IOSDeviceManager {
         await this.checkXcodeVersionMismatch();
         // Awaiting the promise here to trigger immediate error handling.
         const bridge = await makeIOSBridge(
-          settings.idbPath,
+          this.idbConfig.idbPath,
           isDetected,
-          settings.enablePhysicalIOS,
+          this.idbConfig.enablePhysicalIOS,
         );
         this.queryDevicesForever(bridge);
       } catch (err) {