From 94a7b4360d952c911e5e1d7f3adbd8459ebd926f Mon Sep 17 00:00:00 2001
From: Michel Weststrate <mweststrate@fb.com>
Date: Wed, 15 Sep 2021 14:44:53 -0700
Subject: [PATCH] add error handing to ADB client creation

Summary:
Noticed there are uncaught exception if adb cannot be found, that don't make it to the UI, but just cause no devices to be detected.

While testing, found that the unsafeClient fallback we use doesn't find the devices either. So it is unclear to me if it is helping in any case, and figured we'd better provide early signal to the user to set up adb correctly, rather than jumping through hoops and trying to work without it.

Changelog: Handle the absence of ADB better

Reviewed By: timur-valiev

Differential Revision: D30957220

fbshipit-source-id: 4d9a89bffefa96d3861d3f26224b4c74c19abd37
---
 .../src/server/devices/android/adbClient.tsx  | 44 +++----------------
 .../src/server/utils/CertificateProvider.tsx  | 14 +++++-
 2 files changed, 19 insertions(+), 39 deletions(-)

diff --git a/desktop/app/src/server/devices/android/adbClient.tsx b/desktop/app/src/server/devices/android/adbClient.tsx
index e5d96ae02..833b2a92a 100644
--- a/desktop/app/src/server/devices/android/adbClient.tsx
+++ b/desktop/app/src/server/devices/android/adbClient.tsx
@@ -34,42 +34,10 @@ export function getAdbClient(config: Config): Promise<Client> {
 async function createClient(config: Config): Promise<Client> {
   const androidHome = config.androidHome;
   const adbPath = path.resolve(androidHome, 'platform-tools/adb');
-  try {
-    return reportPlatformFailures<Client>(
-      execFile(adbPath, ['start-server']).then(() =>
-        adbkit.createClient(adbConfig()),
-      ),
-      'createADBClient.shell',
-    );
-  } catch (err) {
-    console.log(
-      'Failed to create adb client using shell adb command. Trying with adbkit.\n' +
-        err.toString(),
-    );
-
-    /* In the event that starting adb with the above method fails, fallback
-         to using adbkit, though its known to be unreliable. */
-    const unsafeClient: Client = adbkit.createClient(adbConfig());
-    return await reportPlatformFailures<Client>(
-      promiseRetry<Client>(
-        async (retry, attempt): Promise<Client> => {
-          try {
-            await unsafeClient.listDevices();
-            return unsafeClient;
-          } catch (e) {
-            console.warn(`Failed to start adb client. Retrying. ${e.message}`);
-            if (attempt <= MAX_RETRIES) {
-              retry(e);
-            }
-            throw e;
-          }
-        },
-        {
-          minTimeout: 200,
-          retries: MAX_RETRIES,
-        },
-      ),
-      'createADBClient.adbkit',
-    );
-  }
+  return reportPlatformFailures<Client>(
+    execFile(adbPath, ['start-server']).then(() =>
+      adbkit.createClient(adbConfig()),
+    ),
+    'createADBClient.shell',
+  );
 }
diff --git a/desktop/app/src/server/utils/CertificateProvider.tsx b/desktop/app/src/server/utils/CertificateProvider.tsx
index ea0a9fc3e..cfe821b8b 100644
--- a/desktop/app/src/server/utils/CertificateProvider.tsx
+++ b/desktop/app/src/server/utils/CertificateProvider.tsx
@@ -29,6 +29,7 @@ import archiver from 'archiver';
 import {timeout} from 'flipper-plugin';
 import {v4 as uuid} from 'uuid';
 import {isTest} from '../../utils/isProduction';
+import {message} from 'antd';
 
 export type CertificateExchangeMedium = 'FS_ACCESS' | 'WWW' | 'NONE';
 
@@ -100,7 +101,18 @@ export default class CertificateProvider {
     config: CertificateProviderConfig,
   ) {
     this.logger = logger;
-    this.adb = getAdbClient(config);
+    // TODO: refactor this code to create promise lazily
+    this.adb = getAdbClient(config).catch((e) => {
+      // make sure initialization failure is already logged
+      message.error({
+        duration: null,
+        content:
+          'Failed to initialise ADB. Please check your Android settings, ANDROID_HOME and run the Setup Doctor. ' +
+          e,
+      });
+      console.error('Failed to initialise ADB', e);
+      this.adb = Promise.reject(e);
+    }) as Promise<ADBClient>;
     if (isTest()) {
       this.certificateSetup = Promise.reject(
         new Error('Server certificates not available in test'),