fix(uploader): Handle errors with calling logging endpoint (#803)

* fix(uploader): Handle errors with calling logging endpoint
box · Jan 25, 2019 · e661edb · e661edb
1 parent 7aa67da
commit e661edb
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 16 deletions.
diff --git a/src/api/uploads/MultiputPart.js b/src/api/uploads/MultiputPart.js
@@ -5,10 +5,13 @@
  */
 import noop from 'lodash/noop';
 import getProp from 'lodash/get';
-import BaseMultiput from './BaseMultiput';
 import { updateQueryParameters } from '../../utils/url';
-import { HTTP_PUT } from '../../constants';
 import { getBoundedExpBackoffRetryDelay } from '../../utils/uploads';
+import { retryNumOfTimes } from '../../utils/function';
+
+import BaseMultiput from './BaseMultiput';
+
+import { HTTP_PUT } from '../../constants';
 
 const PART_STATE_NOT_STARTED: 0 = 0;
 const PART_STATE_COMPUTING_DIGEST: 1 = 1;
@@ -233,7 +236,7 @@ class MultiputPart extends BaseMultiput {
      * @param {Error} error
      * @return {void}
      */
-    uploadErrorHandler = (error: Error) => {
+    uploadErrorHandler = async (error: Error) => {
         if (this.isDestroyed()) {
             return;
         }
@@ -256,7 +259,24 @@ class MultiputPart extends BaseMultiput {
         };
 
         const eventInfoString = JSON.stringify(eventInfo);
-        this.logEvent('part_failure', eventInfoString);
+
+        try {
+            if (!this.sessionEndpoints.logEvent) {
+                throw new Error('logEvent endpoint not found');
+            }
+
+            await retryNumOfTimes(
+                (resolve: Function, reject: Function): void => {
+                    this.logEvent('eventInfoString', eventInfoString)
+                        .then(resolve)
+                        .catch(reject);
+                },
+                this.config.retries,
+                this.config.initialRetryDelayMs,
+            );
+        } catch (err) {
+            this.consoleLog('Failure in logEvent ', error);
+        }
 
         if (this.numUploadRetriesPerformed >= this.config.retries) {
             this.onError(error, eventInfoString);

diff --git a/src/api/uploads/MultiputUpload.js b/src/api/uploads/MultiputUpload.js
@@ -378,10 +378,14 @@ class MultiputUpload extends BaseMultiput {
             this.sha1Worker.terminate();
         }
 
-        if (this.sessionEndpoints.abort) {
-            this.xhr.delete({
-                url: this.sessionEndpoints.abort,
-            });
+        if (this.sessionEndpoints.abort && this.sessionId) {
+            this.xhr
+                .delete({
+                    url: this.sessionEndpoints.abort,
+                })
+                .then(() => {
+                    this.sessionId = '';
+                });
         }
     }
 

diff --git a/src/api/uploads/__tests__/MultiputPart-test.js b/src/api/uploads/__tests__/MultiputPart-test.js
@@ -116,10 +116,9 @@ describe('api/uploads/MultiputPart', () => {
             MultiputPartTest.destroyed = false;
             MultiputPartTest.numUploadRetriesPerformed = 100;
             MultiputPartTest.config.retries = 1;
-            MultiputPartTest.logEvent = jest.fn();
+            MultiputPartTest.logEvent = jest.fn().mockResolvedValue();
             MultiputPartTest.onError = jest.fn();
             MultiputPartTest.uploadErrorHandler(error);
-            expect(MultiputPartTest.logEvent).toHaveBeenCalled();
             expect(MultiputPartTest.onError).toHaveBeenCalled();
         });
 
@@ -130,13 +129,12 @@ describe('api/uploads/MultiputPart', () => {
             MultiputPartTest.destroyed = false;
             MultiputPartTest.numUploadRetriesPerformed = 100;
             MultiputPartTest.config.retries = 1000;
-            MultiputPartTest.logEvent = jest.fn();
+            MultiputPartTest.logEvent = jest.fn().mockResolvedValue();
             MultiputPartTest.onError = jest.fn();
             MultiputPartTest.retryUpload = jest.fn();
             MultiputPartTest.uploadErrorHandler(error);
             jest.runOnlyPendingTimers();
             expect(MultiputPartTest.numUploadRetriesPerformed).toBe(101);
-            expect(MultiputPartTest.logEvent).toHaveBeenCalled();
             expect(MultiputPartTest.onError).not.toHaveBeenCalled();
             jest.clearAllTimers();
         });

diff --git a/src/api/uploads/__tests__/MultiputUpload-test.js b/src/api/uploads/__tests__/MultiputUpload-test.js
@@ -496,8 +496,9 @@ describe('api/uploads/MultiputUpload', () => {
             multiputUploadTest.sha1Worker = {
                 terminate: jest.fn(),
             };
-            multiputUploadTest.xhr.delete = jest.fn();
+            multiputUploadTest.xhr.delete = jest.fn().mockResolvedValue();
             multiputUploadTest.sessionEndpoints.abort = 'foo';
+            multiputUploadTest.sessionId = '123';
 
             multiputUploadTest.abortSession(null, '123', '123');
             expect(multiputUploadTest.xhr.delete).toHaveBeenCalled();

diff --git a/src/elements/content-uploader/ContentUploader.js b/src/elements/content-uploader/ContentUploader.js
@@ -834,13 +834,17 @@ class ContentUploader extends Component<Props, State> {
     handleUploadError = (item: UploadItem, error: Error) => {
         const { onError, useUploadsManager } = this.props;
         const { file } = item;
+        const { items } = this.state;
 
         item.status = STATUS_ERROR;
         item.error = error;
         this.numItemsUploading -= 1;
 
-        const { items } = this.state;
-        items[items.indexOf(item)] = item;
+        const newItems = [...items];
+        const index = newItems.findIndex(singleItem => singleItem === item);
+        if (index !== -1) {
+            newItems[index] = item;
+        }
 
         // Broadcast that there was an error uploading a file
         const errorData = useUploadsManager
@@ -855,7 +859,7 @@ class ContentUploader extends Component<Props, State> {
 
         onError(errorData);
 
-        this.updateViewAndCollection(items);
+        this.updateViewAndCollection(newItems);
 
         if (useUploadsManager) {
             this.isAutoExpanded = true;