diff --git a/desktop/app/src/Client.tsx b/desktop/app/src/Client.tsx index a8579ceea..f90808d01 100644 --- a/desktop/app/src/Client.tsx +++ b/desktop/app/src/Client.tsx @@ -21,7 +21,7 @@ import {setPluginState} from './reducers/pluginStates'; import {Payload, ConnectionStatus} from 'rsocket-types'; import {Flowable, Single} from 'rsocket-flowable'; import {performance} from 'perf_hooks'; -import {reportPlatformFailures, reportPluginFailures} from './utils/metrics'; +import {reportPluginFailures} from './utils/metrics'; import {notNull} from './utils/typeUtils'; import {default as isProduction} from './utils/isProduction'; import {registerPlugins} from './reducers/plugins'; @@ -33,7 +33,6 @@ import { defaultEnabledBackgroundPlugins, } from './utils/pluginUtils'; import {processMessagesLater} from './utils/messageQueue'; -import {sideEffect} from './utils/sideEffect'; import {emitBytesReceived} from './dispatcher/tracking'; import {debounce} from 'lodash'; import {batch} from 'react-redux'; @@ -134,11 +133,14 @@ export default class Client extends EventEmitter { connection: FlipperClientConnection | null | undefined; store: Store; activePlugins: Set; + + /** + * @deprecated + * use plugin.deviceSync instead + */ device: Promise; - _deviceResolve: (device: BaseDevice) => void = (_) => {}; - _deviceResolved: BaseDevice | undefined; + deviceSync: BaseDevice; logger: Logger; - lastSeenDeviceList: Array; broadcastCallbacks: Map>>; messageBuffer: Record< string /*pluginKey*/, @@ -168,8 +170,8 @@ export default class Client extends EventEmitter { conn: FlipperClientConnection | null | undefined, logger: Logger, store: Store, - plugins?: Plugins | null | undefined, - device?: BaseDevice, + plugins: Plugins | null | undefined, + device: BaseDevice, ) { super(); this.connected = true; @@ -185,17 +187,9 @@ export default class Client extends EventEmitter { this.broadcastCallbacks = new Map(); this.requestCallbacks = new Map(); this.activePlugins = new Set(); - this.lastSeenDeviceList = []; - this.device = device - ? Promise.resolve(device) - : new Promise((resolve, _reject) => { - this._deviceResolve = resolve; - }); - - if (device != null) { - this._deviceResolved = device; - } + this.device = Promise.resolve(device); + this.deviceSync = device; const client = this; if (conn) { @@ -215,58 +209,6 @@ export default class Client extends EventEmitter { } } - /* All clients should have a corresponding Device in the store. - However, clients can connect before a device is registered, so wait a - while for the device to be registered if it isn't already. */ - async setMatchingDevice(): Promise { - return reportPlatformFailures( - new Promise((resolve, reject) => { - let unsubscribe: () => void = () => {}; - - const device = this.store - .getState() - .connections.devices.find( - (device) => device.serial === this.query.device_id, - ); - if (device) { - this._deviceResolved = device; - resolve(device); - return; - } - - const timeout = setTimeout(() => { - unsubscribe(); - const error = `Timed out waiting for device for client ${this.id}`; - console.error(error); - reject(error); - }, 5000); - unsubscribe = sideEffect( - this.store, - {name: 'waitForDevice', throttleMs: 100}, - (state) => state.connections.devices, - (newDeviceList) => { - if (newDeviceList === this.lastSeenDeviceList) { - return; - } - this.lastSeenDeviceList = newDeviceList; - const matchingDevice = newDeviceList.find( - (device) => device.serial === this.query.device_id, - ); - if (matchingDevice) { - clearTimeout(timeout); - resolve(matchingDevice); - unsubscribe(); - } - }, - ); - }), - 'client-setMatchingDevice', - ).then((device) => { - this._deviceResolved = device; - this._deviceResolve(device); - }); - } - supportsPlugin(pluginId: string): boolean { return this.plugins.includes(pluginId); } @@ -289,7 +231,6 @@ export default class Client extends EventEmitter { } async init() { - this.setMatchingDevice(); await this.loadPlugins(); // this starts all sandy enabled plugins this.plugins.forEach((pluginId) => @@ -434,20 +375,12 @@ export default class Client extends EventEmitter { this.emit('plugins-change'); } + /** + * @deprecated + * use deviceSync.serial + */ async deviceSerial(): Promise { - try { - const device = await this.device; - if (!device) { - console.error('Using "" for deviceId device is not ready'); - return ''; - } - return device.serial; - } catch (e) { - console.error( - 'Using "" for deviceId because client has no matching device', - ); - return ''; - } + return this.deviceSync.serial; } onMessage(msg: string) { @@ -478,7 +411,7 @@ export default class Client extends EventEmitter { flipperMessagesClientPlugin.isConnected() ) { flipperMessagesClientPlugin.newMessage({ - device: this._deviceResolved?.displayTitle(), + device: this.deviceSync?.displayTitle(), app: this.query.app, flipperInternalMethod: method, plugin: data.params?.api, @@ -497,7 +430,7 @@ export default class Client extends EventEmitter { }: ${error.message} + \nDevice Stack Trace: ${error.stacktrace}`, 'deviceError', ); - this.device.then((device) => handleError(this.store, device, error)); + handleError(this.store, this.deviceSync, error); } else if (method === 'refreshPlugins') { this.refreshPlugins(); } else if (method === 'execute') { @@ -576,7 +509,7 @@ export default class Client extends EventEmitter { reject(data.error); const {error} = data; if (error) { - this.device.then((device) => handleError(this.store, device, error)); + handleError(this.store, this.deviceSync, error); } } else { // ??? @@ -670,7 +603,7 @@ export default class Client extends EventEmitter { if (flipperMessagesClientPlugin.isConnected()) { flipperMessagesClientPlugin.newMessage({ - device: this._deviceResolved?.displayTitle(), + device: this.deviceSync?.displayTitle(), app: this.query.app, flipperInternalMethod: method, payload: response, @@ -692,7 +625,7 @@ export default class Client extends EventEmitter { if (flipperMessagesClientPlugin.isConnected()) { flipperMessagesClientPlugin.newMessage({ - device: this._deviceResolved?.displayTitle(), + device: this.deviceSync?.displayTitle(), app: this.query.app, flipperInternalMethod: method, plugin: params?.api, @@ -776,7 +709,7 @@ export default class Client extends EventEmitter { if (flipperMessagesClientPlugin.isConnected()) { flipperMessagesClientPlugin.newMessage({ - device: this._deviceResolved?.displayTitle(), + device: this.deviceSync?.displayTitle(), app: this.query.app, flipperInternalMethod: method, payload: params, diff --git a/desktop/app/src/chrome/__tests__/ExportDataPluginSheet.node.tsx b/desktop/app/src/chrome/__tests__/ExportDataPluginSheet.node.tsx index c882ef355..141b953a7 100644 --- a/desktop/app/src/chrome/__tests__/ExportDataPluginSheet.node.tsx +++ b/desktop/app/src/chrome/__tests__/ExportDataPluginSheet.node.tsx @@ -83,6 +83,7 @@ function getStore(selectedPlugins: Array) { // @ts-ignore mockStore, ['TestPlugin', 'TestDevicePlugin'], + selectedDevice, ); mockStore.dispatch({ type: 'NEW_CLIENT', diff --git a/desktop/app/src/plugin.tsx b/desktop/app/src/plugin.tsx index cdaab6c53..8c82d5d8d 100644 --- a/desktop/app/src/plugin.tsx +++ b/desktop/app/src/plugin.tsx @@ -285,10 +285,17 @@ export class FlipperPlugin< client: PluginClient; realClient: Client; + /** + * @deprecated use .device instead + */ getDevice(): Promise { return this.realClient.device; } + get device() { + return this.realClient.deviceSync; + } + _teardown() { // automatically unsubscribe subscriptions const pluginId = this.constructor.id; diff --git a/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx b/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx index da6f04dcf..8ea40a357 100644 --- a/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx +++ b/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx @@ -378,8 +378,8 @@ export function findBestDevice( return selected; } // if there is an active app, use device owning the app - if (client && client._deviceResolved) { - return client._deviceResolved; + if (client) { + return client.deviceSync; } // if no active app, use the preferred device if (userPreferredDevice) { diff --git a/desktop/app/src/server.tsx b/desktop/app/src/server.tsx index 80ca3e922..cbdec6c6f 100644 --- a/desktop/app/src/server.tsx +++ b/desktop/app/src/server.tsx @@ -13,7 +13,7 @@ import { } from './utils/CertificateProvider'; import {Logger} from './fb-interfaces/Logger'; import {ClientQuery} from './Client'; -import {Store} from './reducers/index'; +import {Store, State} from './reducers/index'; import CertificateProvider from './utils/CertificateProvider'; import {RSocketServer} from 'rsocket-core'; import RSocketTCPServer from 'rsocket-tcp-server'; @@ -38,6 +38,8 @@ import {IncomingMessage} from 'http'; import ws from 'ws'; import {initSelfInpector} from './utils/self-inspection/selfInspectionUtils'; import ClientDevice from './devices/ClientDevice'; +import BaseDevice from './devices/BaseDevice'; +import {sideEffect} from './utils/sideEffect'; type ClientInfo = { connection: FlipperClientConnection | null | undefined; @@ -529,7 +531,7 @@ class Server extends EventEmitter { ); }) : Promise.resolve(query.device_id) - ).then((csrId) => { + ).then(async (csrId) => { query.device_id = csrId; query.app = appNameWithUpdateHint(query); @@ -541,7 +543,18 @@ class Server extends EventEmitter { }); console.debug(`Device connected: ${id}`, 'server'); - const client = new Client(id, query, conn, this.logger, this.store); + const device = + getDeviceBySerial(this.store.getState(), query.device_id) ?? + (await findDeviceForConnection(this.store, query.app, query.device_id)); + const client = new Client( + id, + query, + conn, + this.logger, + this.store, + undefined, + device, + ); const info = { client, @@ -627,4 +640,54 @@ class ConnectionTracker { } } +function getDeviceBySerial( + state: State, + serial: string, +): BaseDevice | undefined { + return state.connections.devices.find((device) => device.serial === serial); +} + +async function findDeviceForConnection( + store: Store, + clientId: string, + serial: string, +): Promise { + let lastSeenDeviceList: BaseDevice[] = []; + /* All clients should have a corresponding Device in the store. + However, clients can connect before a device is registered, so wait a + while for the device to be registered if it isn't already. */ + return reportPlatformFailures( + new Promise((resolve, reject) => { + let unsubscribe: () => void = () => {}; + + const timeout = setTimeout(() => { + unsubscribe(); + const error = `Timed out waiting for device ${serial} for client ${clientId}`; + console.error(error); + reject(error); + }, 5000); + unsubscribe = sideEffect( + store, + {name: 'waitForDevice', throttleMs: 100}, + (state) => state.connections.devices, + (newDeviceList) => { + if (newDeviceList === lastSeenDeviceList) { + return; + } + lastSeenDeviceList = newDeviceList; + const matchingDevice = newDeviceList.find( + (device) => device.serial === serial, + ); + if (matchingDevice) { + clearTimeout(timeout); + resolve(matchingDevice); + unsubscribe(); + } + }, + ); + }), + 'client-setMatchingDevice', + ); +} + export default Server; diff --git a/desktop/app/src/utils/__tests__/exportData.node.tsx b/desktop/app/src/utils/__tests__/exportData.node.tsx index d2e985ec6..bcaa4fc73 100644 --- a/desktop/app/src/utils/__tests__/exportData.node.tsx +++ b/desktop/app/src/utils/__tests__/exportData.node.tsx @@ -726,6 +726,7 @@ test('test determinePluginsToProcess for mutilple clients having plugins present logger, mockStore, ['TestPlugin', 'TestDevicePlugin'], + device1, ); const client2 = new Client( generateClientIdentifier(device1, 'app2'), @@ -739,6 +740,7 @@ test('test determinePluginsToProcess for mutilple clients having plugins present logger, mockStore, ['TestDevicePlugin'], + device1, ); const client3 = new Client( generateClientIdentifier(device1, 'app3'), @@ -752,6 +754,7 @@ test('test determinePluginsToProcess for mutilple clients having plugins present logger, mockStore, ['TestPlugin', 'TestDevicePlugin'], + device1, ); const plugins: PluginsState = { clientPlugins: new Map([ @@ -802,6 +805,7 @@ test('test determinePluginsToProcess for no selected plugin present in any clien logger, mockStore, ['TestPlugin', 'TestDevicePlugin'], + device1, ); const client2 = new Client( generateClientIdentifier(device1, 'app2'), @@ -815,6 +819,7 @@ test('test determinePluginsToProcess for no selected plugin present in any clien logger, mockStore, ['TestDevicePlugin'], + device1, ); const plugins: PluginsState = { clientPlugins: new Map([ @@ -846,6 +851,7 @@ test('test determinePluginsToProcess for multiple clients on same device', async logger, mockStore, ['TestPlugin', 'TestDevicePlugin'], + device1, ); const client2 = new Client( generateClientIdentifier(device1, 'app2'), @@ -859,6 +865,7 @@ test('test determinePluginsToProcess for multiple clients on same device', async logger, mockStore, ['TestDevicePlugin'], + device1, ); const plugins: PluginsState = { clientPlugins: new Map([['TestPlugin', TestPlugin]]), @@ -897,6 +904,7 @@ test('test determinePluginsToProcess for multiple clients on different device', logger, mockStore, ['TestPlugin', 'TestDevicePlugin'], + device1, ); const client2Device1 = new Client( generateClientIdentifier(device1, 'app2'), @@ -910,6 +918,7 @@ test('test determinePluginsToProcess for multiple clients on different device', logger, mockStore, ['TestDevicePlugin'], + device1, ); const client1Device2 = new Client( generateClientIdentifier(device2, 'app'), @@ -923,6 +932,7 @@ test('test determinePluginsToProcess for multiple clients on different device', logger, mockStore, ['TestPlugin', 'TestDevicePlugin'], + device1, ); const client2Device2 = new Client( generateClientIdentifier(device2, 'app2'), @@ -936,6 +946,7 @@ test('test determinePluginsToProcess for multiple clients on different device', logger, mockStore, ['TestDevicePlugin'], + device1, ); const plugins: PluginsState = { clientPlugins: new Map([['TestPlugin', TestPlugin]]), @@ -999,6 +1010,7 @@ test('test determinePluginsToProcess to ignore archived clients', async () => { logger, mockStore, ['TestPlugin', 'TestDevicePlugin'], + archivedDevice, ); const archivedClient = new Client( generateClientIdentifier(archivedDevice, 'app'), @@ -1012,6 +1024,7 @@ test('test determinePluginsToProcess to ignore archived clients', async () => { logger, mockStore, ['TestPlugin', 'TestDevicePlugin'], + archivedDevice, ); const plugins: PluginsState = { clientPlugins: new Map([['TestPlugin', TestPlugin]]), diff --git a/desktop/app/src/utils/js-client-server-utils/serverUtils.tsx b/desktop/app/src/utils/js-client-server-utils/serverUtils.tsx index c1bb5c626..56e1b408d 100644 --- a/desktop/app/src/utils/js-client-server-utils/serverUtils.tsx +++ b/desktop/app/src/utils/js-client-server-utils/serverUtils.tsx @@ -44,9 +44,10 @@ export function initJsEmulatorIPC( (_event: IpcRendererEvent, message: any) => { const {windowId} = message; const {plugins, appName} = message.payload; + const device = new JSDevice(jsDeviceId(windowId), 'jsEmulator', windowId); store.dispatch({ type: 'REGISTER_DEVICE', - payload: new JSDevice(jsDeviceId(windowId), 'jsEmulator', windowId), + payload: device, }); const connection = new JSClientFlipperConnection(windowId); @@ -69,6 +70,7 @@ export function initJsEmulatorIPC( logger, store, plugins, + device, ); flipperConnections.set(clientId, { diff --git a/desktop/app/src/utils/self-inspection/selfInspectionUtils.tsx b/desktop/app/src/utils/self-inspection/selfInspectionUtils.tsx index 373ada4ab..d256d84b2 100644 --- a/desktop/app/src/utils/self-inspection/selfInspectionUtils.tsx +++ b/desktop/app/src/utils/self-inspection/selfInspectionUtils.tsx @@ -32,14 +32,15 @@ export function initSelfInpector( ) { const appName = 'Flipper'; const device_id = 'FlipperSelfInspectionDevice'; + const device = new FlipperSelfInspectionDevice( + device_id, + 'emulator', + appName, + 'JSWebApp', + ); store.dispatch({ type: 'REGISTER_DEVICE', - payload: new FlipperSelfInspectionDevice( - device_id, - 'emulator', - appName, - 'JSWebApp', - ), + payload: device, }); selfInspectionClient.addPlugin(flipperMessagesClientPlugin); @@ -59,6 +60,8 @@ export function initSelfInpector( selfInspectionClient, logger, store, + undefined, + device, ); flipperConnections.set(clientId, { diff --git a/desktop/plugins/layout/index.tsx b/desktop/plugins/layout/index.tsx index 1202ac80f..818456da9 100644 --- a/desktop/plugins/layout/index.tsx +++ b/desktop/plugins/layout/index.tsx @@ -240,7 +240,7 @@ export default class LayoutPlugin extends FlipperPlugin< } if (this.props.isArchivedDevice) { - this.getDevice() + Promise.resolve(this.device) .then((d) => { const handle = (d as ArchivedDevice).getArchivedScreenshotHandle(); if (!handle) {