jens/flipper
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Make sure Flipper server startup errors are propagated for desktop

Browse Source 
Summary:
If openssl is not available, this would lead to an unhandled rejection exception. That is because a lot initialization logic generates promises that don't get a catch chained on immediately. Changed the flipper server startup flow to be more idiomatically async

Fixes https://github.com/facebook/flipper/issues/2766

Changelog: More clearly communicate if flipper server failed to start (e.d. due to port already taken, openssl not being available)

This change fixes it only for desktop flipper, will make sure the browser UI will support this as well (the error will fire correctly there, but there are no listeners during startup)

Reviewed By: nikoant

Differential Revision: D33348923

fbshipit-source-id: f561bb27b18a3041c514b37f7aed11435a49647f
This commit is contained in:
Michel Weststrate
2022-01-04 02:56:33 -08:00
committed by Facebook GitHub Bot
parent 7f944ae946
commit 8259f92983
3 changed files with 70 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
desktop/app/src/init.tsx
View File

@@ -96,7 +96,6 @@ async function start() {
keytar, keytar,
); );
await flipperServer.connect();
const flipperServerConfig = await flipperServer.exec('get-config'); const flipperServerConfig = await flipperServer.exec('get-config');
initializeElectron(flipperServer, flipperServerConfig); initializeElectron(flipperServer, flipperServerConfig);
@@ -110,6 +109,8 @@ async function start() {
// eslint-disable-next-line import/no-commonjs // eslint-disable-next-line import/no-commonjs
require('flipper-ui-core').startFlipperDesktop(flipperServer); require('flipper-ui-core').startFlipperDesktop(flipperServer);
await flipperServer.connect();
// Initialize launcher // Initialize launcher
setupPrefetcher(flipperServerConfig.settings); setupPrefetcher(flipperServerConfig.settings);
} }

107
desktop/flipper-server-core/src/comms/ServerController.tsx
View File

@@ -71,15 +71,14 @@ interface ServerController {
* Additionally, it manages client connections. * Additionally, it manages client connections.
*/ */
class ServerController extends EventEmitter implements ServerEventsListener { class ServerController extends EventEmitter implements ServerEventsListener {
connections: Map<string, ClientInfo>; connections: Map<string, ClientInfo> = new Map();
timestamps: Map<string, ClientTimestampTracker>; timestamps: Map<string, ClientTimestampTracker> = new Map();
initialized: Promise<void> | null; secureServer: ServerAdapter | null = null;
secureServer: Promise<ServerAdapter> | null; insecureServer: ServerAdapter | null = null;
insecureServer: Promise<ServerAdapter> | null; altSecureServer: ServerAdapter | null = null;
altSecureServer: Promise<ServerAdapter> | null; altInsecureServer: ServerAdapter | null = null;
altInsecureServer: Promise<ServerAdapter> | null; browserServer: ServerAdapter | null = null;
browserServer: Promise<ServerAdapter> | null;
certificateProvider: CertificateProvider; certificateProvider: CertificateProvider;
connectionTracker: ConnectionTracker; connectionTracker: ConnectionTracker;
@@ -91,19 +90,11 @@ class ServerController extends EventEmitter implements ServerEventsListener {
constructor(flipperServer: FlipperServerImpl) { constructor(flipperServer: FlipperServerImpl) {
super(); super();
this.flipperServer = flipperServer; this.flipperServer = flipperServer;
this.connections = new Map();
this.timestamps = new Map();
this.certificateProvider = new CertificateProvider( this.certificateProvider = new CertificateProvider(
this, this,
getFlipperServerConfig().settings, getFlipperServerConfig().settings,
); );
this.connectionTracker = new ConnectionTracker(this.logger); this.connectionTracker = new ConnectionTracker(this.logger);
this.secureServer = null;
this.insecureServer = null;
this.altSecureServer = null;
this.altInsecureServer = null;
this.browserServer = null;
this.initialized = null;
} }
onClientMessage(clientId: string, payload: string): void { onClientMessage(clientId: string, payload: string): void {
@@ -122,69 +113,57 @@ class ServerController extends EventEmitter implements ServerEventsListener {
* Initialisation is complete once the initialized promise is fullfilled at * Initialisation is complete once the initialized promise is fullfilled at
* which point Flipper is accepting connections. * which point Flipper is accepting connections.
*/ */
init() { async init() {
if (isTest()) { if (isTest()) {
throw new Error('Spawing new server is not supported in test'); throw new Error('Spawing new server is not supported in test');
} }
this.certificateProvider.init();
const {insecure, secure} = getServerPortsConfig().serverPorts; const {insecure, secure} = getServerPortsConfig().serverPorts;
this.initialized = this.certificateProvider const options = await this.certificateProvider.loadSecureServerConfig();
.loadSecureServerConfig()
.then((options) => { console.info('[conn] secure server listening at port: ', secure);
console.info('[conn] secure server listening at port: ', secure); this.secureServer = await createServer(secure, this, options);
this.secureServer = createServer(secure, this, options); const {secure: altSecure} = getServerPortsConfig().altServerPorts;
const {secure: altSecure} = getServerPortsConfig().altServerPorts; console.info('[conn] secure server (ws) listening at port: ', altSecure);
console.info( this.altSecureServer = await createServer(
'[conn] secure server (ws) listening at port: ', altSecure,
altSecure, this,
); options,
this.altSecureServer = createServer( TransportType.WebSocket,
altSecure, );
this,
options, console.info('[conn] insecure server listening at port: ', insecure);
TransportType.WebSocket, this.insecureServer = await createServer(insecure, this);
); const {insecure: altInsecure} = getServerPortsConfig().altServerPorts;
}) console.info(
.then(() => { '[conn] insecure server (ws) listening at port: ',
console.info('[conn] insecure server listening at port: ', insecure); altInsecure,
this.insecureServer = createServer(insecure, this); );
const {insecure: altInsecure} = getServerPortsConfig().altServerPorts; this.altInsecureServer = await createServer(
console.info( altInsecure,
'[conn] insecure server (ws) listening at port: ', this,
altInsecure, undefined,
); TransportType.WebSocket,
this.altInsecureServer = createServer( );
altInsecure,
this,
undefined,
TransportType.WebSocket,
);
return;
});
if (GK.get('comet_enable_flipper_connection')) { if (GK.get('comet_enable_flipper_connection')) {
console.info('[conn] Browser server (ws) listening at port: ', 8333); console.info('[conn] Browser server (ws) listening at port: ', 8333);
this.browserServer = createBrowserServer(8333, this); this.browserServer = await createBrowserServer(8333, this);
} }
reportPlatformFailures(this.initialized, 'initializeServer');
return this.initialized;
} }
/** /**
* If initialized, it stops any started servers. * If initialized, it stops any started servers.
*/ */
async close() { async close() {
if (this.initialized && (await this.initialized)) { await Promise.all([
await Promise.all([ this.insecureServer?.stop(),
this.insecureServer && (await this.insecureServer).stop(), this.secureServer?.stop(),
this.secureServer && (await this.secureServer).stop(), this.altInsecureServer?.stop(),
this.altInsecureServer && (await this.altInsecureServer).stop(), this.altSecureServer?.stop(),
this.altSecureServer && (await this.altSecureServer).stop(), this.browserServer?.stop(),
this.browserServer && (await this.browserServer).stop(), ]);
]);
}
} }
onConnectionCreated( onConnectionCreated(

49
desktop/flipper-server-core/src/utils/CertificateProvider.tsx
View File

@@ -89,12 +89,12 @@ type CertificateProviderConfig = {
* Flipper CA. * Flipper CA.
*/ */
export default class CertificateProvider { export default class CertificateProvider {
private _adb: Promise<ADBClient> | undefined; private _adb: ADBClient | undefined;
private didCertificateSetup = false; private didCertificateSetup = false;
private config: CertificateProviderConfig; private config: CertificateProviderConfig;
private server: ServerController; private server: ServerController;
get adb(): Promise<ADBClient> { get adb(): ADBClient {
if (this.config.enableAndroid) { if (this.config.enableAndroid) {
if (this._adb) { if (this._adb) {
return this._adb; return this._adb;
@@ -105,22 +105,25 @@ export default class CertificateProvider {
} }
constructor(server: ServerController, config: CertificateProviderConfig) { constructor(server: ServerController, config: CertificateProviderConfig) {
// TODO: refactor this code to create promise lazily
this._adb = config.enableAndroid
? (getAdbClient(config).catch((_e) => {
// make sure initialization failure is already logged
const msg =
'Failed to initialize ADB. Please disable Android support in settings, or configure a correct path';
server.flipperServer.emit('notification', {
type: 'error',
title: 'Failed to initialise ADB',
description: msg,
});
this._adb = undefined; // no adb client available
}) as Promise<ADBClient>)
: undefined;
this.config = config;
this.server = server; this.server = server;
this.config = config;
}
async init() {
if (this.config.enableAndroid) {
try {
this._adb = await getAdbClient(this.config);
} catch (_e) {
// make sure initialization failure is already logged
const msg =
'Failed to initialize ADB. Please disable Android support in settings, or configure a correct path';
this.server.flipperServer.emit('notification', {
type: 'error',
title: 'Failed to initialise ADB',
description: msg,
});
}
}
} }
private uploadFiles = async ( private uploadFiles = async (
@@ -255,10 +258,9 @@ export default class CertificateProvider {
private async ensureOpenSSLIsAvailable(): Promise<void> { private async ensureOpenSSLIsAvailable(): Promise<void> {
if (!(await opensslInstalled())) { if (!(await opensslInstalled())) {
const e = Error( throw new Error(
"It looks like you don't have OpenSSL installed. Please install it to continue.", "It looks like you don't have OpenSSL installed. Please install it to continue.",
); );
this.server.emit('error', e);
} }
} }
@@ -321,9 +323,8 @@ export default class CertificateProvider {
destination, destination,
csr, csr,
); );
const adbClient = await this.adb;
await androidUtil.push( await androidUtil.push(
adbClient, this.adb,
deviceId, deviceId,
appName, appName,
destination + filename, destination + filename,
@@ -379,7 +380,7 @@ export default class CertificateProvider {
deviceCsrFilePath: string, deviceCsrFilePath: string,
csr: string, csr: string,
): Promise<string> { ): Promise<string> {
const devicesInAdb = await this.adb.then((client) => client.listDevices()); const devicesInAdb = await this.adb.listDevices();
if (devicesInAdb.length === 0) { if (devicesInAdb.length === 0) {
throw new Error('No Android devices found'); throw new Error('No Android devices found');
} }
@@ -472,9 +473,8 @@ export default class CertificateProvider {
processName: string, processName: string,
csr: string, csr: string,
): Promise<{isMatch: boolean; foundCsr: string}> { ): Promise<{isMatch: boolean; foundCsr: string}> {
const adbClient = await this.adb;
const deviceCsr = await androidUtil.pull( const deviceCsr = await androidUtil.pull(
adbClient, this.adb,
deviceId, deviceId,
processName, processName,
directory + csrFileName, directory + csrFileName,
@@ -590,6 +590,7 @@ export default class CertificateProvider {
} }
async loadSecureServerConfig(): Promise<SecureServerConfig> { async loadSecureServerConfig(): Promise<SecureServerConfig> {
await this.ensureOpenSSLIsAvailable();
await this.certificateSetup(); await this.certificateSetup();
return { return {
key: await fs.readFile(serverKey), key: await fs.readFile(serverKey),