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
This commit is contained in:
Michel Weststrate
2021-09-15 14:44:53 -07:00
committed by Facebook GitHub Bot
parent 1390bf4a33
commit 94a7b4360d
2 changed files with 19 additions and 39 deletions

View File

@@ -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',
);
}

View File

@@ -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'),