fix(request): Large file upload memory leak (#453)

Author: sebastian-wec

README.md

@@ -230,6 +230,8 @@ Error contains details field with responseBody, responseHeaders, code (only when
 </script>
 ```
 
+Upload abort throws an `FilestackError` with type `FilestackErrorType.ABORTED`
+
 ## Sentry Integration
 
 If you're using [Sentry](https://sentry.io/welcome/) to monitor your application, Filestack will automatically report upload errors to Sentry, and tag them with helpful diagnostic information via [`@sentry/minimal`](https://github.com/getsentry/sentry-javascript/tree/master/packages/minimal).

src/filestack_error.ts

@@ -18,6 +18,7 @@
 export enum FilestackErrorType {
   VALIDATION = 'validation',
   REQUEST = 'request',
+  ABORTED = 'aborted',
   OTHER = 'other',
 }
 

src/lib/api/upload/uploaders/s3.ts

@@ -18,7 +18,7 @@ import Debug from 'debug';
 import PQueue from 'p-queue';
 
 import { FilestackError, FilestackErrorType } from './../../../../filestack_error';
-import { FsCancelToken, FsRequest, FsRequestError, FsResponse } from './../../../request';
+import { FsCancelToken, FsRequest, FsRequestError, FsRequestErrorCode, FsResponse } from './../../../request';
 import { shouldRetry } from './../../../request/helpers';
 import { filterObject, uniqueId, uniqueTime } from './../../../utils';
 import { File, FilePart, FilePartMetadata, FileState } from './../file';
@@ -331,7 +331,7 @@ export class S3Uploader extends UploaderAbstract {
         debug(`[${id}] Start request error %O`, err);
         this.setPayloadStatus(id, FileState.FAILED);
 
-        return Promise.reject(new FilestackError('Cannot upload file. Start request failed', this.getErrorDetails(err), FilestackErrorType.REQUEST));
+        return this.rejectUpload('Cannot upload file. Start request failed', err);
       });
   }
 
@@ -422,7 +422,7 @@ export class S3Uploader extends UploaderAbstract {
     }).catch(err => {
       this.setPayloadStatus(id, FileState.FAILED);
 
-      return Promise.reject(new FilestackError('Cannot get part metadata', this.getErrorDetails(err), FilestackErrorType.REQUEST));
+      return this.rejectUpload('Cannot get part metadata', err);
     });
   }
 
@@ -507,7 +507,7 @@ export class S3Uploader extends UploaderAbstract {
         return this.startPart(id, partNumber);
       }
 
-      return Promise.reject(new FilestackError('Cannot upload file part', this.getErrorDetails(err), FilestackErrorType.REQUEST));
+      return this.rejectUpload('Cannot upload file part', err);
     });
   }
 
@@ -616,7 +616,7 @@ export class S3Uploader extends UploaderAbstract {
         part = null;
         chunk = null;
 
-        return Promise.reject(new FilestackError('Cannot upload file part (FII)', this.getErrorDetails(err), FilestackErrorType.REQUEST));
+        return this.rejectUpload('Cannot upload file part (FII)', err);
       });
   }
 
@@ -653,7 +653,7 @@ export class S3Uploader extends UploaderAbstract {
         return res;
       })
       .catch(err => {
-        return Promise.reject(new FilestackError('Cannot commit file part metadata', this.getErrorDetails(err), FilestackErrorType.REQUEST));
+        return this.rejectUpload('Cannot commit file part metadata', err);
       });
   }
 
@@ -729,7 +729,7 @@ export class S3Uploader extends UploaderAbstract {
       .catch(err => {
         this.setPayloadStatus(id, FileState.FAILED);
 
-        return Promise.reject(new FilestackError('Cannot complete file', this.getErrorDetails(err), FilestackErrorType.REQUEST));
+        return this.rejectUpload('Cannot complete file', err);
       });
   }
 
@@ -860,7 +860,7 @@ export class S3Uploader extends UploaderAbstract {
    *
    * @param err
    */
-  private getErrorDetails(err: FsRequestError) {
+  private parseError(err: FsRequestError) {
     if (!err.response) {
       return {};
     }
@@ -871,4 +871,11 @@ export class S3Uploader extends UploaderAbstract {
       headers: err.response.headers,
     };
   }
+
+  private rejectUpload(message: string, err: FsRequestError) {
+    if (err instanceof FsRequestError && err.code === FsRequestErrorCode.ABORTED) {
+      return Promise.reject(new FilestackError(message, { reason: err.message }, FilestackErrorType.ABORTED));
+    }
+    return Promise.reject(new FilestackError(message, this.parseError(err), FilestackErrorType.REQUEST));
+  }
 }

src/lib/request/adapters/xhr.ts

@@ -76,7 +76,7 @@ export class XhrAdapter implements AdapterInterface {
       let cancelListener;
 
       if (config.cancelToken) {
-        cancelListener = config.cancelToken.once('cancel', (reason) => {
+        cancelListener = (reason) => {
           /* istanbul ignore next: if request is done cancel token should not throw any error */
           if (request) {
             request.abort();
@@ -85,7 +85,9 @@ export class XhrAdapter implements AdapterInterface {
 
           debug('Request canceled by user %s, config: %O', reason, config);
           return reject(new FsRequestError(`Request aborted. Reason: ${reason}`, config, null, FsRequestErrorCode.ABORTED));
-        });
+        };
+
+        config.cancelToken.once('cancel', cancelListener);
       }
 
       request.onreadystatechange = async () => {
@@ -123,7 +125,8 @@ export class XhrAdapter implements AdapterInterface {
 
         // clear cancel token to avoid memory leak
         if (config.cancelToken) {
-          config.cancelToken.removeListener(cancelListener);
+          config.cancelToken.removeListener('cancel', cancelListener);
+          cancelListener = null;
         }
 
         return resolve(response);