From 0e3625d05d5861561447be3de64e5481e2186bb1 Mon Sep 17 00:00:00 2001 From: Matias Simon Date: Sun, 13 Sep 2026 12:44:30 +0200 Subject: [PATCH] Release the thumbnail queue when a file cannot be read `createThumbnail` only handled `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 `_processingThumbnail` set forever. No file added afterwards ever got a thumbnail, and where `transformFile` goes through `createThumbnail` -- which it does as soon as `resizeWidth` or `resizeHeight` is set -- the upload never started either. Hand the error event to the callback, which is what the `img.onerror` path in `createThumbnailFromUrl` already did, so the file is reported with `dictThumbnailError` and the queue carries on. `DropzoneThumbnailCallback` now declares that: `string | Event` rather than `string`, which is what both failure paths have always passed. The two casts that hid it are gone, and with the real signature in place the compiler found `displayExistingFile` emitting the error event as a thumbnail, so a preview whose image URL failed to load ended up with `img.src` set to "[object Event]". Fixes #2365 Co-Authored-By: Claude Opus 5 --- .changeset/thumbnail-read-error.md | 11 +++++ packages/dropzone/src/dropzone.ts | 49 ++++++++++++++----- packages/dropzone/test/unit-tests/all.js | 46 +++++++++++++++++ .../dropzone/test/unit-tests/public-api.js | 14 ++++++ 4 files changed, 107 insertions(+), 13 deletions(-) create mode 100644 .changeset/thumbnail-read-error.md diff --git a/.changeset/thumbnail-read-error.md b/.changeset/thumbnail-read-error.md new file mode 100644 index 000000000..6c9959597 --- /dev/null +++ b/.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]"`. diff --git a/packages/dropzone/src/dropzone.ts b/packages/dropzone/src/dropzone.ts index a4e584576..d489e2cd1 100644 --- a/packages/dropzone/src/dropzone.ts +++ b/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!); diff --git a/packages/dropzone/test/unit-tests/all.js b/packages/dropzone/test/unit-tests/all.js index 3a71785c8..8a9c69fe7 100644 --- a/packages/dropzone/test/unit-tests/all.js +++ b/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 () {}; diff --git a/packages/dropzone/test/unit-tests/public-api.js b/packages/dropzone/test/unit-tests/public-api.js index a643a949d..8112086ec 100644 --- a/packages/dropzone/test/unit-tests/public-api.js +++ b/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