Timeout promise while exporting flipper plugin

Summary:
Added a promiseTimeout util so that we limit the time taken to export flipper plugin. I had received a feedback from the user stating that that the loader for the export stays for ages and never finishes. So this diff adds a timeout to the promise which is returned by `exportPersistedState` function. This function is currently implemented just by Layout inspector to fetch the entire view hierarchy. I suspect the the former mentioned bug may be due to the unresponsive client.

This diff also shows the plugin and the client information for which `exportPersistedState` timed out. Also this will hopefully solve the problem surfaced recently stating that the bug report gets stuck, the reason for which I suspect would be the same as the above mentioned reason, because we export flipper data when we create a bug report.

Reviewed By: danielbuechele

Differential Revision: D14712633

fbshipit-source-id: 35f8cfa722ec3b7ff94ebda943d618834ac3207d

headless/index.js

@@ -101,8 +101,8 @@ function startFlipper({
       // current eventloop task here.
       setTimeout(() => {
         exportStore(store)
-          .then(output => {
-            originalConsole.log(output);
+          .then(({serializedString}) => {
+            originalConsole.log(serializedString);
             process.exit();
           })
           .catch(console.error);
@@ -122,7 +122,9 @@ function startFlipper({
   if (exit == 'sigint') {
     process.on('SIGINT', async () => {
       try {
-        originalConsole.log(await exportStore(store));
+        const {serializedString, errorArray} = await exportStore(store);
+        errorArray.forEach(console.error);
+        originalConsole.log(serializedString);
       } catch (e) {
         console.error(e);
       }

src/chrome/ShareSheet.js

@@ -59,10 +59,20 @@ const InfoText = styled(Text)({
   marginBottom: 15,
 });
 
+const Padder = styled('div')(
+  ({paddingLeft, paddingRight, paddingBottom, paddingTop}) => ({
+    paddingLeft: paddingLeft || 0,
+    paddingRight: paddingRight || 0,
+    paddingBottom: paddingBottom || 0,
+    paddingTop: paddingTop || 0,
+  }),
+);
+
 type Props = {
   onHide: () => mixed,
 };
 type State = {
+  errorArray: Array<Error>,
   result:
     | ?{
         error_class: string,
@@ -79,14 +89,17 @@ export default class ShareSheet extends Component<Props, State> {
   };
 
   state = {
+    errorArray: [],
     result: null,
   };
 
   async componentDidMount() {
-    const storeData = await exportStore(this.context.store);
-    const result = await shareFlipperData(storeData);
-    this.setState({result});
-
+    try {
+      const {serializedString, errorArray} = await exportStore(
+        this.context.store,
+      );
+      const result = await shareFlipperData(serializedString);
+      this.setState({errorArray, result});
       if (result.flipperUrl) {
         clipboard.writeText(String(result.flipperUrl));
         new window.Notification('Sharable Flipper trace created', {
@@ -94,6 +107,15 @@ export default class ShareSheet extends Component<Props, State> {
           requireInteraction: true,
         });
       }
+    } catch (e) {
+      this.setState({
+        result: {
+          error_class: 'EXPORT_ERROR',
+          error: e,
+        },
+      });
+      return;
+    }
   }
 
   render() {
@@ -116,6 +138,21 @@ export default class ShareSheet extends Component<Props, State> {
                     data might contain sensitve information like access tokens
                     used in network requests.
                   </InfoText>
+                  {this.state.errorArray.length > 0 && (
+                    <Padder paddingBottom={8}>
+                      <FlexColumn>
+                        <Title bold>
+                          The following errors occurred while exporting your
+                          data
+                        </Title>
+                        {this.state.errorArray.map((e: Error) => {
+                          return (
+                            <ErrorMessage code>{e.toString()}</ErrorMessage>
+                          );
+                        })}
+                      </FlexColumn>
+                    </Padder>
+                  )}
                 </>
               ) : (
                 <>

src/chrome/ShareSheetExportFile.js

@@ -54,11 +54,21 @@ const InfoText = styled(Text)({
   marginBottom: 15,
 });
 
+const Padder = styled('div')(
+  ({paddingLeft, paddingRight, paddingBottom, paddingTop}) => ({
+    paddingLeft: paddingLeft || 0,
+    paddingRight: paddingRight || 0,
+    paddingBottom: paddingBottom || 0,
+    paddingTop: paddingTop || 0,
+  }),
+);
+
 type Props = {
   onHide: () => mixed,
   file: string,
 };
 type State = {
+  errorArray: Array<Error>,
   result: ?{
     success: boolean,
     error: ?Error,
@@ -71,18 +81,19 @@ export default class ShareSheetExportFile extends Component<Props, State> {
   };
 
   state = {
+    errorArray: [],
     result: null,
   };
 
   async componentDidMount() {
     try {
-      await reportPlatformFailures(
+      const {errorArray} = await reportPlatformFailures(
         exportStoreToFile(this.props.file, this.context.store),
         `${EXPORT_FLIPPER_TRACE_EVENT}:UI`,
       );
-      this.setState({result: {success: true, error: null}});
+      this.setState({errorArray, result: {success: true, error: null}});
     } catch (err) {
-      this.setState({result: {success: false, error: err}});
+      this.setState({errorArray: [], result: {success: false, error: err}});
     }
   }
 
@@ -100,6 +111,16 @@ export default class ShareSheetExportFile extends Component<Props, State> {
                 might contain sensitive information like access tokens used in
                 network requests.
               </InfoText>
+              {this.state.errorArray.length > 0 && (
+                <Padder paddingBottom={8}>
+                  <FlexColumn>
+                    <Title bold>Errors: </Title>
+                    {this.state.errorArray.map((e: Error) => {
+                      return <ErrorMessage code>{e.toString()}</ErrorMessage>;
+                    })}
+                  </FlexColumn>
+                </Padder>
+              )}
             </FlexColumn>
             <FlexRow>
               <Spacer />

src/utils/__tests__/promiseTimeout.node.js

@@ -0,0 +1,37 @@
+/**
+ * Copyright 2018-present Facebook.
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ * @format
+ */
+import promiseTimeout from '../promiseTimeout';
+
+test('test promiseTimeout for timeout to happen', () => {
+  let promise = promiseTimeout(
+    200,
+    new Promise((resolve, reject) => {
+      let id = setTimeout(() => {
+        clearTimeout(id);
+        resolve();
+      }, 500);
+      return 'Executed';
+    }),
+    'Timed out',
+  );
+  return expect(promise).rejects.toThrow('Timed out');
+});
+
+test('test promiseTimeout for timeout not to happen', () => {
+  let promise = promiseTimeout(
+    200,
+    new Promise((resolve, reject) => {
+      let id = setTimeout(() => {
+        clearTimeout(id);
+        resolve();
+      }, 100);
+      resolve('Executed');
+    }),
+    'Timed out',
+  );
+  return expect(promise).resolves.toBe('Executed');
+});

src/utils/exportData.js

@@ -23,7 +23,8 @@ import {remote} from 'electron';
 import {serialize, deserialize} from './serialization';
 import {readCurrentRevision} from './packageMetadata.js';
 import {tryCatchReportPlatformFailures} from './metrics';
-
+import {promisify} from 'util';
+import promiseTimeout from './promiseTimeout';
 export const IMPORT_FLIPPER_TRACE_EVENT = 'import-flipper-trace';
 export const EXPORT_FLIPPER_TRACE_EVENT = 'export-flipper-trace';
 
@@ -185,7 +186,7 @@ export const processStore = async (
 
 export async function getStoreExport(
   store: MiddlewareAPI,
-): Promise<?ExportType> {
+): Promise<{exportData: ?ExportType, errorArray: Array<Error>}> {
   const state = store.getState();
   const {clients} = state.connections;
   const {pluginStates} = state;
@@ -204,6 +205,7 @@ export async function getStoreExport(
   plugins.devicePlugins.forEach((val, key) => {
     pluginsMap.set(key, val);
   });
+  const errorArray: Array<Error> = [];
   for (let client of clients) {
     for (let plugin of client.plugins) {
       const pluginClass: ?Class<
@@ -212,12 +214,17 @@ export async function getStoreExport(
       const exportState = pluginClass ? pluginClass.exportPersistedState : null;
       if (exportState) {
         const key = pluginKey(client.id, plugin);
-        const data = await exportState(
-          callClient(client, plugin),
-          newPluginState[key],
-          store,
+        try {
+          const data = await promiseTimeout(
+            120000, // Timeout in 2 mins
+            exportState(callClient(client, plugin), newPluginState[key], store),
+            `Timed out while collecting data for ${plugin}`,
           );
           newPluginState[key] = data;
+        } catch (e) {
+          errorArray.push(e);
+          continue;
+        }
       }
     }
   }
@@ -226,7 +233,7 @@ export async function getStoreExport(
   const {selectedDevice} = store.getState().connections;
   const {devicePlugins} = store.getState().plugins;
 
-  return processStore(
+  const exportData = await processStore(
     activeNotifications,
     selectedDevice,
     newPluginState,
@@ -234,35 +241,37 @@ export async function getStoreExport(
     devicePlugins,
     uuid.v4(),
   );
+  return {exportData, errorArray};
 }
 
-export function exportStore(store: MiddlewareAPI): Promise<string> {
+export function exportStore(
+  store: MiddlewareAPI,
+): Promise<{serializedString: string, errorArray: Array<Error>}> {
   getLogger().track('usage', EXPORT_FLIPPER_TRACE_EVENT);
   return new Promise(async (resolve, reject) => {
-    const storeExport = await getStoreExport(store);
-    if (!storeExport) {
+    const {exportData, errorArray} = await getStoreExport(store);
+    if (!exportData) {
       console.error('Make sure a device is connected');
       reject('No device is selected');
     }
-    const serializedString = serialize(storeExport);
+    const serializedString = serialize(exportData);
     if (serializedString.length <= 0) {
       reject('Serialize function returned empty string');
     }
-    resolve(serializedString);
+    resolve({serializedString, errorArray});
   });
 }
 
 export const exportStoreToFile = (
   exportFilePath: string,
   store: Store,
-): Promise<void> => {
-  return exportStore(store).then(storeString => {
-    fs.writeFile(exportFilePath, storeString, err => {
-      if (err) {
-        throw new Error(err);
-      }
-      return;
-    });
+): Promise<{errorArray: Array<Error>}> => {
+  return exportStore(store).then(({serializedString, errorArray}) => {
+    return promisify(fs.writeFile)(exportFilePath, serializedString).then(
+      () => {
+        return {errorArray};
+      },
+    );
   });
 };
 

src/utils/promiseTimeout.js

@@ -0,0 +1,23 @@
+/**
+ * Copyright 2018-present Facebook.
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ * @format
+ */
+
+export default function promiseTimeout<T>(
+  ms: number,
+  promise: Promise<T>,
+  timeoutMessage: ?string,
+): Promise<T> | Promise<void> {
+  // Create a promise that rejects in <ms> milliseconds
+  let timeout = new Promise((resolve, reject) => {
+    let id = setTimeout(() => {
+      clearTimeout(id);
+      reject(new Error(timeoutMessage || `Timed out in ${ms} ms.`));
+    }, ms);
+  });
+
+  // Returns a race between our timeout and the passed in promise
+  return Promise.race([promise, timeout]);
+}