HUB-11880: treat etags as optional in upload (#112)

HUB-11880 (Make etags optional in multipart upload response)

Author: rettrich-tng

.changeset/moody-sides-behave.md

@@ -0,0 +1,6 @@
+---
+"@knime/utils": minor
+"@knime/hub-features": patch
+---
+
+treat etags in upload as optional, adjust docs

eslint-suppressions.json

@@ -272,11 +272,6 @@
       "count": 1
     }
   },
-  "packages/hub-features/src/useFileUpload/useFileUpload.ts": {
-    "@typescript-eslint/no-floating-promises": {
-      "count": 1
-    }
-  },
   "packages/jsonforms/src/JsonFormsDialog.vue": {
     "@typescript-eslint/no-misused-promises": {
       "count": 1

packages/hub-features/src/useFileUpload/useFileUpload.ts

@@ -81,7 +81,8 @@ type UseFileUploadOptions = {
  * Once the upload of a single file is complete, the upload manager will trigger the `onFileUploadComplete`
  * callback, which then will supply the ids of all the different parts that got uploaded for a given `uploadId`
  * Then we need to "complete" the upload by sending a request which provides the `uploadId` and a list of
- * the ids of all the parts
+ * the ids of all the parts. Note that some backends don't return the ids of the file parts after a successful
+ * upload of a part, in this case empty strings or null can be used instead of the part id.
  *
  * Cancellation:
  * Alternative to completion, you can also cancel an upload. For this, the composable makes a DELETE request
@@ -285,6 +286,8 @@ export const useFileUpload = (options: UseFileUploadOptions = {}) => {
           }
         });
 
+        // accept floating promise, errors are handled inside the function
+        // eslint-disable-next-line @typescript-eslint/no-floating-promises
         useUploadManagerResult.start(parentId, uploadPayload);
       } catch (error) {
         throw rfcErrors.tryParse(error) ?? error;
@@ -296,7 +299,7 @@ export const useFileUpload = (options: UseFileUploadOptions = {}) => {
 
     cancel: (uploadId: string) => {
       useUploadManagerResult.cancel(uploadId);
-      cancelUpload(uploadId).catch((error) => {
+      cancelUpload(uploadId).catch((error: unknown) => {
         consola.error("There was a problem cancelling the upload", { error });
       });
     },

packages/utils/src/uploadManager/__tests__/index.test.ts

@@ -23,6 +23,7 @@ describe("uploadManager", () => {
   };
 
   const setup = ({ uploadPartResponse }: Options = {}) => {
+    consola.mockTypes(() => vi.fn());
     const server = newServer({
       put: ["/success", uploadPartResponse ?? defaultUploadPartResponse],
     });
@@ -126,6 +127,28 @@ describe("uploadManager", () => {
       expect(cancelled.length).toBe(0);
       expect(resolveFilePartUploadURL).toHaveBeenCalled();
     });
+
+    it("should succeed when etag is missing from upload part response", async () => {
+      setup({
+        uploadPartResponse: (request) => {
+          request.respond(201, {});
+        },
+      });
+
+      const { uploadFiles } = createUploadManager({
+        resolveFilePartUploadURL,
+      });
+
+      const uploadId = getUploadId();
+
+      const { completed, failed, cancelled } = await uploadFiles([
+        { uploadId, file },
+      ]);
+
+      expect(completed).toEqual([uploadId]);
+      expect(failed.length).toBe(0);
+      expect(cancelled.length).toBe(0);
+    });
   });
 
   describe("callbacks", () => {
@@ -274,28 +297,5 @@ describe("uploadManager", () => {
       expect(cancelled.length).toBe(0);
       expect(resolveFilePartUploadURL).toHaveBeenCalled();
     });
-
-    it("should fail when etag is missing from upload part response", async () => {
-      setup({
-        uploadPartResponse: (request) => {
-          request.respond(200, {});
-        },
-      });
-
-      const { uploadFiles } = createUploadManager({
-        resolveFilePartUploadURL,
-      });
-
-      const uploadId = getUploadId();
-
-      const { completed, failed, cancelled } = await uploadFiles([
-        { uploadId, file },
-      ]);
-
-      expect(completed.length).toBe(0);
-      expect(failed).toEqual([uploadId]);
-      expect(cancelled.length).toBe(0);
-      expect(resolveFilePartUploadURL).toHaveBeenCalled();
-    });
   });
 });

packages/utils/src/uploadManager/index.ts

@@ -28,6 +28,7 @@ const uploadChunkWithProgress = (params: {
   const { method, url, chunk, chunkIndex, onProgress, abortSignal } = params;
 
   const OK = 200;
+  const CREATED = 201;
 
   return new Promise<{ partId: string }>((resolve, reject) => {
     const xhr = new XMLHttpRequest();
@@ -44,17 +45,13 @@ const uploadChunkWithProgress = (params: {
     };
 
     xhr.onload = () => {
-      if (xhr.status === OK) {
+      if (xhr.status === OK || xhr.status === CREATED) {
         const etag = xhr.getResponseHeader("ETag");
 
-        if (!etag) {
-          reject(new Error("Invalid part upload response"));
-          return;
-        }
-
-        // remove possible doublequotes (") around the etag and/or weak identifier (if either is present)
+        // 1. note that some backends don't return the etag header, then we just set an empty string
+        // 2. remove possible doublequotes (") around the etag and/or weak identifier (if either is present)
         // https://datatracker.ietf.org/doc/html/rfc7232#section-2.3
-        resolve({ partId: etag.replace(/^(?:W\/)?"|"$/g, "") });
+        resolve({ partId: etag?.replace(/^(?:W\/)?"|"$/g, "") ?? "" });
       } else {
         consola.error("Failed or unexpected XHR response", {
           xhrStatus: xhr.status,