From 148f90de7f8975bc4e58da191a102040da242f78 Mon Sep 17 00:00:00 2001
From: Pritesh Nandgaonkar <prit91@fb.com>
Date: Mon, 16 Mar 2020 05:20:11 -0700
Subject: [PATCH] Catch DataExportError from shareFlipperData

Summary:
There was a bug where if the `shareFlipperData` returned an `error` with a status code of less than 400 then it will be ignored. If the status code was of greater than or equal to 400 then `shareFlipperData` throws an error and it is caught and logged appropriately.

This diff fixes the above issue where if the return type from shareFlipperData is of type DataExportError then it is checked and not ignored. Also this fixes an issue where if DataExportError happens the loader is replaced with an error message.

Basically the bug mentioned here, T55169042. Also this diff logs the metadata of result, so that we can debug the exact error through scuba.

Reviewed By: mweststrate

Differential Revision: D20450159

fbshipit-source-id: 95b1f384886c16fe444ae0cd6f6d9b2251c29005
---
 desktop/src/chrome/ShareSheetExportUrl.tsx | 27 ++++++++++++----------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/desktop/src/chrome/ShareSheetExportUrl.tsx b/desktop/src/chrome/ShareSheetExportUrl.tsx
index 435c0daf1..ebf1b8c44 100644
--- a/desktop/src/chrome/ShareSheetExportUrl.tsx
+++ b/desktop/src/chrome/ShareSheetExportUrl.tsx
@@ -131,12 +131,17 @@ export default class ShareSheetExportUrl extends Component<Props, State> {
       const uploadMarker = `${EXPORT_FLIPPER_TRACE_EVENT}:upload`;
       performance.mark(uploadMarker);
       statusUpdate('Uploading Flipper Trace...');
-      // TODO(T55169042): Implement error handling. Result is not tested
-      // its result type right now, causing the upload indicator to stall forever.
       const result = await reportPlatformFailures(
         shareFlipperData(serializedString),
         `${SHARE_FLIPPER_TRACE_EVENT}`,
       );
+
+      if ((result as DataExportError).error != undefined) {
+        const res = result as DataExportError;
+        const err = new Error(res.error);
+        err.stack = res.stacktrace;
+        throw err;
+      }
       getLogger().trackTimeSince(uploadMarker, uploadMarker, {
         plugins: this.store.getState().plugins.selectedPlugins,
       });
@@ -153,23 +158,21 @@ export default class ShareSheetExportUrl extends Component<Props, State> {
       this.store.dispatch(resetSupportFormV2State());
       this.props.logger.trackTimeSince(mark, 'export:url-success');
     } catch (e) {
+      const result: DataExportError = {
+        error_class: 'EXPORT_ERROR',
+        error: e,
+        stacktrace: '',
+      };
       if (!this.state.runInBackground) {
-        const result: DataExportError = {
-          error_class: 'EXPORT_ERROR',
-          error: '',
-          stacktrace: '',
-        };
-
         if (e instanceof Error) {
           result.error = e.message;
           result.stacktrace = e.stack || '';
-        } else {
-          result.error = e;
         }
+        // Show the error in UI.
         this.setState({result});
       }
       this.store.dispatch(unsetShare());
-      this.props.logger.trackTimeSince(mark, 'export:url-error');
+      this.props.logger.trackTimeSince(mark, 'export:url-error', result);
       throw e;
     }
   }
@@ -221,7 +224,7 @@ export default class ShareSheetExportUrl extends Component<Props, State> {
 
   render() {
     const {result, statusUpdate, errorArray} = this.state;
-    if (!result || !(result as DataExportResult).flipperUrl) {
+    if (!result) {
       return this.renderPending(statusUpdate);
     }