From 4c8743291f9eaab260f781fff5c3141506c7ab1b Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Thu, 27 Aug 2026 16:58:57 -0300 Subject: [PATCH 01/15] chore: Fix chunk upload error handling - release timeout slot, dedupe rows, surface status, route polling failures to error UI --- src/components/inputs/dropzone/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/inputs/dropzone/index.js b/src/components/inputs/dropzone/index.js index 6f8846b7..c1178f0c 100644 --- a/src/components/inputs/dropzone/index.js +++ b/src/components/inputs/dropzone/index.js @@ -172,7 +172,7 @@ export class DropzoneJS extends React.Component { return; } else if (data.status === 'error') { this.stopPolling(file); - this.reportPollingError(file, data.message || 'Upload failed'); + this.reportPollingError(file, data.message || data.error || 'Upload failed'); return; } // any other status (e.g. 'uploading') means keep polling From 3226aece39be4202f512174ca26501f4bbfb6e94 Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Thu, 27 Aug 2026 17:37:08 -0300 Subject: [PATCH 02/15] chore: pass xdr status to onError --- .../inputs/upload-input-v3/index.js | 42 ++++++++++++------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/src/components/inputs/upload-input-v3/index.js b/src/components/inputs/upload-input-v3/index.js index 24780a18..fbbedfe4 100644 --- a/src/components/inputs/upload-input-v3/index.js +++ b/src/components/inputs/upload-input-v3/index.js @@ -43,6 +43,32 @@ const formatFileSize = (bytes) => { return `${Math.round(bytes / 1024)} KB`; }; +/** + * Maps the fixed error strings Dropzone and pollUploadStatus report onto translated messages. + * Anything else is a server-provided message and is passed through untouched. + */ +const resolveErrorMessage = (text, status) => { + // status 0 means the request never got a real server response (connection dropped/timed + // out) - Dropzone's own message for that case is "Server responded with 0 code.", which + // is not something a user can act on. + if (status === 0) return T.translate('upload_input_v3.network_error'); + + switch (text) { + case 'Network error': + return T.translate('upload_input_v3.network_error'); + case 'Upload timed out': + return T.translate('upload_input_v3.upload_timed_out'); + case 'Upload failed': + return T.translate('upload_input_v3.upload_failed'); + case 'Auth error': + return T.translate('upload_input_v3.auth_error'); + case 'Max files reached.': + return T.translate('upload_input_v3.max_files_reached'); + default: + return text; + } +}; + const UploadInputV3 = ({ value = [], onRemove, @@ -223,21 +249,7 @@ const UploadInputV3 = ({ ? message : (message?.message ?? T.translate('upload_input_v3.upload_failed')); - // status 0 means the request never got a real server response (connection dropped/timed - // out) - Dropzone's own message for that case is "Server responded with 0 code.", which - // is not something a user can act on. - const displayMessage = - status === 0 || text === 'Network error' - ? T.translate('upload_input_v3.network_error') - : text === 'Upload timed out' - ? T.translate('upload_input_v3.upload_timed_out') - : text === 'Upload failed' - ? T.translate('upload_input_v3.upload_failed') - : text === 'Auth error' - ? T.translate('upload_input_v3.auth_error') - : text === 'Max files reached.' - ? T.translate('upload_input_v3.max_files_reached') - : text; + const displayMessage = resolveErrorMessage(text, status); setErrorFiles(prev => { const existingIndex = prev.findIndex(f => f.name === file.name && f.size === file.size); From f717646dd4232a971dbb9d9343242865a69fc43d Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Thu, 27 Aug 2026 18:30:01 -0300 Subject: [PATCH 03/15] v5.0.58-beta.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 14f6e9bb..1e862b46 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "openstack-uicore-foundation", - "version": "5.0.65", + "version": "5.0.58-beta.0", "description": "ui reactjs components for openstack marketing site", "main": "lib/openstack-uicore-foundation.js", "scripts": { From 788b2032930cc7c82178f8e24f39bb7df8a769c2 Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Fri, 4 Sep 2026 18:59:29 -0300 Subject: [PATCH 04/15] chore: pr review --- src/components/inputs/dropzone/index.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/components/inputs/dropzone/index.js b/src/components/inputs/dropzone/index.js index c1178f0c..c4e745ff 100644 --- a/src/components/inputs/dropzone/index.js +++ b/src/components/inputs/dropzone/index.js @@ -172,7 +172,11 @@ export class DropzoneJS extends React.Component { return; } else if (data.status === 'error') { this.stopPolling(file); +<<<<<<< HEAD this.reportPollingError(file, data.message || data.error || 'Upload failed'); +======= + this.reportPollingError(file, data.message || 'Upload failed'); +>>>>>>> 68fd188 (chore: pr review) return; } // any other status (e.g. 'uploading') means keep polling From 84088fa69a526e8dccdc920ac6b19f4e9d8b4563 Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Thu, 17 Sep 2026 12:51:29 -0300 Subject: [PATCH 05/15] v5.0.58-beta.2 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 1e862b46..cef51c6f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "openstack-uicore-foundation", - "version": "5.0.58-beta.0", + "version": "5.0.58-beta.2", "description": "ui reactjs components for openstack marketing site", "main": "lib/openstack-uicore-foundation.js", "scripts": { From 422b4becc9b52ebd212d34f99941aed0f34581f2 Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Thu, 17 Sep 2026 12:53:10 -0300 Subject: [PATCH 06/15] v5.0.58-beta.3 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index cef51c6f..5da27d3c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "openstack-uicore-foundation", - "version": "5.0.58-beta.2", + "version": "5.0.58-beta.3", "description": "ui reactjs components for openstack marketing site", "main": "lib/openstack-uicore-foundation.js", "scripts": { From 1470de70e65ca67e938bf28908ef98fae1a9e367 Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Thu, 17 Sep 2026 12:57:28 -0300 Subject: [PATCH 07/15] v5.0.58-beta.4 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 5da27d3c..6db5a718 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "openstack-uicore-foundation", - "version": "5.0.58-beta.3", + "version": "5.0.58-beta.4", "description": "ui reactjs components for openstack marketing site", "main": "lib/openstack-uicore-foundation.js", "scripts": { From 1832746f28611ae7da6146d59f4951430bb6fdf9 Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Thu, 27 Aug 2026 18:30:01 -0300 Subject: [PATCH 08/15] v5.0.58-beta.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 6db5a718..1e862b46 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "openstack-uicore-foundation", - "version": "5.0.58-beta.4", + "version": "5.0.58-beta.0", "description": "ui reactjs components for openstack marketing site", "main": "lib/openstack-uicore-foundation.js", "scripts": { From 468a16dda5b8fc20e2c44bfe330eb9973cb01eea Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Thu, 3 Sep 2026 19:17:44 -0300 Subject: [PATCH 09/15] feat: create a retry ledger so that on retries it only uploads missing chunks --- .../__tests__/dropzone-resume.test.js | 304 ++++++++++++++++++ .../dropzone/__tests__/upload-ledger.test.js | 129 ++++++++ src/components/inputs/dropzone/index.js | 105 ++++++ .../inputs/dropzone/upload-ledger.js | 111 +++++++ .../__tests__/upload-input-v3.test.js | 56 +++- .../inputs/upload-input-v3/index.js | 30 ++ src/i18n/en.json | 3 +- 7 files changed, 735 insertions(+), 3 deletions(-) create mode 100644 src/components/inputs/dropzone/__tests__/dropzone-resume.test.js create mode 100644 src/components/inputs/dropzone/__tests__/upload-ledger.test.js create mode 100644 src/components/inputs/dropzone/upload-ledger.js diff --git a/src/components/inputs/dropzone/__tests__/dropzone-resume.test.js b/src/components/inputs/dropzone/__tests__/dropzone-resume.test.js new file mode 100644 index 00000000..9909dd87 --- /dev/null +++ b/src/components/inputs/dropzone/__tests__/dropzone-resume.test.js @@ -0,0 +1,304 @@ +/** + * Copyright 2018 OpenStack Foundation + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + **/ + +import React from 'react'; +import { render, cleanup } from '@testing-library/react'; +import { DropzoneJS } from '../index'; +import { getOrCreateUploadLedger, acknowledgeChunk, isChunkAcknowledged } from '../upload-ledger'; + +jest.mock('../../../security/methods', () => ({ + getAccessToken: jest.fn(() => Promise.resolve('mock-token')), + initLogOut: jest.fn() +})); + +jest.mock('../../../../utils/crypto', () => ({ + getMD5: jest.fn(() => Promise.resolve('mock-md5-hash')) +})); + +// A separate mock module instance from dropzone.test.js's - each test file gets its own +// jest module registry, so extending this shape (vs. the other suite's) carries no risk +// of changing behavior other tests already rely on. +let mockCapturedOptions = {}; +// _originalUploadData ends up as a bound-native-function (not a jest mock, since +// Function.prototype.bind on a jest.fn() drops its .mock tracking) - assertions use +// this captured reference to the pre-bind mock instead. +let mockUploadDataFn; + +jest.mock('dropzone', () => { + return jest.fn().mockImplementation((element, options) => { + mockCapturedOptions = options; + mockUploadDataFn = jest.fn(); + const dz = { + options, + _uploadData: mockUploadDataFn, + _getChunk: jest.fn((file, xhr) => + (file.upload?.chunks || []).find((c) => c && c.xhr === xhr) + ), + uploadFiles: jest.fn(), + on: jest.fn(), + off: jest.fn(), + destroy: jest.fn(() => null), + getActiveFiles: jest.fn(() => []) + }; + dz.emit = jest.fn((event, ...args) => { + dz.on.mock.calls + .filter(([evt]) => evt === event) + .forEach(([, handler]) => handler(...args)); + }); + return dz; + }); +}); + +const getEventHandler = (instance, eventName) => { + const call = instance.dropzone.on.mock.calls + .slice() + .reverse() + .find(([evt]) => evt === eventName); + return call ? call[1] : null; +}; + +describe('DropzoneJS - Resumable Chunked Uploads', () => { + const defaultProps = { + id: 'test-namespace', + config: { postUrl: 'https://example.com/upload' }, + djsConfig: { chunking: true, chunkSize: 1000, maxFilesize: 100 }, + eventHandlers: {}, + data: {}, + uploadCount: 0 + }; + + beforeEach(() => { + jest.clearAllMocks(); + mockCapturedOptions = {}; + window.localStorage.clear(); + }); + + afterEach(() => { + cleanup(); + }); + + const mountInstance = (props = {}) => { + const ref = React.createRef(); + render(); + return ref.current; + }; + + test('accept() reassigns dzuuid to the ledger id and seeds progress from acked chunks', async () => { + const seeded = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); + acknowledgeChunk(seeded, 0); + acknowledgeChunk(seeded, 1); + + const instance = mountInstance(); + const file = { name: 'video.mp4', size: 5000, upload: { uuid: 'dropzone-own-random-uuid' } }; + + await mockCapturedOptions.accept(file, jest.fn()); + + expect(file.upload.uuid).toBe(seeded.uploadId); + expect(file._resumeLedger.ackedChunks).toEqual([0, 1]); + expect(file._completedBytes).toBe(2000); + expect(instance.dropzone.emit).toHaveBeenCalledWith('uploadprogress', file, 40, 2000); + }); + + test('a fresh file (no prior ledger) gets a new random id and no progress seed', async () => { + const instance = mountInstance(); + const file = { name: 'video.mp4', size: 5000, upload: { uuid: 'dropzone-own-random-uuid' } }; + + await mockCapturedOptions.accept(file, jest.fn()); + + expect(file.upload.uuid).not.toBe('dropzone-own-random-uuid'); + expect(file._resumeLedger.ackedChunks).toEqual([]); + expect(file._completedBytes).toBeUndefined(); + expect(instance.dropzone.emit).not.toHaveBeenCalledWith('uploadprogress', expect.anything(), expect.anything(), expect.anything()); + }); + + test('a chunk already acknowledged is skipped: never dispatched, never occupies a slot', () => { + const ledger = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); + acknowledgeChunk(ledger, 0); + + const instance = mountInstance(); + const chunk0 = { index: 0 }; + const chunk1 = { index: 1 }; + const file = { + name: 'video.mp4', + size: 5000, + _resumeLedger: ledger, + upload: { chunks: [chunk0, chunk1], finishedChunkUpload: jest.fn() } + }; + instance.chunksInFlight = 3; + + instance.dropzone._uploadData([file], [{ chunkIndex: 0 }]); + + expect(mockUploadDataFn).not.toHaveBeenCalled(); + expect(file.upload.finishedChunkUpload).toHaveBeenCalledWith(chunk0); + expect(file._completedBytes).toBe(1000); + // never took a concurrency slot, so there is none to release + expect(instance.chunksInFlight).toBe(3); + }); + + test('a chunk not yet acknowledged is queued and dispatched for real', () => { + const ledger = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); + acknowledgeChunk(ledger, 0); + + const instance = mountInstance(); + const file = { + name: 'video.mp4', + size: 5000, + _resumeLedger: ledger, + upload: { chunks: [{ index: 0 }, { index: 1 }], finishedChunkUpload: jest.fn() } + }; + + instance.dropzone._uploadData([file], [{ chunkIndex: 1 }]); + + expect(mockUploadDataFn).toHaveBeenCalledWith([file], [{ chunkIndex: 1 }]); + expect(file.upload.finishedChunkUpload).not.toHaveBeenCalled(); + }); + + test('a chunk response with status 0 (connection drop) is not acknowledged', () => { + const ledger = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); + const instance = mountInstance(); + const mockXhr = { + readyState: XMLHttpRequest.DONE, + status: 0, + responseText: '', + setRequestHeader: jest.fn(), + onload: jest.fn(), + onerror: jest.fn(), + abort: jest.fn() + }; + const file = { + name: 'video.mp4', + size: 5000, + _resumeLedger: ledger, + upload: { chunked: true, chunks: [{ index: 0, xhr: mockXhr }] } + }; + + getEventHandler(instance, 'sending')(file, mockXhr, { append: jest.fn() }); + mockXhr.onload({}); + + expect(isChunkAcknowledged(ledger, 0)).toBe(false); + }); + + test.each([200, 202])('a chunk response with status %i is acknowledged', (status) => { + const ledger = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); + const instance = mountInstance(); + const mockXhr = { + readyState: XMLHttpRequest.DONE, + status, + responseText: JSON.stringify({ done: 40, status: true }), + setRequestHeader: jest.fn(), + onload: jest.fn(), + onerror: jest.fn(), + abort: jest.fn() + }; + const file = { + name: 'video.mp4', + size: 5000, + _resumeLedger: ledger, + upload: { chunked: true, chunks: [{ index: 2, xhr: mockXhr }] } + }; + + getEventHandler(instance, 'sending')(file, mockXhr, { append: jest.fn() }); + mockXhr.onload({}); + + expect(isChunkAcknowledged(ledger, 2)).toBe(true); + }); + + test('an over-claiming ledger self-corrects once under the same id, then gives up cleanly with no error', () => { + const instance = mountInstance(); + const ledger = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); + const originalId = ledger.uploadId; + const file = { + name: 'video.mp4', + size: 5000, + md5: 'mock-md5-hash', + _resumeLedger: ledger, + _resumeSkippedThisAttempt: true, + _asyncProcessing: false, + upload: { uuid: originalId } + }; + const done = jest.fn(); + + // First occurrence: same id, ackedChunks cleared, one clean re-drive. + mockCapturedOptions.chunksUploaded(file, done); + + expect(instance.dropzone.uploadFiles).toHaveBeenCalledWith([file]); + expect(file.upload.uuid).toBe(originalId); + expect(file._resumeLedger.correctionAttempted).toBe(true); + expect(file._resumeLedger.ackedChunks).toEqual([]); + expect(file._completedBytes).toBe(0); + expect(done).not.toHaveBeenCalled(); + expect(instance.dropzone.emit).not.toHaveBeenCalledWith('error', expect.anything(), expect.anything()); + + // Second occurrence: the corrected pass ALSO came up short - discard the id, + // start a clean full upload under a fresh one, still no error surfaced. + file._resumeSkippedThisAttempt = true; + instance.dropzone.uploadFiles.mockClear(); + + mockCapturedOptions.chunksUploaded(file, done); + + expect(instance.dropzone.uploadFiles).toHaveBeenCalledWith([file]); + expect(file.upload.uuid).not.toBe(originalId); + expect(file._resumeLedger.ackedChunks).toEqual([]); + expect(done).not.toHaveBeenCalled(); + expect(instance.dropzone.emit).not.toHaveBeenCalledWith('error', expect.anything(), expect.anything()); + }); + + test('a genuinely completed resume (async 202) never enters the correction path', () => { + const instance = mountInstance(); + const ledger = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); + const file = { + name: 'video.mp4', + size: 5000, + _resumeLedger: ledger, + _resumeSkippedThisAttempt: true, + _asyncProcessing: true, // the last real chunk got a 202 + upload: { uuid: ledger.uploadId } + }; + const done = jest.fn(); + + mockCapturedOptions.chunksUploaded(file, done); + + expect(instance.dropzone.uploadFiles).not.toHaveBeenCalled(); + expect(file._chunksUploadedDone).toBe(done); + expect(done).not.toHaveBeenCalled(); + }); + + test('a chunkSize change starts a fresh upload id instead of reusing the old one', async () => { + const instance = mountInstance(); + const file = { name: 'video.mp4', size: 5000, upload: {} }; + + await mockCapturedOptions.accept(file, jest.fn()); + const firstId = file.upload.uuid; + + instance.dropzone.options.chunkSize = 2000; + const file2 = { name: 'video.mp4', size: 5000, upload: {} }; + await mockCapturedOptions.accept(file2, jest.fn()); + + expect(file2.upload.uuid).not.toBe(firstId); + }); + + test('a retried File keeps its md5 across accept() calls, so the same ledger is found', async () => { + const instance = mountInstance(); + const file = { name: 'video.mp4', size: 5000, upload: {} }; + + await mockCapturedOptions.accept(file, jest.fn()); + const firstId = file.upload.uuid; + + // Simulate removeFile + addFile reusing the same object: dropzone's native addFile + // resets file.upload, but never touches file.md5. + file.upload = { uuid: 'brand-new-native-random-uuid' }; + await mockCapturedOptions.accept(file, jest.fn()); + + expect(file.upload.uuid).toBe(firstId); + }); +}); diff --git a/src/components/inputs/dropzone/__tests__/upload-ledger.test.js b/src/components/inputs/dropzone/__tests__/upload-ledger.test.js new file mode 100644 index 00000000..c7943e7e --- /dev/null +++ b/src/components/inputs/dropzone/__tests__/upload-ledger.test.js @@ -0,0 +1,129 @@ +/** + * Copyright 2018 OpenStack Foundation + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + **/ + +import { + getOrCreateUploadLedger, + acknowledgeChunk, + isChunkAcknowledged, + markCorrectionAttempted, + clearLedger, +} from '../upload-ledger'; + +describe('upload-ledger', () => { + beforeEach(() => { + window.localStorage.clear(); + }); + + test('creates a fresh random id with an empty ledger on first use', () => { + const ledger = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + + expect(typeof ledger.uploadId).toBe('string'); + expect(ledger.uploadId).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/); + expect(ledger.ackedChunks).toEqual([]); + expect(ledger.correctionAttempted).toBe(false); + }); + + test('a second call for the same file returns the same id and preserves acked chunks', () => { + const first = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + acknowledgeChunk(first, 0); + acknowledgeChunk(first, 1); + + const second = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + + expect(second.uploadId).toBe(first.uploadId); + expect(second.ackedChunks).toEqual([0, 1]); + }); + + test('acknowledging the same index twice does not duplicate it', () => { + const ledger = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + acknowledgeChunk(ledger, 3); + acknowledgeChunk(ledger, 3); + + expect(ledger.ackedChunks).toEqual([3]); + expect(isChunkAcknowledged(ledger, 3)).toBe(true); + expect(isChunkAcknowledged(ledger, 4)).toBe(false); + }); + + test('isChunkAcknowledged is false for a null ledger', () => { + expect(isChunkAcknowledged(null, 0)).toBe(false); + }); + + test('a ledger older than the TTL is discarded for a fresh one', () => { + const nowSpy = jest.spyOn(Date, 'now').mockReturnValue(1000); + const first = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10, 5000); + acknowledgeChunk(first, 0); + + nowSpy.mockReturnValue(1000 + 5001); + const second = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10, 5000); + + expect(second.uploadId).not.toBe(first.uploadId); + expect(second.ackedChunks).toEqual([]); + + nowSpy.mockRestore(); + }); + + test('a chunkSize change discards the ledger for a fresh one', () => { + const first = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + acknowledgeChunk(first, 0); + + const second = getOrCreateUploadLedger('ns', 'md5-a', 1000, 200, 5); + + expect(second.uploadId).not.toBe(first.uploadId); + expect(second.ackedChunks).toEqual([]); + }); + + test('a totalChunks change discards the ledger for a fresh one', () => { + const first = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + acknowledgeChunk(first, 0); + + const second = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 11); + + expect(second.uploadId).not.toBe(first.uploadId); + }); + + test('different namespaces for the same file do not collide', () => { + const a = getOrCreateUploadLedger('slot-a', 'md5-a', 1000, 100, 10); + const b = getOrCreateUploadLedger('slot-b', 'md5-a', 1000, 100, 10); + + expect(a.uploadId).not.toBe(b.uploadId); + }); + + test('markCorrectionAttempted clears ackedChunks and keeps the same id', () => { + const ledger = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + acknowledgeChunk(ledger, 0); + acknowledgeChunk(ledger, 1); + const uploadId = ledger.uploadId; + + const corrected = markCorrectionAttempted(ledger); + + expect(corrected.uploadId).toBe(uploadId); + expect(corrected.ackedChunks).toEqual([]); + expect(corrected.correctionAttempted).toBe(true); + + // and it's actually persisted, not just mutated in memory + const reread = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + expect(reread.correctionAttempted).toBe(true); + expect(reread.ackedChunks).toEqual([]); + }); + + test('clearLedger removes the entry so the next call creates a brand new one', () => { + const first = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + acknowledgeChunk(first, 0); + + clearLedger('ns', 'md5-a', 1000); + + const second = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + expect(second.uploadId).not.toBe(first.uploadId); + expect(second.ackedChunks).toEqual([]); + }); +}); diff --git a/src/components/inputs/dropzone/index.js b/src/components/inputs/dropzone/index.js index c4e745ff..5fcc35c2 100644 --- a/src/components/inputs/dropzone/index.js +++ b/src/components/inputs/dropzone/index.js @@ -6,6 +6,14 @@ import PropTypes from 'prop-types'; import {getAccessToken, initLogOut} from '../../security/methods'; import {AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR} from '../../security/constants'; import {getMD5} from "../../../utils/crypto"; +import { + getOrCreateUploadLedger, + acknowledgeChunk, + isChunkAcknowledged, + markCorrectionAttempted, + clearLedger, + UPLOAD_LEDGER_TTL_MS, +} from "./upload-ledger"; let Dropzone = null; @@ -90,6 +98,14 @@ export class DropzoneJS extends React.Component { const isThrottledChunk = dataBlocks.length === 1 && dataBlocks[0].chunkIndex !== undefined; files.forEach(file => { file._isThrottledChunk = isThrottledChunk; }); if (isThrottledChunk) { + const [file] = files; + const chunkIndex = dataBlocks[0].chunkIndex; + // A resumed chunk the server already acked is resolved here, before it + // ever takes a concurrency slot - never queued, never sent over the wire. + if (isChunkAcknowledged(file._resumeLedger, chunkIndex)) { + this.skipAcknowledgedChunk(file, chunkIndex); + return; + } this.chunkQueue.push({ files, dataBlocks }); this.processChunkQueue(); } else { @@ -99,11 +115,28 @@ export class DropzoneJS extends React.Component { }; } + // Synthesizes a successful chunk without a network round-trip, using Dropzone's own + // finishedChunkUpload to drive its native chunk state machine (next chunk / chunksUploaded) + // exactly as a real success would. + skipAcknowledgedChunk(file, chunkIndex) { + const chunk = file.upload?.chunks?.[chunkIndex]; + if (!chunk) return; + file._resumeSkippedThisAttempt = true; + const chunkSize = this.dropzone?.options?.chunkSize || 2000000; + file._completedBytes = Math.min((file._completedBytes || 0) + chunkSize, file.size); + file.upload.finishedChunkUpload(chunk); + } + onChunkComplete() { this.chunksInFlight = Math.max(0, this.chunksInFlight - 1); this.processChunkQueue(); } + clearResumeLedger(file) { + clearLedger(this.props.id, file.md5, file.size); + file._resumeLedger = null; + } + async pollUploadStatus(fileId, baseUrl, file) { // Guard against multiple polling loops for the same file if (file._pollingActive) { @@ -164,6 +197,7 @@ export class DropzoneJS extends React.Component { } if (data.status === 'complete') { this.stopPolling(file); + if (file._resumeLedger) this.clearResumeLedger(file); // Call the stored done callback to trigger Dropzone's success event if (file?._chunksUploadedDone) { file._chunksUploadedDone(); @@ -235,11 +269,67 @@ export class DropzoneJS extends React.Component { return; } + // A retried File (removeFile + addFile) keeps whatever these were set to on its + // previous failed pass - addFile/removeFile never clear custom properties. + file._asyncProcessing = false; + file._chunksUploadedDone = null; + file._resumeSkippedThisAttempt = false; + + if (options.chunking) { + const chunkSize = options.chunkSize || 2000000; + const totalChunks = Math.ceil(file.size / chunkSize) || 1; + const ledger = getOrCreateUploadLedger( + this.props.id, file.md5, file.size, chunkSize, totalChunks, + this.props.resumeLedgerTtlMs || UPLOAD_LEDGER_TTL_MS + ); + // Overwrites Dropzone's own randomly-generated dzuuid (already set by + // addFile() before accept() ever runs) with our persisted, stable one - + // the only thing that lets a retry reuse the server's in-progress upload. + file.upload.uuid = ledger.uploadId; + file._resumeLedger = ledger; + + if (ledger.ackedChunks.length > 0) { + const acknowledgedBytes = Math.min(ledger.ackedChunks.length * chunkSize, file.size); + file._completedBytes = acknowledgedBytes; + // Reuses the existing uploadprogress bridge so a resumed row shows its + // real starting percentage immediately, with no new event/plumbing. + if (typeof this.dropzone.emit === 'function') { + this.dropzone.emit( + 'uploadprogress', file, (acknowledgedBytes / file.size) * 100, acknowledgedBytes + ); + } + } + } + done(); }; // Override chunksUploaded to defer success event for async processing (HTTP 202) options.chunksUploaded = (file, done) => { + // Every local chunk succeeded (real or resume-skipped) but the file never got + // a real 202 - proof a skipped chunk wasn't actually held by the server. The + // ledger's belief was wrong, not just incomplete: self-correct, bounded to one + // retry under the same id, then one clean full upload under a fresh id - never + // loop, and never surface this to the user, since a clean pass can't skip + // anything and so can't re-trigger this branch. + if (file._resumeLedger && file._resumeSkippedThisAttempt && !file._asyncProcessing) { + file._resumeSkippedThisAttempt = false; + file._completedBytes = 0; + if (!file._resumeLedger.correctionAttempted) { + file._resumeLedger = markCorrectionAttempted(file._resumeLedger); + } else { + const chunkSize = file._resumeLedger.chunkSize; + const totalChunks = file._resumeLedger.totalChunks; + this.clearResumeLedger(file); + file._resumeLedger = getOrCreateUploadLedger( + this.props.id, file.md5, file.size, chunkSize, totalChunks, + this.props.resumeLedgerTtlMs || UPLOAD_LEDGER_TTL_MS + ); + file.upload.uuid = file._resumeLedger.uploadId; + } + this.dropzone.uploadFiles([file]); + return; + } if (file._asyncProcessing) { // Store the done callback for later execution after polling completes file._chunksUploadedDone = done; @@ -488,6 +578,13 @@ export class DropzoneJS extends React.Component { // load callback from dropzone let dropzoneOnLoad = xhr.onload; xhr.onload = function (e) { + // Resolved BEFORE dropzoneOnLoad: on a successful chunked response, Dropzone's + // native finishedChunkUpload (called from within dropzoneOnLoad) nulls + // chunk.xhr, so _getChunk can no longer find it afterwards. + const chunk = (file?.upload?.chunked && _this.dropzone?._getChunk) + ? _this.dropzone._getChunk(file, xhr) + : null; + // Remove this XHR from active tracking const xhrs = _this.activeXHRs.get(file); if (xhrs) { @@ -515,6 +612,13 @@ export class DropzoneJS extends React.Component { file._asyncProcessing = true; } + // Acknowledge only on a real 200/202 - never on a connection failure (status 0, + // xhr.onerror/ontimeout below), so an unacknowledged chunk always gets re-sent + // on the next resume, which is safe because the server dedups a held chunk. + if (chunk && (xhr?.status === 200 || xhr?.status === 202) && file._resumeLedger) { + acknowledgeChunk(file._resumeLedger, chunk.index); + } + dropzoneOnLoad(e); // The user may have cancelled while this response was in flight: abort() on an @@ -524,6 +628,7 @@ export class DropzoneJS extends React.Component { if(xhr?.status == 200) { if (typeof uploadResponse.name === 'string') { + if (file._resumeLedger) _this.clearResumeLedger(file); _this.onUploadComplete(uploadResponse); } } diff --git a/src/components/inputs/dropzone/upload-ledger.js b/src/components/inputs/dropzone/upload-ledger.js new file mode 100644 index 00000000..55e57982 --- /dev/null +++ b/src/components/inputs/dropzone/upload-ledger.js @@ -0,0 +1,111 @@ +/** + * Copyright 2018 OpenStack Foundation + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + **/ + +import { putOnLocalStorage, getFromLocalStorage, removeFromLocalStorage } from '../../../utils/methods'; + +const LEDGER_KEY_PREFIX = 'dz_resume_ledger_v1'; + +// Must stay shorter than file-upload-api's ABANDONED_UPLOAD_RETENTION_SECONDS +// (1hr default) so the client never resumes against chunks the server's reaper +// already deleted. +export const UPLOAD_LEDGER_TTL_MS = 30 * 60 * 1000; + +// Mirrors Dropzone.uuidv4() byte-for-byte without importing the 'dropzone' package +// here - a plain uuid4 is what file-upload-api's sanitize_upload_id (Django slugify) +// passes through unchanged. +const uuidv4 = () => + 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, (c) => { + const r = (Math.random() * 16) | 0; + const v = c === 'x' ? r : (r & 0x3) | 0x8; + return v.toString(16); + }); + +const buildKey = (namespace, fileMd5, fileSize) => + `${LEDGER_KEY_PREFIX}:${namespace || 'default'}:${fileMd5}:${fileSize}`; + +const readLedger = (key) => { + const raw = getFromLocalStorage(key); + if (!raw) return null; + try { + const parsed = JSON.parse(raw); + return parsed && typeof parsed === 'object' ? parsed : null; + } catch (e) { + return null; + } +}; + +const writeLedger = (ledger) => { + const { _key, ...persisted } = ledger; + putOnLocalStorage(_key, JSON.stringify(persisted)); +}; + +/** + * Loads the persisted ledger for a physical file (identified by content, not name, + * so a retry resumes even if the file is re-selected under a different name) within + * a caller-supplied namespace, or creates a fresh one. A stored ledger is discarded + * (fresh random id, empty ackedChunks) when it is older than UPLOAD_LEDGER_TTL_MS, or + * when chunkSize/totalChunks no longer match - reusing an id under a different chunk + * size is what triggers the server's `dztotalchunkcount changed` 412. + */ +export const getOrCreateUploadLedger = (namespace, fileMd5, fileSize, chunkSize, totalChunks, ttlMs = UPLOAD_LEDGER_TTL_MS) => { + const key = buildKey(namespace, fileMd5, fileSize); + const existing = readLedger(key); + const now = Date.now(); + + const isFresh = + !!existing && + now - existing.createdAt < ttlMs && + existing.chunkSize === chunkSize && + existing.totalChunks === totalChunks; + + if (isFresh) return { ...existing, _key: key }; + + const ledger = { + uploadId: uuidv4(), + chunkSize, + totalChunks, + ackedChunks: [], + createdAt: now, + correctionAttempted: false, + _key: key, + }; + writeLedger(ledger); + return ledger; +}; + +export const acknowledgeChunk = (ledger, chunkIndex) => { + if (!ledger || ledger.ackedChunks.includes(chunkIndex)) return ledger; + ledger.ackedChunks.push(chunkIndex); + writeLedger(ledger); + return ledger; +}; + +export const isChunkAcknowledged = (ledger, chunkIndex) => + !!ledger && ledger.ackedChunks.includes(chunkIndex); + +/** + * One bounded self-correction pass: the client believed every acked index was held + * by the server, but it wasn't. Clears that belief without discarding the upload id, + * so the next pass re-sends exactly the previously-skipped chunks under the same id. + */ +export const markCorrectionAttempted = (ledger) => { + if (!ledger) return ledger; + ledger.correctionAttempted = true; + ledger.ackedChunks = []; + writeLedger(ledger); + return ledger; +}; + +export const clearLedger = (namespace, fileMd5, fileSize) => { + removeFromLocalStorage(buildKey(namespace, fileMd5, fileSize)); +}; diff --git a/src/components/inputs/upload-input-v3/__tests__/upload-input-v3.test.js b/src/components/inputs/upload-input-v3/__tests__/upload-input-v3.test.js index 05973137..11939a76 100644 --- a/src/components/inputs/upload-input-v3/__tests__/upload-input-v3.test.js +++ b/src/components/inputs/upload-input-v3/__tests__/upload-input-v3.test.js @@ -298,8 +298,10 @@ describe('UploadInputV3', () => { expect(screen.getByText(/File is too big/)).toBeInTheDocument(); expect(container.querySelector('.dropzone-mock')).not.toBeVisible(); + // The error row has two buttons (Retry, then dismiss) - dismiss is last. + const errorRowButtons = screen.getAllByRole('button'); act(() => { - fireEvent.click(screen.getByRole('button')); + fireEvent.click(errorRowButtons[errorRowButtons.length - 1]); }); expect(screen.queryByText(/File is too big/)).not.toBeInTheDocument(); @@ -363,6 +365,54 @@ describe('UploadInputV3', () => { }); expect(container.querySelector('.dropzone-mock')).not.toBeVisible(); }); + + test('retrying an error resumes: removeFile + addFile on the same dropzone file object, flags reset', () => { + const removeFile = jest.fn(); + const addFile = jest.fn(); + // still carrying flags from its previous pass, same as after a real interrupted upload + const dzFile = { name: 'video.mp4', size: 9999999, accepted: true, _canceled: true }; + + render(); + act(() => { + dropzoneCallbacks.onDropzoneReady({ files: [dzFile], removeFile, addFile }); + }); + act(() => { + dropzoneCallbacks.onFileError({ name: 'video.mp4', size: 9999999 }, 'Network error'); + }); + + act(() => { + fireEvent.click(screen.getByTitle('Retry')); + }); + + expect(removeFile).toHaveBeenCalledWith(dzFile); + expect(addFile).toHaveBeenCalledWith(dzFile); + // dropzone's own accept() gate and our xhr.onload guard both key off these - + // stale values from the failed attempt must not survive into the retry. + expect(dzFile.accepted).toBe(false); + expect(dzFile._canceled).toBe(false); + expect(screen.queryByText('video.mp4')).not.toBeInTheDocument(); + }); + + test('retrying an error is a no-op when the underlying dropzone file is gone', () => { + const removeFile = jest.fn(); + const addFile = jest.fn(); + + render(); + act(() => { + dropzoneCallbacks.onDropzoneReady({ files: [], removeFile, addFile }); + }); + act(() => { + dropzoneCallbacks.onFileError({ name: 'video.mp4', size: 9999999 }, 'Network error'); + }); + + act(() => { + fireEvent.click(screen.getByTitle('Retry')); + }); + + expect(removeFile).not.toHaveBeenCalled(); + expect(addFile).not.toHaveBeenCalled(); + expect(screen.queryByText('video.mp4')).not.toBeInTheDocument(); + }); }); describe('Configuration', () => { @@ -590,7 +640,9 @@ describe('UploadInputV3', () => { }); expect(URL.revokeObjectURL).toHaveBeenCalledWith('blob:photo-a.jpg'); - act(() => { fireEvent.click(screen.getByRole('button')); }); + // The error row now has two buttons (Retry, then dismiss) - the dismiss one is last. + const errorRowButtons = screen.getAllByRole('button'); + act(() => { fireEvent.click(errorRowButtons[errorRowButtons.length - 1]); }); act(() => { dropzoneCallbacks.onAddedFile({ name: 'photo-b.jpg', size: 20000, type: 'image/jpeg' }); dropzoneCallbacks.onFileCompleted({ name: 'photo-b.jpg', size: 20000 }); diff --git a/src/components/inputs/upload-input-v3/index.js b/src/components/inputs/upload-input-v3/index.js index fbbedfe4..647825ad 100644 --- a/src/components/inputs/upload-input-v3/index.js +++ b/src/components/inputs/upload-input-v3/index.js @@ -25,6 +25,7 @@ import DeleteIcon from "@mui/icons-material/Delete"; import CheckCircleIcon from "@mui/icons-material/CheckCircle"; import ErrorOutlineIcon from "@mui/icons-material/ErrorOutline"; import CloseIcon from "@mui/icons-material/Close"; +import ReplayIcon from "@mui/icons-material/Replay"; import { DropzoneV3 } from './dropzone-v3'; import ProgressiveImg from '../../progressive-img'; import file_icon from '../upload-input/file.png'; @@ -269,6 +270,27 @@ const UploadInputV3 = ({ setErrorFiles(prev => prev.filter(f => !(f.name === file.name && f.size === file.size))); }, []); + // An errored file is never auto-removed from dropzone's own file list, so the same + // (name,size) lookup handleDismissError uses still finds it here - re-adding it + // re-triggers accept(), which is where the resume ledger lookup happens. + const handleRetryError = useCallback((file) => { + const dz = dropzoneInstanceRef.current; + const dzFile = dz?.files?.find(f => f.name === file.name && f.size === file.size); + setErrorFiles(prev => prev.filter(f => !(f.name === file.name && f.size === file.size))); + if (!dz || !dzFile) return; // file object gone (e.g. full remount) - re-selecting the + // same file still resumes via the md5-keyed ledger the next time it hits accept() + dz.removeFile(dzFile); + // Reusing the same File object means it still carries flags from its previous pass. + // dropzone's OWN accept() gate runs before our custom options.accept ever does, and + // rejects the file as "too many files" if accepted stays true from its original + // successful accept. Our removedfile handler also sets _canceled, which later blocks + // xhr.onload from ever calling onUploadComplete/pollUploadStatus once resent chunks + // succeed. Neither is cleared by removeFile/addFile - must reset both here. + dzFile.accepted = false; + dzFile._canceled = false; + dz.addFile(dzFile); + }, []); + const handleDeleteUploading = useCallback((file) => { const entry = uploadingFilesRef.current.find(f => f.name === file.name && f.size === file.size); if (entry?.previewUrl) URL.revokeObjectURL(entry.previewUrl); @@ -472,6 +494,14 @@ const UploadInputV3 = ({ + handleRetryError(file)} + title={T.translate("upload_input_v3.retry")} + sx={{ color: 'primary.main' }} + > + + handleDismissError(file)} diff --git a/src/i18n/en.json b/src/i18n/en.json index fcc151ba..50ea6794 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -199,7 +199,8 @@ "upload_failed": "Upload failed. Please retry.", "auth_error": "Authentication error. Please sign in again.", "max_files_reached": "Maximum number of files reached.", - "loading": "Uploading" + "loading": "Uploading", + "retry": "Retry" }, "grid_filter": { "filter_by": "Filter by ", From 03292308c349f51939a5edf7049dc97e5f2e1e3a Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Thu, 3 Sep 2026 19:18:29 -0300 Subject: [PATCH 10/15] v5.0.58-beta.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 1e862b46..b4538fe4 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "openstack-uicore-foundation", - "version": "5.0.58-beta.0", + "version": "5.0.58-beta.1", "description": "ui reactjs components for openstack marketing site", "main": "lib/openstack-uicore-foundation.js", "scripts": { From e2d4960667c089a1ee35106583431b62fa7eb94c Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Fri, 4 Sep 2026 09:45:22 -0300 Subject: [PATCH 11/15] fix: never let a localStorage failure stall a resumable upload getOrCreateUploadLedger was called outside options.accept's try/catch, so a localStorage error (blocked/full storage) would leave the file stuck without ever surfacing an error. readLedger/writeLedger/clearLedger now swallow storage errors and fall back to an in-memory ledger. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GSiT8RW5vTG8gzP9zEnJxt --- .../dropzone/__tests__/upload-ledger.test.js | 43 +++++++++++++++++++ .../inputs/dropzone/upload-ledger.js | 20 ++++++--- 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/src/components/inputs/dropzone/__tests__/upload-ledger.test.js b/src/components/inputs/dropzone/__tests__/upload-ledger.test.js index c7943e7e..e189b43e 100644 --- a/src/components/inputs/dropzone/__tests__/upload-ledger.test.js +++ b/src/components/inputs/dropzone/__tests__/upload-ledger.test.js @@ -126,4 +126,47 @@ describe('upload-ledger', () => { expect(second.uploadId).not.toBe(first.uploadId); expect(second.ackedChunks).toEqual([]); }); + + describe('when localStorage throws (blocked/full storage)', () => { + let getItemSpy; + let setItemSpy; + let removeItemSpy; + + beforeEach(() => { + getItemSpy = jest.spyOn(window.localStorage.__proto__, 'getItem').mockImplementation(() => { + throw new Error('storage blocked'); + }); + setItemSpy = jest.spyOn(window.localStorage.__proto__, 'setItem').mockImplementation(() => { + throw new Error('storage blocked'); + }); + removeItemSpy = jest.spyOn(window.localStorage.__proto__, 'removeItem').mockImplementation(() => { + throw new Error('storage blocked'); + }); + }); + + afterEach(() => { + getItemSpy.mockRestore(); + setItemSpy.mockRestore(); + removeItemSpy.mockRestore(); + }); + + test('getOrCreateUploadLedger still returns a fresh in-memory ledger instead of throwing', () => { + expect(() => getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10)).not.toThrow(); + + const ledger = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + expect(typeof ledger.uploadId).toBe('string'); + expect(ledger.ackedChunks).toEqual([]); + }); + + test('acknowledgeChunk does not throw when the underlying write fails', () => { + const ledger = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + + expect(() => acknowledgeChunk(ledger, 0)).not.toThrow(); + expect(ledger.ackedChunks).toEqual([0]); + }); + + test('clearLedger does not throw when the underlying removal fails', () => { + expect(() => clearLedger('ns', 'md5-a', 1000)).not.toThrow(); + }); + }); }); diff --git a/src/components/inputs/dropzone/upload-ledger.js b/src/components/inputs/dropzone/upload-ledger.js index 55e57982..7a252306 100644 --- a/src/components/inputs/dropzone/upload-ledger.js +++ b/src/components/inputs/dropzone/upload-ledger.js @@ -34,9 +34,9 @@ const buildKey = (namespace, fileMd5, fileSize) => `${LEDGER_KEY_PREFIX}:${namespace || 'default'}:${fileMd5}:${fileSize}`; const readLedger = (key) => { - const raw = getFromLocalStorage(key); - if (!raw) return null; try { + const raw = getFromLocalStorage(key); + if (!raw) return null; const parsed = JSON.parse(raw); return parsed && typeof parsed === 'object' ? parsed : null; } catch (e) { @@ -44,9 +44,15 @@ const readLedger = (key) => { } }; +// Best-effort: a write failure (storage blocked/full) must not break the upload itself - +// the caller already has the ledger object in memory and keeps using it for this attempt. const writeLedger = (ledger) => { - const { _key, ...persisted } = ledger; - putOnLocalStorage(_key, JSON.stringify(persisted)); + try { + const { _key, ...persisted } = ledger; + putOnLocalStorage(_key, JSON.stringify(persisted)); + } catch (e) { + // no-op - persistence is opportunistic + } }; /** @@ -107,5 +113,9 @@ export const markCorrectionAttempted = (ledger) => { }; export const clearLedger = (namespace, fileMd5, fileSize) => { - removeFromLocalStorage(buildKey(namespace, fileMd5, fileSize)); + try { + removeFromLocalStorage(buildKey(namespace, fileMd5, fileSize)); + } catch (e) { + // no-op - a failed clear is harmless, the ledger just expires via its TTL + } }; From b249d23b8ff2deb0471a1cdadbe03c4347b7490f Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Thu, 17 Sep 2026 15:10:30 -0300 Subject: [PATCH 12/15] chore: pr review --- .../dropzone/__tests__/dropzone-resume.test.js | 9 +++++++-- src/components/inputs/dropzone/index.js | 5 ++--- src/components/inputs/upload-input-v3/index.js | 15 ++++++++------- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/components/inputs/dropzone/__tests__/dropzone-resume.test.js b/src/components/inputs/dropzone/__tests__/dropzone-resume.test.js index 9909dd87..6f1b530e 100644 --- a/src/components/inputs/dropzone/__tests__/dropzone-resume.test.js +++ b/src/components/inputs/dropzone/__tests__/dropzone-resume.test.js @@ -121,7 +121,7 @@ describe('DropzoneJS - Resumable Chunked Uploads', () => { expect(instance.dropzone.emit).not.toHaveBeenCalledWith('uploadprogress', expect.anything(), expect.anything(), expect.anything()); }); - test('a chunk already acknowledged is skipped: never dispatched, never occupies a slot', () => { + test('a chunk already acknowledged is skipped: never dispatched, never occupies a slot', async () => { const ledger = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); acknowledgeChunk(ledger, 0); @@ -139,8 +139,13 @@ describe('DropzoneJS - Resumable Chunked Uploads', () => { instance.dropzone._uploadData([file], [{ chunkIndex: 0 }]); expect(mockUploadDataFn).not.toHaveBeenCalled(); + // finishedChunkUpload is deferred to a microtask to avoid recursing the stack + // through Dropzone's own handleNextChunk on a long run of skipped chunks. + await Promise.resolve(); expect(file.upload.finishedChunkUpload).toHaveBeenCalledWith(chunk0); - expect(file._completedBytes).toBe(1000); + // accept() is what restores _completedBytes in bulk from the ledger; skipping + // a chunk here doesn't touch it, so it stays whatever it started as. + expect(file._completedBytes).toBeUndefined(); // never took a concurrency slot, so there is none to release expect(instance.chunksInFlight).toBe(3); }); diff --git a/src/components/inputs/dropzone/index.js b/src/components/inputs/dropzone/index.js index 5fcc35c2..cc4a6560 100644 --- a/src/components/inputs/dropzone/index.js +++ b/src/components/inputs/dropzone/index.js @@ -122,9 +122,8 @@ export class DropzoneJS extends React.Component { const chunk = file.upload?.chunks?.[chunkIndex]; if (!chunk) return; file._resumeSkippedThisAttempt = true; - const chunkSize = this.dropzone?.options?.chunkSize || 2000000; - file._completedBytes = Math.min((file._completedBytes || 0) + chunkSize, file.size); - file.upload.finishedChunkUpload(chunk); + // Deferred: finishedChunkUpload can recurse back into skipAcknowledgedChunk synchronously. + Promise.resolve().then(() => file.upload.finishedChunkUpload(chunk)); } onChunkComplete() { diff --git a/src/components/inputs/upload-input-v3/index.js b/src/components/inputs/upload-input-v3/index.js index 647825ad..6b9bf3e5 100644 --- a/src/components/inputs/upload-input-v3/index.js +++ b/src/components/inputs/upload-input-v3/index.js @@ -253,8 +253,9 @@ const UploadInputV3 = ({ const displayMessage = resolveErrorMessage(text, status); setErrorFiles(prev => { - const existingIndex = prev.findIndex(f => f.name === file.name && f.size === file.size); - const entry = { name: file.name, size: file.size, message: displayMessage }; + const uuid = file.upload?.uuid; + const existingIndex = prev.findIndex(f => f.uuid === uuid); + const entry = { uuid, name: file.name, size: file.size, message: displayMessage }; if (existingIndex === -1) return [...prev, entry]; return prev.map((f, i) => (i === existingIndex ? entry : f)); }); @@ -263,20 +264,20 @@ const UploadInputV3 = ({ const handleDismissError = useCallback((file) => { if (dropzoneInstanceRef.current) { const dzFile = dropzoneInstanceRef.current.files?.find( - f => f.name === file.name && f.size === file.size + f => f.upload?.uuid === file.uuid ); if (dzFile) dropzoneInstanceRef.current.removeFile(dzFile); } - setErrorFiles(prev => prev.filter(f => !(f.name === file.name && f.size === file.size))); + setErrorFiles(prev => prev.filter(f => f.uuid !== file.uuid)); }, []); // An errored file is never auto-removed from dropzone's own file list, so the same - // (name,size) lookup handleDismissError uses still finds it here - re-adding it + // uuid lookup handleDismissError uses still finds it here - re-adding it // re-triggers accept(), which is where the resume ledger lookup happens. const handleRetryError = useCallback((file) => { const dz = dropzoneInstanceRef.current; - const dzFile = dz?.files?.find(f => f.name === file.name && f.size === file.size); - setErrorFiles(prev => prev.filter(f => !(f.name === file.name && f.size === file.size))); + const dzFile = dz?.files?.find(f => f.upload?.uuid === file.uuid); + setErrorFiles(prev => prev.filter(f => f.uuid !== file.uuid)); if (!dz || !dzFile) return; // file object gone (e.g. full remount) - re-selecting the // same file still resumes via the md5-keyed ledger the next time it hits accept() dz.removeFile(dzFile); From 52ee0c44aeb121d6f4836803ffd746beb944361f Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Mon, 21 Sep 2026 16:05:10 -0300 Subject: [PATCH 13/15] chore: fix conflict --- src/components/inputs/dropzone/index.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/components/inputs/dropzone/index.js b/src/components/inputs/dropzone/index.js index cc4a6560..a61bfc00 100644 --- a/src/components/inputs/dropzone/index.js +++ b/src/components/inputs/dropzone/index.js @@ -205,11 +205,7 @@ export class DropzoneJS extends React.Component { return; } else if (data.status === 'error') { this.stopPolling(file); -<<<<<<< HEAD this.reportPollingError(file, data.message || data.error || 'Upload failed'); -======= - this.reportPollingError(file, data.message || 'Upload failed'); ->>>>>>> 68fd188 (chore: pr review) return; } // any other status (e.g. 'uploading') means keep polling From 86270560e853d7e5cd3600fd819abd29a9b0c6fa Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Mon, 21 Sep 2026 16:23:45 -0300 Subject: [PATCH 14/15] chore: pr review --- .../__tests__/dropzone-resume.test.js | 67 ++++++++++++++++--- src/components/inputs/dropzone/index.js | 45 +++++++++++-- 2 files changed, 94 insertions(+), 18 deletions(-) diff --git a/src/components/inputs/dropzone/__tests__/dropzone-resume.test.js b/src/components/inputs/dropzone/__tests__/dropzone-resume.test.js index 6f1b530e..2454bdf9 100644 --- a/src/components/inputs/dropzone/__tests__/dropzone-resume.test.js +++ b/src/components/inputs/dropzone/__tests__/dropzone-resume.test.js @@ -218,46 +218,91 @@ describe('DropzoneJS - Resumable Chunked Uploads', () => { expect(isChunkAcknowledged(ledger, 2)).toBe(true); }); - test('an over-claiming ledger self-corrects once under the same id, then gives up cleanly with no error', () => { - const instance = mountInstance(); + test('an over-claiming ledger self-corrects once under the same id, then starts a clean upload under a fresh one', async () => { const ledger = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); + acknowledgeChunk(ledger, 0); const originalId = ledger.uploadId; + + const instance = mountInstance(); + const chunk0 = { index: 0 }; const file = { name: 'video.mp4', size: 5000, md5: 'mock-md5-hash', _resumeLedger: ledger, - _resumeSkippedThisAttempt: true, - _asyncProcessing: false, - upload: { uuid: originalId } + upload: { uuid: originalId, chunks: [chunk0], finishedChunkUpload: jest.fn() } }; const done = jest.fn(); - // First occurrence: same id, ackedChunks cleared, one clean re-drive. + // Drive the skip for real: the ledger claims chunk 0, so _uploadData resolves it + // without a request and sets _resumeSkippedThisAttempt itself. + instance.dropzone._uploadData([file], [{ chunkIndex: 0 }]); + await Promise.resolve(); + expect(file._resumeSkippedThisAttempt).toBe(true); + + // Pass 1 ended without a 202: one correction under the same id. mockCapturedOptions.chunksUploaded(file, done); expect(instance.dropzone.uploadFiles).toHaveBeenCalledWith([file]); expect(file.upload.uuid).toBe(originalId); expect(file._resumeLedger.correctionAttempted).toBe(true); expect(file._resumeLedger.ackedChunks).toEqual([]); + expect(file._resumeCorrectionPass).toBe(true); expect(file._completedBytes).toBe(0); expect(done).not.toHaveBeenCalled(); - expect(instance.dropzone.emit).not.toHaveBeenCalledWith('error', expect.anything(), expect.anything()); - // Second occurrence: the corrected pass ALSO came up short - discard the id, - // start a clean full upload under a fresh one, still no error surfaced. - file._resumeSkippedThisAttempt = true; + // Pass 2 re-sent every chunk, so it skipped nothing - no flag is set by hand here: + // _resumeCorrectionPass is what carries the pass, and its own lack of a 202 is the signal. instance.dropzone.uploadFiles.mockClear(); - mockCapturedOptions.chunksUploaded(file, done); expect(instance.dropzone.uploadFiles).toHaveBeenCalledWith([file]); expect(file.upload.uuid).not.toBe(originalId); expect(file._resumeLedger.ackedChunks).toEqual([]); + expect(file._resumeCorrectionPass).toBe(false); expect(done).not.toHaveBeenCalled(); expect(instance.dropzone.emit).not.toHaveBeenCalledWith('error', expect.anything(), expect.anything()); }); + test('a 202 clears the ledger, so a retry after a failed poll uploads cleanly under a new id', () => { + const ledger = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); + acknowledgeChunk(ledger, 0); + const originalId = ledger.uploadId; + + const instance = mountInstance(); + // The 202 starts polling; this test is only about the ledger, and a live poll loop + // would outlive it. + instance.pollUploadStatus = jest.fn(); + const mockXhr = { + readyState: XMLHttpRequest.DONE, + status: 202, + responseText: JSON.stringify({ file_id: 'server-file-id', status: 'uploading' }), + setRequestHeader: jest.fn(), + onload: jest.fn(), + onerror: jest.fn(), + abort: jest.fn() + }; + const file = { + name: 'video.mp4', + size: 5000, + md5: 'mock-md5-hash', + _resumeLedger: ledger, + upload: { uuid: originalId, chunked: true, chunks: [{ index: 4, xhr: mockXhr }] } + }; + + getEventHandler(instance, 'sending')(file, mockXhr, { append: jest.fn() }); + mockXhr.onload({}); + + expect(file._asyncProcessing).toBe(true); + expect(file._resumeLedger).toBeNull(); + + // Whatever happens to the poll from here, the next attempt for this same physical + // file cannot resume against an upload the server has already assembled. + const next = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); + expect(next.uploadId).not.toBe(originalId); + expect(next.ackedChunks).toEqual([]); + }); + test('a genuinely completed resume (async 202) never enters the correction path', () => { const instance = mountInstance(); const ledger = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); diff --git a/src/components/inputs/dropzone/index.js b/src/components/inputs/dropzone/index.js index a61bfc00..c4365193 100644 --- a/src/components/inputs/dropzone/index.js +++ b/src/components/inputs/dropzone/index.js @@ -82,8 +82,14 @@ export class DropzoneJS extends React.Component { const maxConcurrent = this.props.maxConcurrentChunks || 6; while (this.chunkQueue.length > 0 && this.chunksInFlight < maxConcurrent) { const { files, dataBlocks } = this.chunkQueue.shift(); - // Only one file per chunk array; skip chunks left queued for an already-errored file. - if (files[0].status === DROPZONE_STATUS_ERROR) continue; + // Only one file per chunk array; skip chunks left queued for a file that has + // already errored or been cancelled - the same pair checked at xhr.onload. + const [queuedFile] = files; + if ( + queuedFile.status === DROPZONE_STATUS_ERROR || + queuedFile._canceled || + queuedFile.status === DROPZONE_STATUS_CANCELED + ) continue; this.chunksInFlight++; this._originalUploadData(files, dataBlocks); } @@ -182,7 +188,8 @@ export class DropzoneJS extends React.Component { this.stopPolling(file); // Don't report an error for a file the user already removed, or after unmount. if (!file._canceled && !this._unmounted) { - this.reportPollingError(file, response.status === 403 ? 'Auth error' : 'Network error'); + const isAuthFailure = response.status === 401 || response.status === 403; + this.reportPollingError(file, isAuthFailure ? 'Auth error' : 'Network error'); } return; } @@ -269,6 +276,8 @@ export class DropzoneJS extends React.Component { file._asyncProcessing = false; file._chunksUploadedDone = null; file._resumeSkippedThisAttempt = false; + file._resumeCorrectionPass = false; + file._errorReported = false; if (options.chunking) { const chunkSize = options.chunkSize || 2000000; @@ -305,13 +314,21 @@ export class DropzoneJS extends React.Component { // a real 202 - proof a skipped chunk wasn't actually held by the server. The // ledger's belief was wrong, not just incomplete: self-correct, bounded to one // retry under the same id, then one clean full upload under a fresh id - never - // loop, and never surface this to the user, since a clean pass can't skip - // anything and so can't re-trigger this branch. - if (file._resumeLedger && file._resumeSkippedThisAttempt && !file._asyncProcessing) { + // loop, and never surface this to the user: the clean pass skips nothing and is + // no longer flagged as a correction, so neither test below can fire again. + const skippedWithoutAsync = + !!file._resumeLedger && file._resumeSkippedThisAttempt && !file._asyncProcessing; + // A correction pass re-sends every chunk, so it skips nothing and can never match + // the test above - its own failure to reach a 202 is what marks it as come up short. + const correctionCameUpShort = + !!file._resumeLedger && file._resumeCorrectionPass && !file._asyncProcessing; + + if (skippedWithoutAsync || correctionCameUpShort) { file._resumeSkippedThisAttempt = false; file._completedBytes = 0; - if (!file._resumeLedger.correctionAttempted) { + if (!correctionCameUpShort && !file._resumeLedger.correctionAttempted) { file._resumeLedger = markCorrectionAttempted(file._resumeLedger); + file._resumeCorrectionPass = true; } else { const chunkSize = file._resumeLedger.chunkSize; const totalChunks = file._resumeLedger.totalChunks; @@ -321,6 +338,7 @@ export class DropzoneJS extends React.Component { this.props.resumeLedgerTtlMs || UPLOAD_LEDGER_TTL_MS ); file.upload.uuid = file._resumeLedger.uploadId; + file._resumeCorrectionPass = false; } this.dropzone.uploadFiles([file]); return; @@ -605,6 +623,9 @@ export class DropzoneJS extends React.Component { // Set async flag BEFORE dropzoneOnLoad so chunksUploaded sees it if (xhr?.status == 202 && uploadResponse.file_id) { file._asyncProcessing = true; + // Assembled server-side: the chunk counter and temp dir are already gone, + // so every ack is stale from here on and there is nothing left to resume. + if (file._resumeLedger) _this.clearResumeLedger(file); } // Acknowledge only on a real 200/202 - never on a connection failure (status 0, @@ -652,6 +673,16 @@ export class DropzoneJS extends React.Component { } if (dropzoneOnTimeout) dropzoneOnTimeout(e); } + + // abort() fires onabort, not onerror/ontimeout, so cancelling a file mid-upload + // would otherwise strand a slot per in-flight chunk and stall every later upload. + let dropzoneOnAbort = xhr.onabort; + xhr.onabort = function(e) { + if (file._isThrottledChunk) { + _this.onChunkComplete(); + } + if (dropzoneOnAbort) dropzoneOnAbort(e); + } }) // xhr.status is 0 for a transport failure, vs a real non-2xx server response. From 3f617df590e3d8a78b9a99580c55c110cfee84fb Mon Sep 17 00:00:00 2001 From: smarcet Date: Wed, 23 Sep 2026 11:02:49 -0300 Subject: [PATCH 15/15] fix: reset the progress floor when a retried upload starts over A retry reuses the same File object, and _completedBytes kept the value from the previous pass. After a 202 the ledger is cleared while _completedBytes already equals file.size, so a Retry that follows a failed status poll (network, auth, timeout, processing error) showed 100% for the whole re-upload. accept() now resets it with the other per-attempt flags; the ledger restore still seeds it from the acknowledged chunks when there is something to resume. --- .../__tests__/dropzone-resume.test.js | 32 ++++++++++++++++++- src/components/inputs/dropzone/index.js | 3 ++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/components/inputs/dropzone/__tests__/dropzone-resume.test.js b/src/components/inputs/dropzone/__tests__/dropzone-resume.test.js index 2454bdf9..817243a2 100644 --- a/src/components/inputs/dropzone/__tests__/dropzone-resume.test.js +++ b/src/components/inputs/dropzone/__tests__/dropzone-resume.test.js @@ -117,10 +117,40 @@ describe('DropzoneJS - Resumable Chunked Uploads', () => { expect(file.upload.uuid).not.toBe('dropzone-own-random-uuid'); expect(file._resumeLedger.ackedChunks).toEqual([]); - expect(file._completedBytes).toBeUndefined(); + expect(file._completedBytes).toBe(0); expect(instance.dropzone.emit).not.toHaveBeenCalledWith('uploadprogress', expect.anything(), expect.anything(), expect.anything()); }); + test('a retry after the async phase failed starts from 0, not from the previous pass', async () => { + const instance = mountInstance(); + instance.pollUploadStatus = jest.fn(); + const file = { name: 'video.mp4', size: 5000, upload: { uuid: 'dz-1', chunked: true, chunks: [] } }; + + await mockCapturedOptions.accept(file, jest.fn()); + for (let i = 0; i < 5; i++) { + const last = i === 4; + const xhr = { + readyState: XMLHttpRequest.DONE, + status: last ? 202 : 200, + responseText: JSON.stringify(last ? { file_id: 'fid' } : { done: 20 * (i + 1), status: true }), + setRequestHeader: jest.fn(), onload: jest.fn(), onerror: jest.fn(), abort: jest.fn() + }; + file.upload.chunks[i] = { index: i, xhr }; + getEventHandler(instance, 'sending')(file, xhr, { append: jest.fn() }); + xhr.onload({}); + } + // Assembled: nothing left to resume, but every byte was counted on the way. + expect(file._resumeLedger).toBeNull(); + expect(file._completedBytes).toBe(5000); + + // Polling then fails -> Retry: removeFile + addFile hands accept() a new upload object. + file.upload = { uuid: 'dz-2', chunked: true, chunks: [] }; + await mockCapturedOptions.accept(file, jest.fn()); + + expect(file._resumeLedger.ackedChunks).toEqual([]); + expect(file._completedBytes).toBe(0); + }); + test('a chunk already acknowledged is skipped: never dispatched, never occupies a slot', async () => { const ledger = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); acknowledgeChunk(ledger, 0); diff --git a/src/components/inputs/dropzone/index.js b/src/components/inputs/dropzone/index.js index c4365193..12950a97 100644 --- a/src/components/inputs/dropzone/index.js +++ b/src/components/inputs/dropzone/index.js @@ -278,6 +278,9 @@ export class DropzoneJS extends React.Component { file._resumeSkippedThisAttempt = false; file._resumeCorrectionPass = false; file._errorReported = false; + // Progress floor from the previous pass: the ledger restore below re-seeds it + // from the acked chunks, and an attempt with nothing to resume starts from 0. + file._completedBytes = 0; if (options.chunking) { const chunkSize = options.chunkSize || 2000000;