diff --git a/package.json b/package.json index 4754205e..1e862b46 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "openstack-uicore-foundation", - "version": "5.0.54", + "version": "5.0.58-beta.0", "description": "ui reactjs components for openstack marketing site", "main": "lib/openstack-uicore-foundation.js", "scripts": { diff --git a/src/components/inputs/dropzone/__tests__/dropzone.test.js b/src/components/inputs/dropzone/__tests__/dropzone.test.js index 9a1c2497..36d18c6b 100644 --- a/src/components/inputs/dropzone/__tests__/dropzone.test.js +++ b/src/components/inputs/dropzone/__tests__/dropzone.test.js @@ -31,13 +31,22 @@ let mockCapturedOptions = {}; jest.mock('dropzone', () => { return jest.fn().mockImplementation((element, options) => { mockCapturedOptions = options; - return { + const dz = { options, on: jest.fn(), off: jest.fn(), destroy: jest.fn(() => null), getActiveFiles: jest.fn(() => []) }; + // Mimics Dropzone's real Emitter: replays every handler registered via `on` + // for that event, in registration order - the same "multiple listeners on + // one event" behavior the ontimeout/pollUploadStatus fixes rely on. + dz.emit = jest.fn((event, ...args) => { + dz.on.mock.calls + .filter(([evt]) => evt === event) + .forEach(([, handler]) => handler(...args)); + }); + return dz; }); }); @@ -515,6 +524,142 @@ describe('DropzoneJS - HTTP 202 Polling UX', () => { done(); }, 10); }); + + /** + * Test Case 10: a chunk that times out releases its concurrency slot + * + * Dropzone's own default xhr.ontimeout (set before the 'sending' handler runs) + * still fires 'error', but without wrapping it here onChunkComplete() never + * runs, so chunksInFlight never decrements and later chunks stay queued forever. + */ + test('test_dropzone_ontimeout_releases_chunk_slot', () => { + const ref = React.createRef(); + + render( + + ); + + const instance = ref.current; + const mockFile = { name: 'test.pdf', size: 1024000 }; + const dropzoneOnTimeout = jest.fn(); + const mockXhr = { + readyState: XMLHttpRequest.DONE, + setRequestHeader: jest.fn(), + onload: jest.fn(), + onerror: jest.fn(), + ontimeout: dropzoneOnTimeout, + abort: jest.fn() + }; + + instance.chunksInFlight = 1; + getEventHandler(instance, 'sending')(mockFile, mockXhr, { append: jest.fn() }); + + mockXhr.ontimeout({}); + + expect(instance.chunksInFlight).toBe(0); + // Dropzone's own timeout handling (which still reports the error) must still run. + expect(dropzoneOnTimeout).toHaveBeenCalledTimes(1); + }); + + /** + * Test Cases 11-13: pollUploadStatus's three failure branches route through the + * file-level error channel (dropzone.emit('error', file, message)) instead of + * calling onError directly, so the row clears and the consumer is told exactly once. + */ + test('test_dropzone_poll_timeout_emits_a_string_message_and_calls_onError_once', async () => { + jest.useFakeTimers({ doNotFake: ['queueMicrotask'] }); + global.fetch = jest.fn(() => + Promise.resolve({ json: () => Promise.resolve({ status: 'uploading' }) }) + ); + + const ref = React.createRef(); + render( + + ); + + const instance = ref.current; + const mockFile = { name: 'big.pdf', size: 1024000 }; + instance.pollUploadStatus('file-timeout', 'https://example.com/upload', mockFile); + + // maxAttempts is 300 at 2s/tick - advance one tick past the ceiling. + await jest.advanceTimersByTimeAsync(2000 * 301); + + expect(onErrorMock).toHaveBeenCalledTimes(1); + const [message] = onErrorMock.mock.calls[0]; + expect(typeof message).toBe('string'); + expect(message).toBe('Upload timed out'); + + jest.useRealTimers(); + }, 20000); + + test('test_dropzone_poll_server_error_status_emits_readable_message_not_object', (done) => { + global.fetch = jest.fn(() => + Promise.resolve({ + json: () => Promise.resolve({ status: 'error', message: 'processing failed' }) + }) + ); + + const ref = React.createRef(); + render( + + ); + + setTimeout(() => { + const instance = ref.current; + const mockFile = { name: 'test.pdf', size: 1024000 }; + instance.pollUploadStatus('file-server-error', 'https://example.com/upload', mockFile); + + setTimeout(() => { + expect(onErrorMock).toHaveBeenCalledTimes(1); + const [message] = onErrorMock.mock.calls[0]; + expect(message).toBe('processing failed'); + expect(message).not.toBe('[object Object]'); + done(); + }, 2500); + }, 10); + }, 10000); + + test('test_dropzone_poll_fetch_rejection_emits_readable_message_and_calls_onError_once', (done) => { + global.fetch = jest.fn(() => Promise.reject(new Error('network down'))); + + const ref = React.createRef(); + render( + + ); + + setTimeout(() => { + const instance = ref.current; + const mockFile = { name: 'test.pdf', size: 1024000 }; + instance.pollUploadStatus('file-network-error', 'https://example.com/upload', mockFile); + + setTimeout(() => { + expect(onErrorMock).toHaveBeenCalledTimes(1); + const [message] = onErrorMock.mock.calls[0]; + expect(message).toBe('Network error'); + done(); + }, 2500); + }, 10); + }, 10000); }); describe('DropzoneJS - Progress Bar Monotonicity', () => { diff --git a/src/components/inputs/dropzone/index.js b/src/components/inputs/dropzone/index.js index d619fc1e..846c1ab3 100644 --- a/src/components/inputs/dropzone/index.js +++ b/src/components/inputs/dropzone/index.js @@ -106,7 +106,7 @@ export class DropzoneJS extends React.Component { attempts++; if (attempts > maxAttempts) { this.stopPolling(file); - this.onError({ message: 'Upload timed out' }); + this.dropzone.emit('error', file, 'Upload timed out'); return; } try { @@ -131,11 +131,12 @@ export class DropzoneJS extends React.Component { this.onUploadComplete(data); } else if (data.status === 'error') { this.stopPolling(file); - this.onError(data); + this.dropzone.emit('error', file, data.message || data.error || 'Upload failed'); } } catch (error) { this.stopPolling(file); - this.onError(error); + // fetch fail is always connection error + this.dropzone.emit('error', file, 'Network error'); } }, 2000); @@ -494,11 +495,19 @@ export class DropzoneJS extends React.Component { _this.onChunkComplete(); if (dropzoneOnError) dropzoneOnError(e); } + + // Without this wrapper a timed-out chunk never releases its concurrency slot. + let dropzoneOnTimeout = xhr.ontimeout; + xhr.ontimeout = function(e) { + _this.onChunkComplete(); + if (dropzoneOnTimeout) dropzoneOnTimeout(e); + } }) - this.dropzone.on('error', (file, message) => { + // xhr.status is 0 for a transport failure, vs a real non-2xx server response. + this.dropzone.on('error', (file, message, xhr) => { console.log(`DropzoneJS::error`, message); - this.onError(message); + this.onError(message, xhr?.status); }); } diff --git a/src/components/inputs/upload-input-v3/__tests__/dropzone-v3.test.js b/src/components/inputs/upload-input-v3/__tests__/dropzone-v3.test.js index 3bdae599..a9042f8b 100644 --- a/src/components/inputs/upload-input-v3/__tests__/dropzone-v3.test.js +++ b/src/components/inputs/upload-input-v3/__tests__/dropzone-v3.test.js @@ -90,3 +90,43 @@ describe('DropzoneV3 - uploadprogress to React bridging', () => { expect(onUploadProgress).toHaveBeenCalledWith(file, 40); }); }); + +describe('DropzoneV3 - error status forwarding', () => { + beforeEach(() => { + capturedEventHandlers = null; + }); + + test('forwards the xhr status from the error event to onFileError', () => { + const onFileError = jest.fn(); + render( + + ); + + const file = { name: 'video.mp4' }; + capturedEventHandlers.error(file, 'Server responded with 0 code.', { status: 0 }); + + expect(onFileError).toHaveBeenCalledWith(file, 'Server responded with 0 code.', 0); + }); + + test('passes undefined status when Dropzone emits error without an xhr (e.g. client-side validation)', () => { + const onFileError = jest.fn(); + render( + + ); + + const file = { name: 'huge.mp4' }; + capturedEventHandlers.error(file, 'File is too big.'); + + expect(onFileError).toHaveBeenCalledWith(file, 'File is too big.', undefined); + }); +}); 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 59f3b058..3a81bf68 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 @@ -246,6 +246,47 @@ describe('UploadInputV3', () => { expect(screen.getByText(/File is too big/)).toBeInTheDocument(); }); + test('two failed chunks for the same file collapse into a single error row', () => { + render(); + act(() => { + dropzoneCallbacks.onAddedFile({ name: 'big-file.png', size: 9999999 }); + }); + act(() => { + dropzoneCallbacks.onFileError({ name: 'big-file.png', size: 9999999 }, 'Server responded with 0 code.', 0); + }); + act(() => { + dropzoneCallbacks.onFileError({ name: 'big-file.png', size: 9999999 }, 'Server responded with 0 code.', 0); + }); + expect(screen.getAllByText('big-file.png')).toHaveLength(1); + expect(screen.queryByText('Server responded with 0 code.')).not.toBeInTheDocument(); + expect(screen.getAllByText('Upload interrupted by a connection error. Please retry.')).toHaveLength(1); + }); + + test('a status-0 connection failure shows a readable message instead of the raw "Server responded with 0 code."', () => { + render(); + act(() => { + dropzoneCallbacks.onFileError({ name: 'video.mp4', size: 9999999 }, 'Server responded with 0 code.', 0); + }); + expect(screen.getByText('Upload interrupted by a connection error. Please retry.')).toBeInTheDocument(); + expect(screen.queryByText('Server responded with 0 code.')).not.toBeInTheDocument(); + }); + + test('a fetch-rejection during status polling ("Network error") shows the same readable connection message', () => { + render(); + act(() => { + dropzoneCallbacks.onFileError({ name: 'video.mp4', size: 9999999 }, 'Network error'); + }); + expect(screen.getByText('Upload interrupted by a connection error. Please retry.')).toBeInTheDocument(); + }); + + test('a real non-zero-status server error still shows the server-provided message', () => { + render(); + act(() => { + dropzoneCallbacks.onFileError({ name: 'video.mp4', size: 9999999 }, 'File type not allowed', 415); + }); + expect(screen.getByText('File type not allowed')).toBeInTheDocument(); + }); + test('dismissing an error removes it from the view and restores the dropzone', () => { const { container } = render(); act(() => { diff --git a/src/components/inputs/upload-input-v3/dropzone-v3.js b/src/components/inputs/upload-input-v3/dropzone-v3.js index c38495b7..f351d770 100644 --- a/src/components/inputs/upload-input-v3/dropzone-v3.js +++ b/src/components/inputs/upload-input-v3/dropzone-v3.js @@ -63,9 +63,9 @@ export const DropzoneV3 = ({ if (onFileCompleted) onFileCompleted(file); if (eventHandlers.success) eventHandlers.success(file); }, - error: (file, message) => { - if (onFileError) onFileError(file, message); - if (eventHandlers.error) eventHandlers.error(file, message); + error: (file, message, xhr) => { + if (onFileError) onFileError(file, message, xhr?.status); + if (eventHandlers.error) eventHandlers.error(file, message, xhr?.status); }, }; diff --git a/src/components/inputs/upload-input-v3/index.js b/src/components/inputs/upload-input-v3/index.js index 8e2d474c..bb9a97d6 100644 --- a/src/components/inputs/upload-input-v3/index.js +++ b/src/components/inputs/upload-input-v3/index.js @@ -205,19 +205,29 @@ const UploadInputV3 = ({ })); }, [value]); - const handleFileError = useCallback((file, message) => { + const handleFileError = useCallback((file, message, status) => { setUploadingFiles(prev => { const entry = prev.find(f => f.name === file.name && f.size === file.size); if (entry?.previewUrl) URL.revokeObjectURL(entry.previewUrl); return prev.filter(f => !(f.name === file.name && f.size === file.size)); }); - // Dropzone turns a cancelled upload into an error carrying dictUploadCanceled. A cancel - // is not a failure to report back to the user - the row just goes away. 'canceled' is the - // value of Dropzone.CANCELED, matched as a literal so this does not depend on the - // Dropzone module being loaded here; _userCanceled also covers files Dropzone never got - // to mark, such as one removed before its upload reached the UPLOADING state. + // 'canceled' is the value of Dropzone.CANCELED, matched as a literal so this does not depend on the + // Dropzone module being loaded here; _userCanceled is when removed before its upload reached the UPLOADING state. if (file._userCanceled || file.status === 'canceled') return; - setErrorFiles(prev => [...prev, { name: file.name, size: file.size, message }]); + + // 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 || message === 'Network error' + ? T.translate('upload_input_v3.network_error') + : message; + + 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 }; + if (existingIndex === -1) return [...prev, entry]; + return prev.map((f, i) => (i === existingIndex ? entry : f)); + }); }, []); const handleDismissError = useCallback((file) => { diff --git a/src/i18n/en.json b/src/i18n/en.json index a8da85c6..4ca2c9ff 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -186,7 +186,8 @@ "drag_and_drop": "or drag and drop", "see_preview": "See Preview", "preview_file": "Preview file", - "complete": "Complete" + "complete": "Complete", + "network_error": "Upload interrupted by a connection error. Please retry." }, "grid_filter": { "filter_by": "Filter by ",