Complete errored uploads and release form loading refs

assets/js/phoenix_live_view/upload_entry.js

@@ -42,6 +42,7 @@ export default class UploadEntry {
     this.meta = null;
     this._isCancelled = false;
     this._isDone = false;
+    this._isErrored = false;
     this._progress = 0;
     this._lastProgressSent = -1;
     this._onCancel = function () {};
@@ -93,13 +94,26 @@ export default class UploadEntry {
   }
 
   error(reason = "failed") {
+    if (this._isErrored) {
+      return;
+    }
+    this._isErrored = true;
+    this._isDone = true;
     this.fileEl.removeEventListener(PHX_LIVE_FILE_UPDATED, this._onElUpdated);
-    this.view.pushFileProgress(this.fileEl, this.ref, { error: reason });
-    if (!this.isAutoUpload()) {
-      LiveUploader.clearFiles(this.fileEl);
+    try {
+      this.view.pushFileProgress(this.fileEl, this.ref, { error: reason });
+      if (!this.isAutoUpload()) {
+        LiveUploader.clearFiles(this.fileEl);
+      }
+    } finally {
+      this._onDone();
     }
   }
 
+  isErrored() {
+    return this._isErrored;
+  }
+
   isAutoUpload() {
     return this.autoUpload;
   }
@@ -116,6 +130,8 @@ export default class UploadEntry {
 
   onDone(callback) {
     this._onDone = () => {
+      // A late progress reply or cancellation must not complete an entry twice.
+      this._onDone = function () {};
       this.fileEl.removeEventListener(PHX_LIVE_FILE_UPDATED, this._onElUpdated);
       callback();
     };

assets/js/phoenix_live_view/view.ts

@@ -2345,14 +2345,21 @@ export default class View {
     const joinCountAtUpload = this.joinCount;
     const inputEls = LiveUploader.activeFileInputs(formEl);
     let numFileInputsInProgress = inputEls.length;
+    let uploadFailed = false;
 
     // get each file input
     inputEls.forEach((inputEl) => {
       const uploader = new LiveUploader(inputEl, this, () => {
         this.activeUploaders.delete(uploader);
+        uploadFailed ||= uploader.entries().some((entry) => entry.isErrored());
         numFileInputsInProgress--;
         if (numFileInputsInProgress === 0) {
-          onComplete();
+          if (uploadFailed) {
+            this.cancelSubmit(formEl, phxEvent);
+            this.undoRefs(ref, phxEvent);
+          } else {
+            onComplete();
+          }
         }
       });
       this.activeUploaders.add(uploader);

assets/test/upload_entry_test.ts

@@ -0,0 +1,38 @@
+import UploadEntry from "phoenix_live_view/upload_entry";
+import LiveUploader from "phoenix_live_view/live_uploader";
+import { PHX_LIVE_FILE_UPDATED } from "phoenix_live_view/constants";
+
+describe("UploadEntry", () => {
+  test.each([false, true])(
+    "an error completes once without waiting for progress (auto upload: %s)",
+    (autoUpload) => {
+      const input = document.createElement("input");
+      input.type = "file";
+      const file = new File(["contents"], "file.txt");
+      LiveUploader.trackFiles(input, [file]);
+      const replies: (() => void)[] = [];
+      const view = {
+        pushFileProgress: jest.fn((_input, _ref, _progress, onReply) => {
+          if (onReply) replies.push(onReply);
+        }),
+      };
+      const entry = new UploadEntry(input, file, view, autoUpload);
+      const onDone = jest.fn();
+      entry.onDone(onDone);
+      entry.progress(100);
+      entry.error("failed");
+      entry.error("failed again");
+      entry.cancel();
+      replies[0]();
+      input.dispatchEvent(new CustomEvent(PHX_LIVE_FILE_UPDATED));
+
+      expect(entry.isDone()).toBe(true);
+      expect(entry.isErrored()).toBe(true);
+      expect(onDone).toHaveBeenCalledTimes(1);
+      expect(view.pushFileProgress).toHaveBeenCalledTimes(2);
+      expect(view.pushFileProgress).toHaveBeenLastCalledWith(input, entry.ref, {
+        error: "failed",
+      });
+    },
+  );
+});

assets/test/view_test.ts

@@ -2,6 +2,8 @@ import { Socket } from "phoenix";
 import { createHook } from "phoenix_live_view/index";
 import LiveSocket from "phoenix_live_view/live_socket";
 import DOM from "phoenix_live_view/dom";
+import LiveUploader from "phoenix_live_view/live_uploader";
+import UploadEntry from "phoenix_live_view/upload_entry";
 import View from "phoenix_live_view/view";
 import ViewHook, { HooksOptions } from "phoenix_live_view/view_hook";
 
@@ -1982,6 +1984,87 @@ describe("View Hooks", function () {
     ]);
   });
 
+  test.each([
+    [false, 1],
+    [true, 1],
+    [false, 2],
+    [true, 2],
+  ])(
+    "upload errors release form refs without submitting (auto upload: %s, inputs: %s)",
+    async (autoUpload, numInputs) => {
+      const entries: UploadEntry[] = [];
+      liveSocket = new LiveSocket("/live", Socket, {
+        uploaders: { Test: (uploads) => entries.push(...uploads) },
+      });
+      const el = liveViewDOM(`
+        <form id="upload-form" phx-submit="save">
+          ${Array.from(
+            { length: Number(numInputs) },
+            (_, i) => `
+            <input id="upload-${i}" type="file" name="files-${i}" multiple
+              data-phx-upload-ref="upload-ref-${i}" data-phx-active-refs=""
+              data-phx-preflighted-refs="" data-phx-done-refs=""
+              ${autoUpload ? 'data-phx-auto-upload=""' : ""}>
+          `,
+          ).join("")}
+          <button id="submit" type="submit">Save</button>
+        </form>
+      `);
+      const view = simulateJoinedView(el, liveSocket);
+      const form = view.el.querySelector("form")!;
+      const button = form.querySelector("button")!;
+      const files = [
+        new File(["first"], "first.txt"),
+        new File(["second"], "second.txt"),
+      ];
+      form.querySelectorAll("input").forEach((input, i) => {
+        LiveUploader.trackFiles(input, numInputs === 1 ? files : [files[i]]);
+        input.setAttribute(
+          "data-phx-active-refs",
+          LiveUploader.activeFiles(input)
+            .map((file) => LiveUploader.genFileRef(file))
+            .join(","),
+        );
+      });
+      const push = jest
+        .spyOn(view, "pushWithReply")
+        .mockImplementation((_ref, event, payload) => {
+          expect(event).toBe("allow_upload");
+          return Promise.resolve({
+            type: "ok",
+            resp: {
+              entries: Object.fromEntries(
+                payload.entries.map((entry) => [
+                  entry.ref,
+                  { uploader: "Test" },
+                ]),
+              ),
+            },
+          } as any);
+        });
+      // No server progress reply is needed to release a failed upload.
+      jest.spyOn(view, "pushFileProgress").mockImplementation(() => {});
+      const onReply = jest.fn();
+      view.pushFormSubmit(form, form, "save", button, {}, onReply);
+      await Promise.resolve();
+      expect(form.classList.contains("phx-submit-loading")).toBe(true);
+      expect(button.disabled).toBe(true);
+      expect(entries).toHaveLength(2);
+
+      entries[0].error("timeout");
+      entries[0].error("closed");
+      entries[0].cancel();
+      expect(view["activeUploaders"].size).toBe(1);
+      entries[1].cancel();
+
+      expect(view["activeUploaders"].size).toBe(0);
+      expect(form.classList.contains("phx-submit-loading")).toBe(false);
+      expect(button.disabled).toBe(false);
+      expect(push).toHaveBeenCalledTimes(Number(numInputs));
+      expect(onReply).not.toHaveBeenCalled();
+    },
+  );
+
   test("dispatches uploads", async () => {
     const hooks = { Recorder: {} };
     const liveSocket = new LiveSocket("/live", Socket, { hooks });