Skip to content

Don't take the queue down when cancelling a transforming file - #2379

Merged
enyo merged 1 commit into
mainfrom
cancel-during-transform
Sep 24, 2026
Merged

enyo merged 1 commit into
mainfrom
cancel-during-transform

Conversation

@enyo

@enyo enyo commented Sep 24, 2026

Copy link
Copy Markdown
Owner

Refs #2231.

transformFile is asynchronous — resizing or compressing a large image, or asking a server to presign the upload — and processFile has already set the file to UPLOADING before it runs. For that whole window the file has no xhr, and cancelUpload got it wrong in two separate ways.

The queue gets cancelled too

let groupedFiles = this._getFilesWithXhr(file.xhr!);

_getFilesWithXhr filters on file.xhr === xhr, so passing undefined matches every file whose xhr is also undefined — every file still sitting in the queue. Cancelling one upload set them all to CANCELED and emitted a canceled event for each. Against main:

expected [ 'uploading', 'queued1', 'queued2' ] to deeply equal [ 'uploading' ]

The grouping exists for uploadMultiple, where several files share one request. With no request there is nobody to share it with, so the group is just the file. That also removes the ! — file.xhr is genuinely optional here, and the assertion was covering for exactly the case that was broken.

And the cancelled file is uploaded anyway

Nothing rechecked the status when the transform came back, so the callback went straight on to _uploadData. Cancel a file mid-transform and it was still sent — which is the "removes from the dropzone but still sends to the server" in #2231. Against main:

expected Captured{ readyState: 1, …(19) } to be undefined

uploadFiles now returns early when every file in the batch has been cancelled.

Scope

Two tests in .cancelUpload(), one per half, both failing against main as above.

One case this deliberately does not solve: with uploadMultiple, cancelling one file of a batch mid-transform still sends that file's data, because the batch is a single request whose parts are already aligned with files. Dropping a member would shift the _getParamName(i) indices the server sees, so it wants its own change. Every file in the batch being cancelled — which is what cancelUpload produces once the request exists — is handled.

`transformFile` is asynchronous, and a file is already UPLOADING while it
runs. For that window it has no `xhr`, which broke `cancelUpload` twice
over.

`_getFilesWithXhr(file.xhr)` was called with `undefined` and matched
every file whose `xhr` was also undefined -- that is, everything still
queued. Cancelling one upload set them all to CANCELED and emitted a
`canceled` event for each. Group by the request only when there is one;
without it the file is its own group.

Then, because nothing rechecked the status, the transform callback went
on to upload the file the user had just cancelled. Bail out instead when
every file in the batch has been cancelled.

Refs #2231
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 79.91% 895 / 1120
🔵 Statements 80.35% 945 / 1176
🔵 Functions 93.45% 200 / 214
🔵 Branches 77.25% 530 / 686
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/dropzone/src/dropzone.ts 78.12% 75.88% 93.37% 77.53% 238, 248, 273, 279-282, 367, 410-411, 470, 582, 604, 623-624, 731, 754-757, 794-796, 826-829, 852, 1026, 1056, 1060, 1149, 1183, 1193-1195, 1212-1213, 1219-1252, 1381, 1447, 1496-1497, 1625, 1649-1650, 1704, 1738, 1758-1789, 1797, 1811-1815, 1826, 1839-1844, 1857-1858, 1862-1863, 1881-1882, 1960-1975, 2020, 2036, 2207-2209, 2244, 2254, 2287-2321, 2328-2429, 2438-2441
Generated in workflow #159 for commit eae9121 by the Vitest Coverage Report Action

@enyo
enyo merged commit f5b122d into main Sep 24, 2026
3 checks passed
@enyo
enyo deleted the cancel-during-transform branch September 24, 2026 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant