From 71f8eaff35b7c972eb8744e5cb982444fe151176 Mon Sep 17 00:00:00 2001
From: Om Singhal <wengsinghal@gmail.com>
Date: Sat, 22 Aug 2026 19:22:52 -0400
Subject: [PATCH] fix(storage): reject uploadFileInChunks when the automatic
 abort succeeds

When a chunked XML multipart upload failed and the automatic abort of the
upload session succeeded, uploadFileInChunks returned undefined instead of
throwing. Callers had no way to tell such an upload apart from a successful
one, so a failed upload looked like a written file. The doc comment on the
method promises an error carrying the uploadId and the map of uploaded
parts, and every other failure path already throws a MultiPartUploadError.

The catch block now falls through to the existing throw after a successful
abort, so the original error is surfaced as a MultiPartUploadError with the
uploadId and the parts map, exactly as it is when the abort itself fails or
when autoAbortFailure is disabled.

The existing unit test for this path used assert.doesNotThrow on a promise
returning function without awaiting it, so it never observed the outcome of
the promise. It now awaits assert.rejects and checks the error type, the
message, the uploadId and the parts map. A second test covers the case
where the abort itself fails. The neighbouring test for the disabled
autoAbortFailure path had an assert.rejects call that was not awaited and
now awaits it as well.

Fixes #9193
---
 handwritten/storage/src/transfer-manager.ts  |  1 -
 handwritten/storage/test/transfer-manager.ts | 64 +++++++++++++++++---
 2 files changed, 54 insertions(+), 11 deletions(-)

diff --git a/handwritten/storage/src/transfer-manager.ts b/handwritten/storage/src/transfer-manager.ts
index 3a17e08a3fe4..407d52eea1c6 100644
--- a/handwritten/storage/src/transfer-manager.ts
+++ b/handwritten/storage/src/transfer-manager.ts
@@ -952,7 +952,6 @@ export class TransferManager {
       ) {
         try {
           await mpuHelper.abortUpload();
-          return;
         } catch (e) {
           throw new MultiPartUploadError(
             (e as Error).message,
diff --git a/handwritten/storage/test/transfer-manager.ts b/handwritten/storage/test/transfer-manager.ts
index 364618cc6f84..9df8860fdece 100644
--- a/handwritten/storage/test/transfer-manager.ts
+++ b/handwritten/storage/test/transfer-manager.ts
@@ -810,7 +810,7 @@ describe('Transfer Manager', () => {
         fakeHelper.abortUpload.resolves();
         return fakeHelper;
       };
-      assert.rejects(
+      await assert.rejects(
         transferManager.uploadFileInChunks(
           filePath,
           {autoAbortFailure: false},
@@ -849,23 +849,27 @@ describe('Transfer Manager', () => {
     });
 
     it('should call abortUpload when a failure occurs after an uploadID is established', async () => {
+      const fakeId = '123';
+      const fakePartsMap = new Map<number, string>([[1, 'abc']]);
       const expectedErr = new MultiPartUploadError(
         'Hello World',
-        '',
-        new Map<number, string>()
+        fakeId,
+        new Map<number, string>(fakePartsMap)
       );
-      const fakeId = '123';
 
       mockGeneratorFunction = (bucket, fileName, uploadId, partsMap) => {
         fakeHelper = sandbox.createStubInstance(FakeXMLHelper);
         fakeHelper.uploadId = uploadId || '';
         fakeHelper.partsMap = partsMap || new Map<number, string>();
-        fakeHelper.initiateUpload.resolves();
-        fakeHelper.uploadPart.callsFake(() => {
+        fakeHelper.initiateUpload.callsFake(() => {
           fakeHelper.uploadId = fakeId;
-          return Promise.reject(expectedErr);
+          return Promise.resolve();
         });
-        fakeHelper.completeUpload.resolves();
+        fakeHelper.uploadPart.callsFake(() => {
+          fakeHelper.partsMap = fakePartsMap;
+          return Promise.resolve();
+        });
+        fakeHelper.completeUpload.rejects(new Error('Hello World'));
         fakeHelper.abortUpload.callsFake(() => {
           assert.strictEqual(fakeHelper.uploadId, fakeId);
           return Promise.resolve();
@@ -873,9 +877,49 @@ describe('Transfer Manager', () => {
         return fakeHelper;
       };
 
-      assert.doesNotThrow(() =>
-        transferManager.uploadFileInChunks(filePath, {}, mockGeneratorFunction)
+      await assert.rejects(
+        transferManager.uploadFileInChunks(filePath, {}, mockGeneratorFunction),
+        (err: unknown) => {
+          assert(err instanceof MultiPartUploadError);
+          assert.deepStrictEqual(err, expectedErr);
+          return true;
+        }
+      );
+      assert.strictEqual(fakeHelper.abortUpload.calledOnce, true);
+    });
+
+    it('should reject with the abort error when abortUpload also fails', async () => {
+      const fakeId = '123';
+      const abortErr = new Error('abort failed');
+      const expectedErr = new MultiPartUploadError(
+        abortErr.message,
+        fakeId,
+        new Map<number, string>()
+      );
+
+      mockGeneratorFunction = (bucket, fileName, uploadId, partsMap) => {
+        fakeHelper = sandbox.createStubInstance(FakeXMLHelper);
+        fakeHelper.uploadId = uploadId || '';
+        fakeHelper.partsMap = partsMap || new Map<number, string>();
+        fakeHelper.initiateUpload.callsFake(() => {
+          fakeHelper.uploadId = fakeId;
+          return Promise.resolve();
+        });
+        fakeHelper.uploadPart.resolves();
+        fakeHelper.completeUpload.rejects(new Error('Hello World'));
+        fakeHelper.abortUpload.rejects(abortErr);
+        return fakeHelper;
+      };
+
+      await assert.rejects(
+        transferManager.uploadFileInChunks(filePath, {}, mockGeneratorFunction),
+        (err: unknown) => {
+          assert(err instanceof MultiPartUploadError);
+          assert.deepStrictEqual(err, expectedErr);
+          return true;
+        }
       );
+      assert.strictEqual(fakeHelper.abortUpload.calledOnce, true);
     });
 
     it('should set the appropriate `GCCL_GCS_CMD_KEY`', async () => {