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

Don't send messages to disconnected clients. Make exportPersistedState compatible with disconnected devices.

Browse Source 
Summary:
This diff addresses two problems:

1. Since clients plugins can be active beyond having a connection, we have to make it possible for plugin authors to check if they are connected before they make a call.
2. if there is a custom `exportPersistedState`, plugins should be able to skip making calls if the device has disconnected.

Introducing this change makes it possible to interact with a reasonable level with disconnected clients, and makes it possible to create Flipper traces for disconnected clients.

Note that both items were already problems before supporting offline clients; as there can be a noticeable delay between disconnecting and Flipper detecting that (i've seen up to 30 secs). What happend previously in those cases is that the export would simply hang, as would other user interactions, as loosing the connection in the middle of a process would cause the promise chains to be neither rejected or resolved, which is pretty iffy.

Before this diff, trying to export a disconnected device would hang forever like:

{F369600601}

Reviewed By: nikoant

Differential Revision: D26250895

fbshipit-source-id: 177624a116883c3cba14390cd0fe164e243bb97c
This commit is contained in:
Michel Weststrate
2021-02-09 04:12:09 -08:00
committed by Facebook GitHub Bot
parent 7650ab620d
commit 1bb1cae167
9 changed files with 106 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files

76
desktop/app/src/Client.tsx
View File

@@ -601,46 +601,48 @@ export default class Client extends EventEmitter {
const mark = this.getPerformanceMark(metadata);
performance.mark(mark);
if (!this.connected) {
reject(new Error('Not connected to client'));
}
if (!fromPlugin || this.isAcceptingMessagesFromPlugin(plugin)) {
this.connection &&
this.connection
.requestResponse({data: JSON.stringify(data)})
.subscribe({
onComplete: (payload) => {
if (!fromPlugin || this.isAcceptingMessagesFromPlugin(plugin)) {
const logEventName = this.getLogEventName(data);
this.logger.trackTimeSince(mark, logEventName);
emitBytesReceived(
plugin || 'unknown',
payload.data.length * 2,
);
const response: {
success?: Object;
error?: ErrorType;
} = JSON.parse(payload.data);
this.connection!.requestResponse({
data: JSON.stringify(data),
}).subscribe({
onComplete: (payload) => {
if (!fromPlugin || this.isAcceptingMessagesFromPlugin(plugin)) {
const logEventName = this.getLogEventName(data);
this.logger.trackTimeSince(mark, logEventName);
emitBytesReceived(plugin || 'unknown', payload.data.length * 2);
const response: {
success?: Object;
error?: ErrorType;
} = JSON.parse(payload.data);
this.onResponse(response, resolve, reject);
this.onResponse(response, resolve, reject);
if (flipperMessagesClientPlugin.isConnected()) {
flipperMessagesClientPlugin.newMessage({
device: this.deviceSync?.displayTitle(),
app: this.query.app,
flipperInternalMethod: method,
payload: response,
plugin,
pluginMethod: params?.method,
direction: 'toFlipper:response',
});
}
}
},
// Open fresco then layout and you get errors because responses come back after deinit.
onError: (e) => {
if (this.isAcceptingMessagesFromPlugin(plugin)) {
reject(e);
}
},
});
if (flipperMessagesClientPlugin.isConnected()) {
flipperMessagesClientPlugin.newMessage({
device: this.deviceSync?.displayTitle(),
app: this.query.app,
flipperInternalMethod: method,
payload: response,
plugin,
pluginMethod: params?.method,
direction: 'toFlipper:response',
});
}
}
},
onError: (e) => {
reject(e);
},
});
} else {
reject(
new Error(
`Cannot send ${method}, client is not accepting messages for plugin ${plugin}`,
),
);
}
if (flipperMessagesClientPlugin.isConnected()) {

10
desktop/app/src/plugin.tsx
View File

@@ -61,6 +61,7 @@ export function supportsMethod(
}
export interface PluginClient {
isConnected: boolean;
// eslint-disable-next-line
send(method: string, params?: Parameters): void;
// eslint-disable-next-line
@@ -130,7 +131,9 @@ export abstract class FlipperBasePlugin<
static maxQueueSize: number = DEFAULT_MAX_QUEUE_SIZE;
static exportPersistedState:
| ((
callClient: (method: string, params?: any) => Promise<any>,
callClient:
| undefined
| ((method: string, params?: any) => Promise<any>),
persistedState: StaticPersistedState | undefined,
store: ReduxState | undefined,
idler?: Idler,
@@ -256,8 +259,11 @@ export class FlipperPlugin<
// @ts-ignore constructor should be assigned already
const {id} = this.constructor;
this.subscriptions = [];
this.realClient = props.target as Client;
const realClient = (this.realClient = props.target as Client);
this.client = {
get isConnected() {
return realClient.connected.get();
},
call: (method, params) => this.realClient.call(id, method, true, params),
send: (method, params) => this.realClient.send(id, method, params),
subscribe: (method, callback) => {

12
desktop/app/src/utils/exportData.tsx
View File

@@ -518,9 +518,13 @@ export async function fetchMetadata(
client,
pluginKey,
} of pluginsToProcess) {
const exportState = pluginClass ? pluginClass.exportPersistedState : null;
const exportState =
pluginClass && !isSandyPlugin(pluginClass)
? pluginClass.exportPersistedState
: null;
if (exportState) {
const fetchMetaDataMarker = `${EXPORT_FLIPPER_TRACE_EVENT}:fetch-meta-data-per-plugin`;
const isConnected = client.connected.get();
performance.mark(fetchMetaDataMarker);
try {
statusUpdate &&
@@ -528,12 +532,14 @@ export async function fetchMetadata(
const data = await promiseTimeout(
240000, // Fetching MobileConfig data takes ~ 3 mins, thus keeping timeout at 4 mins.
exportState(
callClient(client, pluginId),
isConnected ? callClient(client, pluginId) : undefined,
newPluginState[pluginKey],
state,
idler,
statusUpdate,
supportsMethod(client, pluginId),
isConnected
? supportsMethod(client, pluginId)
: () => Promise.resolve(false),
),
`Timed out while collecting data for ${pluginName}`,
);

2
desktop/app/src/utils/pluginUtils.tsx
View File

@@ -110,7 +110,7 @@ function isExportablePlugin(
plugin: PluginDefinition,
): boolean {
// can generate an export when requested
if (plugin.exportPersistedState) {
if (!isSandyPlugin(plugin) && plugin.exportPersistedState) {
return true;
}
const pluginKey = isDevicePluginDefinition(plugin)

12
desktop/flipper-plugin/src/plugin/SandyPluginDefinition.tsx
View File

@@ -45,18 +45,6 @@ export class SandyPluginDefinition {
details: ActivatablePluginDetails;
isDevicePlugin: boolean;
// TODO: Implement T68683476
exportPersistedState:
| ((
callClient: (method: string, params?: any) => Promise<any>,
persistedState: any, // TODO: type StaticPersistedState | undefined,
store: any, // TODO: ReduxState | undefined,
idler?: any, // TODO: Idler,
statusUpdate?: (msg: string) => void,
supportsMethod?: (method: string) => Promise<boolean>,
) => Promise<any /* TODO: StaticPersistedState | undefined */>)
| undefined = undefined;
constructor(
details: ActivatablePluginDetails,
module: FlipperPluginModule<any> | FlipperDevicePluginModule,

4
desktop/plugins/fresco/index.tsx
View File

@@ -96,7 +96,7 @@ export default class FlipperImagesPlugin extends FlipperPlugin<
};
static exportPersistedState = (
callClient: (method: string, params?: any) => Promise<any>,
callClient: undefined | ((method: string, params?: any) => Promise<any>),
persistedState: PersistedState,
store?: ReduxState,
): Promise<PersistedState> => {
@@ -104,7 +104,7 @@ export default class FlipperImagesPlugin extends FlipperPlugin<
if (!persistedState) {
persistedState = FlipperImagesPlugin.defaultPersistedState;
}
if (!store) {
if (!store || !callClient) {
return defaultPromise;
}
return Promise.all([

3
desktop/plugins/layout/ProxyArchiveClient.tsx
View File

@@ -91,6 +91,9 @@ class ProxyArchiveClient {
this.persistedState = cloneDeep(persistedState);
this.onElementHighlighted = onElementHighlighted;
}
isConnected = true;
persistedState: PersistedState;
onElementHighlighted: ((id: string) => void) | undefined;
subscribe(_method: string, _callback: (params: any) => void): void {

1
desktop/plugins/layout/__tests__/Inspector.node.tsx
View File

@@ -25,6 +25,7 @@ beforeEach(() => {
extraInfo: {},
};
const client: PluginClient = {
isConnected: true,
send: () => {},
call: () => Promise.resolve(mockRoot),
subscribe: () => {},

75
desktop/plugins/layout/index.tsx
View File

@@ -80,14 +80,16 @@ export default class LayoutPlugin extends FlipperPlugin<
PersistedState
> {
static exportPersistedState = async (
callClient: (method: ClientMethodCalls, params?: any) => Promise<any>,
callClient:
| undefined
| ((method: ClientMethodCalls, params?: any) => Promise<any>),
persistedState: PersistedState | undefined,
store: ReduxState | undefined,
_idler?: Idler | undefined,
statusUpdate?: (msg: string) => void,
supportsMethod?: (method: ClientMethodCalls) => Promise<boolean>,
): Promise<PersistedState | undefined> => {
if (!store) {
if (!store || !callClient) {
return persistedState;
}
statusUpdate && statusUpdate('Fetching Root Node...');
@@ -211,26 +213,29 @@ export default class LayoutPlugin extends FlipperPlugin<
// If the selected plugin from the previous session was layout, then while importing the flipper export, the redux store doesn't get updated in the first render, due to which the plugin crashes, as it has no persisted state
this.props.setPersistedState(this.constructor.defaultPersistedState);
}
// persist searchActive state when moving between plugins to prevent multiple
// TouchOverlayViews since we can't edit the view heirarchy in onDisconnect
this.client.call('isSearchActive').then(({isSearchActive}) => {
this.setState({inTargetMode: isSearchActive});
});
// disable target mode after
this.client.subscribe('select', () => {
if (this.state.inTargetMode) {
this.onToggleTargetMode();
}
});
if (this.client.isConnected) {
// persist searchActive state when moving between plugins to prevent multiple
// TouchOverlayViews since we can't edit the view heirarchy in onDisconnect
this.client.call('isSearchActive').then(({isSearchActive}) => {
this.setState({inTargetMode: isSearchActive});
});
this.client.subscribe('resolvePath', (params: ClassFileParams) => {
this.resolvePath(params);
});
// disable target mode after
this.client.subscribe('select', () => {
if (this.state.inTargetMode) {
this.onToggleTargetMode();
}
});
this.client.subscribe('openInIDE', (params: OpenFileParams) => {
this.openInIDE(params);
});
this.client.subscribe('resolvePath', (params: ClassFileParams) => {
this.resolvePath(params);
});
this.client.subscribe('openInIDE', (params: OpenFileParams) => {
this.openInIDE(params);
});
}
// since the first launch of Myles might produce a lag (Myles daemon needs to start)
// try to invoke Myles during the first launch of the Layout Plugin
@@ -270,10 +275,12 @@ export default class LayoutPlugin extends FlipperPlugin<
params.dirRoot,
);
const resolvedPath = IDEFileResolver.getBestPath(paths, params.className);
this.client.send('setResolvedPath', {
className: params.className,
resolvedPath: resolvedPath,
});
if (this.client.isConnected) {
this.client.send('setResolvedPath', {
className: params.className,
resolvedPath: resolvedPath,
});
}
};
openInIDE = async (params: OpenFileParams) => {
@@ -294,9 +301,11 @@ export default class LayoutPlugin extends FlipperPlugin<
};
onToggleTargetMode = () => {
const inTargetMode = !this.state.inTargetMode;
this.setState({inTargetMode});
this.client.send('setSearchActive', {active: inTargetMode});
if (this.client.isConnected) {
const inTargetMode = !this.state.inTargetMode;
this.setState({inTargetMode});
this.client.send('setSearchActive', {active: inTargetMode});
}
};
onToggleAXMode = () => {
@@ -311,13 +320,15 @@ export default class LayoutPlugin extends FlipperPlugin<
: this.client;
}
onToggleAlignmentMode = () => {
if (this.state.selectedElement) {
this.client.send('setHighlighted', {
id: this.state.selectedElement,
inAlignmentMode: !this.state.inAlignmentMode,
});
if (this.client.isConnected) {
if (this.state.selectedElement) {
this.client.send('setHighlighted', {
id: this.state.selectedElement,
inAlignmentMode: !this.state.inAlignmentMode,
});
this.setState({inAlignmentMode: !this.state.inAlignmentMode});
}
}
this.setState({inAlignmentMode: !this.state.inAlignmentMode});
};
onToggleVisualizer = () => {