Release the thumbnail queue when a file cannot be read

.changeset/thumbnail-read-error.md

@@ -0,0 +1,11 @@
+---
+"dropzone": patch
+---
+
+Fix the thumbnail queue deadlocking when a file cannot be read.
+
+`createThumbnail` only listened for `FileReader`'s `load` event. A file that had been moved, locked by another process, or was otherwise unreadable since it was dropped fires `error` instead, so the callback was never invoked and `_processThumbnailQueue` kept its lock forever: no file added afterwards got a thumbnail, and with `resizeWidth`/`resizeHeight` or a `transformFile` that uses `createThumbnail`, the upload never started either.
+
+The read error now reaches the callback the same way an undecodable image already did, so the file gets `dictThumbnailError` and the queue moves on.
+
+`DropzoneThumbnailCallback` says what it has always done, too: its first argument is `string | Event`, the error event standing in for the data URL when no thumbnail could be produced. That also fixes `displayExistingFile`, which used to emit that event as a thumbnail when the image URL failed to load, leaving the preview with `img.src` set to `"[object Event]"`.

packages/dropzone/src/dropzone.ts

@@ -68,9 +68,14 @@ export type DropzoneTransformCallback = (file: DropzoneFile | Blob) => void;
 /**
  * Invoked with the rendered thumbnail as a data URL, and the canvas it was
  * drawn on -- which is null when the image needed no resizing.
+ *
+ * There is no separate error callback: when the thumbnail cannot be produced,
+ * because the file cannot be read or the image cannot be decoded, this is
+ * invoked with the error event in place of the data URL. Check with
+ * `typeof dataUrl === "string"` before using it.
  */
 export type DropzoneThumbnailCallback = (
-  dataUrl: string,
+  dataUrl: string | Event,
   canvas?: HTMLCanvasElement | null,
 ) => void;
 
@@ -979,11 +984,11 @@ export default class Dropzone extends Emitter {
       this.options.thumbnailHeight,
       this.options.thumbnailMethod,
       true,
-      (dataUrl: string) => {
-        // `createThumbnailFromUrl` hands its callback the error event when the
-        // image cannot be decoded, so anything that is not a data URL means
-        // the thumbnail failed. Emitting it as one would set the preview's
-        // `img.src` to "[object Event]" and render a broken image. See #2218.
+      (dataUrl: string | Event) => {
+        // Both failure paths -- the file not being readable, and the image
+        // not being decodable -- hand the callback the error event. Emitting
+        // that as a thumbnail would set the preview's `img.src` to
+        // "[object Event]" and render a broken image. See #2218 and #2365.
         if (typeof dataUrl === "string") {
           this.emit("thumbnail", file, dataUrl);
         } else {
@@ -1039,9 +1044,10 @@ export default class Dropzone extends Emitter {
       height,
       resizeMethod,
       true,
-      (dataUrl: string, canvas?: HTMLCanvasElement | null) => {
+      (dataUrl: string | Event, canvas?: HTMLCanvasElement | null) => {
         if (canvas == null) {
-          // The image has not been resized
+          // The image has not been resized, or could not be read or decoded at
+          // all -- either way there is nothing to send but the original file.
           return callback(file);
         } else {
           let { resizeMimeType } = this.options;
@@ -1094,6 +1100,18 @@ export default class Dropzone extends Emitter {
       this.createThumbnailFromUrl(file, width, height, resizeMethod, fixOrientation, callback);
     };
 
+    // A file that cannot be read -- moved, locked by another process, or on a
+    // drive that went away since it was dropped -- fires `error` and never
+    // `load`. Without this the callback is never invoked at all, which leaves
+    // `_processThumbnailQueue` holding its lock forever: no later file gets a
+    // thumbnail, and an upload waiting on `transformFile` never sends. See
+    // #2365.
+    fileReader.onerror = (e) => {
+      if (callback != null) {
+        callback(e);
+      }
+    };
+
     fileReader.readAsDataURL(file);
   }
 
@@ -1117,8 +1135,14 @@ export default class Dropzone extends Emitter {
       this.emit("thumbnail", mockFile, imageUrl);
       if (callback) callback();
     } else {
-      let onDone = (thumbnail: string) => {
-        this.emit("thumbnail", mockFile, thumbnail);
+      let onDone = (thumbnail: string | Event) => {
+        // An image URL that does not load -- gone from the server, or blocked
+        // by CORS -- arrives here as the error event. Emitting that as a
+        // thumbnail would set the preview's `img.src` to "[object Event]", so
+        // leave the preview alone and just report that we are done.
+        if (typeof thumbnail === "string") {
+          this.emit("thumbnail", mockFile, thumbnail);
+        }
         if (callback) callback();
       };
       mockFile.dataURL = imageUrl;
@@ -1247,9 +1271,8 @@ export default class Dropzone extends Emitter {
 
     if (callback != null) {
       // The same callback does double duty: it receives the thumbnail on
-      // success, and is called as the image's error handler on failure, where
-      // an Event arrives instead of a data URL.
-      img.onerror = callback as unknown as OnErrorEventHandler;
+      // success, and the error event on failure.
+      img.onerror = (e) => callback(e);
     }
 
     return (img.src = file.dataURL!);

packages/dropzone/test/unit-tests/all.js

@@ -1363,6 +1363,52 @@ describe("Dropzone", function () {
             dropzone.addFile(corrupt);
           }));
 
+        it("should emit an error and keep the queue moving if a file can't be read", async function () {
+          dropzone.processFile = function () {};
+          dropzone.uploadFile = function () {};
+
+          // A file that has gone away since it was dropped, or is locked by
+          // another process, makes FileReader fire `error` and never `load`.
+          // See #2365.
+          let readAsDataURL = vi
+            .spyOn(FileReader.prototype, "readAsDataURL")
+            .mockImplementation(function () {
+              setTimeout(() => this.dispatchEvent(new ProgressEvent("error")), 0);
+            });
+
+          let unreadable = getMockFile("image/png", "unreadable.png");
+
+          let message = await new Promise(function (resolve) {
+            dropzone.on("error", function (file, message) {
+              if (file === unreadable) resolve(message);
+            });
+            dropzone.addFile(unreadable);
+          });
+
+          expect(message).toBe(dropzone.options.dictThumbnailError);
+          // The lock has to be released, or nothing queued behind the failed
+          // file is ever processed again.
+          expect(dropzone._processingThumbnail).toBe(false);
+
+          readAsDataURL.mockRestore();
+
+          let readable = await new Promise(function (resolve) {
+            let canvas = document.createElement("canvas");
+            canvas.width = canvas.height = 10;
+            canvas.toBlob(
+              (blob) => resolve(new File([blob], "readable.png", { type: "image/png" })),
+              "image/png",
+            );
+          });
+
+          let thumbnailed = await new Promise(function (resolve) {
+            dropzone.on("thumbnail", (file) => resolve(file));
+            dropzone.addFile(readable);
+          });
+
+          expect(thumbnailed).toBe(readable);
+        });
+
         it("should not let the thumbnail itself be dragged", function () {
           dropzone.processFile = function () {};
           dropzone.uploadFile = function () {};

packages/dropzone/test/unit-tests/public-api.js

@@ -66,6 +66,20 @@ describe("public API", function () {
       expect(dropzone.createThumbnailFromUrl).toHaveBeenCalledTimes(1);
     });
 
+    it("should not emit a thumbnail when the image url fails to load", async function () {
+      create();
+      let thumbnail = null;
+      dropzone.on("thumbnail", (file, url) => (thumbnail = url));
+
+      // A URL that never resolves to an image: the preview would otherwise be
+      // handed the error event and set `img.src` to "[object Event]".
+      await new Promise((done) =>
+        dropzone.displayExistingFile(mockFile(), "/does-not-exist.png", done),
+      );
+
+      expect(thumbnail).toBe(null);
+    });
+
     // #2003 and the 7.0 roadmap: files added this way never reach this.files,
     // so maxFiles cannot see them. The documented workaround is to push them
     // by hand, which is why fixing it is a breaking change rather than a