fix: renterd multipart etag error

.changeset/tricky-tables-know.md

@@ -0,0 +1,5 @@
+---
+'renterd': minor
+---
+
+Uploads will now error and abort if responses are missing etags.

apps/renterd/lib/multipartUpload.spec.ts

@@ -1,5 +1,9 @@
 import { Response, delay } from '@siafoundation/react-core'
-import { MultipartParams, MultipartUpload } from './multipartUpload'
+import {
+  ErrorNoETag,
+  MultipartParams,
+  MultipartUpload,
+} from './multipartUpload'
 
 describe('MultipartUpload', () => {
   it('should report progress and complete with serial parts', async () => {
@@ -190,6 +194,39 @@ describe('MultipartUpload', () => {
     expect(elapsedTime).toBeGreaterThanOrEqual(3500)
   }, 10_000)
 
+  it('should abort the entire upload and error if a part is missing an etag', async () => {
+    // note that the upload mock is configured to report progress 2 times per part
+    const partSize = 2
+    const params = getMockedParams({
+      file: new File(['012456'], 'test.txt', { type: 'text/plain' }),
+      partSize,
+      apiWorkerUploadPart: buildMockApiWorkerUploadPart({
+        partSize,
+        failures: [{ failCallIndex: 1, failPartIndex: 1, type: 'missingEtag' }],
+      }),
+      maxConcurrentParts: 1,
+    })
+    const multipartUpload = new MultipartUpload(params)
+    await multipartUpload.create()
+    await multipartUpload.start()
+    expect(params.onProgress.mock.calls.length).toBe(4)
+    expect(
+      params.onProgress.mock.calls.map(([params]) => [
+        params.sent,
+        params.total,
+        params.percentage,
+      ])
+    ).toEqual([
+      [1, 6, 17], // call 0
+      [2, 6, 33],
+      [3, 6, 50], // call 1
+      [4, 6, 67], // fail
+    ])
+    expect(params.apiBusUploadComplete.post).not.toHaveBeenCalled()
+    expect(params.onComplete).not.toHaveBeenCalled()
+    expect(params.onError).toHaveBeenCalledWith(expect.any(ErrorNoETag))
+  })
+
   it('should handle an upload create error', async () => {
     const params = getMockedParams()
     const multipartUpload = new MultipartUpload({
@@ -255,6 +292,7 @@ function getMockedParams(params?: Partial<Record<keyof MultipartParams, any>>) {
 type Failure = {
   failCallIndex: number
   failPartIndex: number
+  type?: 'partFailed' | 'missingEtag'
 }
 
 function buildMockApiWorkerUploadPart({
@@ -284,22 +322,24 @@ function buildMockApiWorkerUploadPart({
             reject(new Error('Abort'))
             return
           }
-          const shouldFail = failures.find(
+          const failure = failures.find(
             (failure) =>
               callIndex === failure.failCallIndex &&
               partIndex === failure.failPartIndex
           )
           loaded += progressPartSize
           onUploadProgress({ type: 'progress', loaded, total })
-          if (shouldFail) {
+          if ((failure && !failure.type) || failure?.type === 'partFailed') {
             clearInterval(intervalId)
             reject(new Error('Upload failed'))
           } else {
             if (loaded >= partSize) {
               clearInterval(intervalId)
               resolve({
                 status: 200,
-                headers: { etag: eTag },
+                headers: {
+                  etag: failure?.type === 'missingEtag' ? undefined : eTag,
+                },
               })
             }
             partIndex++

apps/renterd/lib/multipartUpload.ts

@@ -139,6 +139,7 @@ export class MultipartUpload {
     } catch (e) {
       triggerErrorToast(e.message)
     }
+    this.#resolve()
   }
 
   public setOnProgress(
@@ -195,7 +196,15 @@ export class MultipartUpload {
       // On successful upload, reset the delay
       this.#resetDelay()
     } catch (error) {
-      if (error.name === 'canceled') {
+      // if the upload was canceled, don't retry
+      if (error instanceof ErrorCanceledRequest) {
+        return
+      }
+      // if the upload failed due to a missing ETag, abort the entire upload
+      // and report the error
+      if (error instanceof ErrorNoETag) {
+        await this.abort()
+        this.#onError(error)
         return
       }
       this.#pendingPartNumbers.push(partNumber)
@@ -282,26 +291,40 @@ export class MultipartUpload {
 
       // errors such as aborted/canceled request
       if (response.error) {
+        if (response.error === 'canceled') {
+          throw new ErrorCanceledRequest()
+        }
         throw new Error(response.error)
       }
 
       const eTag = response.headers['etag']
       if (!eTag) {
-        throw new Error(
-          'No ETag in response, add ETag to Access-Control-Expose-Headers list'
-        )
+        throw new ErrorNoETag()
       }
+
       const uploadedPart = {
         partNumber: partNumber,
         // removing the " enclosing characters from the raw ETag
         eTag: eTag.replace(/"/g, ''),
       }
 
       this.#uploadedParts.push(uploadedPart)
+    } finally {
       delete this.#activeConnections[partNumber]
-    } catch (e) {
-      delete this.#activeConnections[partNumber]
-      throw e
     }
   }
 }
+
+export class ErrorCanceledRequest extends Error {
+  constructor() {
+    super('canceled')
+    this.name = 'CanceledError'
+  }
+}
+
+export class ErrorNoETag extends Error {
+  constructor() {
+    super('No ETag in response, add ETag to Access-Control-Expose-Headers list')
+    this.name = 'NoETagError'
+  }
+}