diff --git a/src/renderer/src/components/Uploader.tsx b/src/renderer/src/components/Uploader.tsx index 135738229..aae6fb334 100644 --- a/src/renderer/src/components/Uploader.tsx +++ b/src/renderer/src/components/Uploader.tsx @@ -25,7 +25,8 @@ import JSONAPISource from '@orbit/jsonapi'; import PassageRecordDlg from './PassageRecordDlg'; import { restoreScroll } from '../utils'; import { shallowEqual, useSelector } from 'react-redux'; -import { NextUploadProps } from '../store'; +import { NextUploadProps, UploadFailureInfo } from '../store'; +import { suggestsConnectionProblem } from '../store/upload/uploadRetry'; import { useDispatch } from 'react-redux'; import { mediaTabSelector, sharedSelector } from '../selector'; import { passageDefaultSuffix } from '../utils/passageDefaultFilename'; @@ -244,9 +245,15 @@ export const Uploader = (props: IProps) => { } return num; }; - const itemComplete = async (n: number, success: boolean, data?: any) => { + const itemComplete = async ( + n: number, + success: boolean, + data?: any, + failure?: UploadFailureInfo + ) => { if (success) successCount.current += 1; - else setOrbitRetries(OrbitNetworkErrorRetries - 1); //notify of possible network issue + else if (suggestsConnectionProblem(failure)) + setOrbitRetries(OrbitNetworkErrorRetries - 1); //notify of possible network issue const uploadList = fileList.current; if (!uploadList) return; // This should never happen if (data?.stringId) mediaIdRef.current.push(data?.stringId); diff --git a/src/renderer/src/crud/useMediaUpload.test.ts b/src/renderer/src/crud/useMediaUpload.test.ts index feefe2361..25df14916 100644 --- a/src/renderer/src/crud/useMediaUpload.test.ts +++ b/src/renderer/src/crud/useMediaUpload.test.ts @@ -2,6 +2,7 @@ import React from 'react'; import type { MediaFileD } from '../model'; import { UPLOAD_COMPLETE } from '../store/upload/types'; +import { UploadFailureReason } from '../store/upload/uploadRetry'; import { OrbitNetworkErrorRetries } from '../../api-variable'; jest.mock('react-redux', () => ({ @@ -178,7 +179,7 @@ describe('useMediaUpload', () => { async function completeUpload( upload: (files: File[]) => Promise, files: File[], - ...cbArgs: [number, boolean, unknown?] + ...cbArgs: [number, boolean, unknown?, unknown?] ) { const uploadPromise = upload(files); const { nextUpload } = require('../store'); @@ -187,7 +188,8 @@ describe('useMediaUpload', () => { const cb = uploadProps.cb as ( n: number, success: boolean, - data?: unknown + data?: unknown, + failure?: unknown ) => void | Promise; await cb(...cbArgs); return uploadPromise; @@ -225,7 +227,7 @@ describe('useMediaUpload', () => { expect(afterUploadCb).toHaveBeenCalledWith('media-1'); }); - it('failure path: orbit retries, 0 of 1 snackbar, empty media id, no pullTableList', async () => { + it('failure path with no failure info: no orbit retries, 0 of 1 snackbar, empty media id, no pullTableList', async () => { const afterUploadCb = jest.fn().mockResolvedValue(undefined); const { result } = renderUploadHook({ artifactId: null, @@ -238,9 +240,7 @@ describe('useMediaUpload', () => { completeUpload(upload, [makeFile()], 0, false, undefined) ).rejects.toThrow('Upload Failed!'); - expect(mockSetOrbitRetries).toHaveBeenCalledWith( - OrbitNetworkErrorRetries - 1 - ); + expect(mockSetOrbitRetries).not.toHaveBeenCalled(); expect(pullTableList).not.toHaveBeenCalled(); expect(mockShowMessage).toHaveBeenCalledWith( '0 of 1 files uploaded successfully.' @@ -248,6 +248,60 @@ describe('useMediaUpload', () => { expect(afterUploadCb).toHaveBeenCalledWith(''); }); + it.each([[UploadFailureReason.NoResponse], [UploadFailureReason.Timeout]])( + '%s failure notifies of a possible connection problem', + async (reason) => { + const afterUploadCb = jest.fn().mockResolvedValue(undefined); + const { result } = renderUploadHook({ + artifactId: null, + passageId: 'psg-1', + afterUploadCb, + }); + const upload = result.current as (files: File[]) => Promise; + + await expect( + completeUpload(upload, [makeFile()], 0, false, undefined, { reason }) + ).rejects.toThrow('Upload Failed!'); + + expect(mockSetOrbitRetries).toHaveBeenCalledWith( + OrbitNetworkErrorRetries - 1 + ); + } + ); + + it.each([ + [UploadFailureReason.Rejected, 403], + [UploadFailureReason.ServerError, 500], + [UploadFailureReason.UnsupportedType, undefined], + [UploadFailureReason.TooBig, undefined], + [UploadFailureReason.LocalWriteFailed, undefined], + ])( + '%s failure does not notify of a connection problem', + async (reason, statusNum) => { + const afterUploadCb = jest.fn().mockResolvedValue(undefined); + const { result } = renderUploadHook({ + artifactId: null, + passageId: 'psg-1', + afterUploadCb, + }); + const upload = result.current as (files: File[]) => Promise; + + await expect( + completeUpload(upload, [makeFile()], 0, false, undefined, { + reason, + statusNum, + }) + ).rejects.toThrow('Upload Failed!'); + + expect(mockSetOrbitRetries).not.toHaveBeenCalled(); + // the rest of the failure cleanup still runs + expect(mockShowMessage).toHaveBeenCalledWith( + '0 of 1 files uploaded successfully.' + ); + expect(afterUploadCb).toHaveBeenCalledWith(''); + } + ); + it('offline success: createMedia, snackbar, afterUploadCb with created id, no pullTableList', async () => { mockOffline = true; const afterUploadCb = jest.fn().mockResolvedValue(undefined); diff --git a/src/renderer/src/crud/useMediaUpload.ts b/src/renderer/src/crud/useMediaUpload.ts index 5db0fd219..ee8aaf273 100644 --- a/src/renderer/src/crud/useMediaUpload.ts +++ b/src/renderer/src/crud/useMediaUpload.ts @@ -20,6 +20,7 @@ import { AlertSeverity, useSnackBar } from '../hoc/SnackBar'; import { mediaTabSelector, sharedSelector } from '../selector'; import { OrbitNetworkErrorRetries } from '../../api-variable'; import { formatUploadTerminalFailureMessage } from '../store/upload/uploadTerminalMessages'; +import { suggestsConnectionProblem } from '../store/upload/uploadRetry'; interface IProps { artifactId: string | null; @@ -103,9 +104,11 @@ export const useMediaUpload = ({ const itemComplete = async ( n: number, success: boolean, - data?: any + data?: any, + failure?: actions.UploadFailureInfo ): Promise => { - if (!success) setOrbitRetries(OrbitNetworkErrorRetries - 1); //notify of possible network issue + if (!success && suggestsConnectionProblem(failure)) + setOrbitRetries(OrbitNetworkErrorRetries - 1); //notify of possible network issue const uploadList = fileList.current; if (!uploadList) return; // This should never happen if (data?.stringId) { @@ -227,8 +230,8 @@ export const useMediaUpload = ({ offline: getGlobal('offline'), errorReporter: reporter, uploadType: UploadType.Media, - cb: (n, success, data) => { - void itemComplete(n, success, data) + cb: (n, success, data, failure) => { + void itemComplete(n, success, data, failure) .then(() => { if (success) resolve(true); else reject(new Error(t.uploadFailed)); diff --git a/src/renderer/src/store/upload/actions.failureReason.test.ts b/src/renderer/src/store/upload/actions.failureReason.test.ts new file mode 100644 index 000000000..474ad9934 --- /dev/null +++ b/src/renderer/src/store/upload/actions.failureReason.test.ts @@ -0,0 +1,177 @@ +/* eslint-disable @typescript-eslint/no-require-imports */ +import Axios from 'axios'; +import { UploadType } from '../../components/UploadType'; +import { type MediaFileAttributes } from '../../model'; +import { UploadFailureReason } from './uploadRetry'; + +jest.mock('../../../api-variable', () => ({ + API_CONFIG: { host: 'https://api.test', sizeLimit: '500' }, +})); +jest.mock('../../utils/typeLimit', () => ({ + typeLimit: () => 500, +})); +jest.mock('axios'); +jest.mock('../../auth/bugsnagClient', () => ({})); + +jest.mock('./pendingMediaUploads', () => ({ + appendPendingMediaUpload: jest.fn(() => ({ + id: 'pending-1', + localAbsolutePath: '/staged/test.mp3', + })), + updatePendingMediaUpload: jest.fn(() => ({ + id: 'pending-1', + localAbsolutePath: '/staged/test.mp3', + })), + removePendingMediaUpload: jest.fn(), +})); + +jest.mock('../../utils', () => ({ + dataPath: jest.fn(), + infoMsg: jest.fn((e: Error) => e.message), + logError: jest.fn(), + PathType: { MEDIA: 'media' }, + Severity: { error: 'error' }, + createPathFolder: jest.fn(), + removeExtension: jest.fn((name: string) => ({ name, ext: 'mp3' })), +})); + +jest.mock('./uploadRetry', () => { + const actual = jest.requireActual('./uploadRetry'); + return { + ...actual, + waitForImportExportIdle: jest.fn(async () => undefined), + sleepMs: jest.fn(() => Promise.resolve()), + }; +}); + +const { nextUpload } = require('./actions') as typeof import('./actions'); + +const mockedAxios = Axios as jest.Mocked; + +const baseRecord = { + planId: '1', + versionNumber: 1, + originalFile: 'test.mp3', + contentType: 'audio/mpeg', + artifactTypeId: '', + passageId: '', + userId: '1', + recordedbyUserId: '1', + sourceMediaId: '', + sourceSegments: '{}', + performedBy: null, + topic: '', + eafUrl: '', + transcription: '', +} as unknown as MediaFileAttributes; + +const makeFile = (name = 'test.mp3') => + new File([new Uint8Array([1, 2, 3])], name, { type: 'audio/mpeg' }); + +const flushPromises = async (times = 40) => { + for (let i = 0; i < times; i += 1) await Promise.resolve(); +}; + +/** Drive the PUT through XMLHttpRequest with the outcome a real S3 attempt would give. */ +function stubPutResponse(outcome: 'networkError' | 'timeout' | number) { + const xhrProto = XMLHttpRequest.prototype; + jest.spyOn(xhrProto, 'open').mockImplementation(() => undefined); + jest.spyOn(xhrProto, 'setRequestHeader').mockImplementation(() => undefined); + jest.spyOn(xhrProto, 'send').mockImplementation(function ( + this: XMLHttpRequest + ) { + if (outcome === 'networkError') { + // A dropped connection: the browser reports status 0, not an HTTP status. + Object.defineProperty(this, 'status', { value: 0, configurable: true }); + this.onerror?.(new ProgressEvent('error')); + return; + } + if (outcome === 'timeout') { + this.ontimeout?.(new ProgressEvent('timeout')); + return; + } + Object.defineProperty(this, 'status', { + value: outcome, + configurable: true, + }); + this.onload?.(new ProgressEvent('load')); + }); +} + +async function runUpload(file = makeFile(), record = baseRecord) { + const cb = jest.fn(); + const action = nextUpload({ + record, + files: [file], + n: 0, + token: 'token', + offline: false, + errorReporter: {} as never, + uploadType: UploadType.Media, + cb, + }); + action(jest.fn()); + await flushPromises(); + return cb; +} + +const reasonOf = (cb: jest.Mock) => cb.mock.calls.at(-1)?.[3]?.reason; + +describe('nextUpload failure reason reported to cb', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockedAxios.post.mockResolvedValue({ + data: { data: { id: 7, attributes: { 'audio-url': 'https://s3/put' } } }, + } as never); + mockedAxios.delete.mockResolvedValue({} as never); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('reports NoResponse when the PUT never reaches the server', async () => { + stubPutResponse('networkError'); + expect(reasonOf(await runUpload())).toBe(UploadFailureReason.NoResponse); + }); + + it('reports Timeout when the PUT times out', async () => { + stubPutResponse('timeout'); + expect(reasonOf(await runUpload())).toBe(UploadFailureReason.Timeout); + }); + + it('reports Rejected when the PUT is refused', async () => { + stubPutResponse(403); + expect(reasonOf(await runUpload())).toBe(UploadFailureReason.Rejected); + }); + + it('reports ServerError when the PUT fails server-side', async () => { + stubPutResponse(500); + expect(reasonOf(await runUpload())).toBe(UploadFailureReason.ServerError); + }); + + it('reports NoResponse when the POST gets no response', async () => { + stubPutResponse(200); + mockedAxios.post.mockRejectedValue({ message: 'Network Error' } as never); + expect(reasonOf(await runUpload())).toBe(UploadFailureReason.NoResponse); + }); + + it('reports Rejected when the POST is refused', async () => { + stubPutResponse(200); + mockedAxios.post.mockRejectedValue({ + response: { status: 403 }, + message: 'Forbidden', + } as never); + expect(reasonOf(await runUpload())).toBe(UploadFailureReason.Rejected); + }); + + it('reports UnsupportedType before any request is made', async () => { + stubPutResponse(200); + const cb = await runUpload( + new File([new Uint8Array([1])], 'notes.xyz', { type: 'application/xyz' }), + { ...baseRecord, originalFile: 'notes.xyz' } as MediaFileAttributes + ); + expect(reasonOf(cb)).toBe(UploadFailureReason.UnsupportedType); + expect(mockedAxios.post).not.toHaveBeenCalled(); + }); +}); diff --git a/src/renderer/src/store/upload/actions.tsx b/src/renderer/src/store/upload/actions.tsx index 23453e00d..10ac6d591 100644 --- a/src/renderer/src/store/upload/actions.tsx +++ b/src/renderer/src/store/upload/actions.tsx @@ -35,7 +35,12 @@ import { UPLOAD_MAX_ATTEMPTS, waitForImportExportIdle, isRetryableUploadStatus, + uploadFailureReasonFromStatus, + UploadFailureReason, + type UploadFailureInfo, } from './uploadRetry'; + +export type { UploadFailureInfo, UploadFailureReason } from './uploadRetry'; import { appendPendingMediaUpload, PendingUploadRecord, @@ -44,6 +49,8 @@ import { removePendingMediaUpload, updatePendingMediaUpload, } from './pendingMediaUploads'; +// TEMPORARY (TT-7583 manual test) — remove with fakeUploadFailure.ts +import { fakeUploadFailureStatus } from './fakeUploadFailure'; const ipc = window?.api as MainAPI; @@ -145,6 +152,18 @@ export const uploadFile = ( errorReporter: typeof bugsnagClient ): Promise<{ statusNum: number; statusText: string }> => { return new Promise((resolve, reject) => { + // TEMPORARY (TT-7583 manual test) — remove with fakeUploadFailure.ts + const faked = fakeUploadFailureStatus('put'); + if (faked !== undefined) { + const rej: UploadFileReject = { + statusNum: faked, + statusText: + faked === 0 ? 'FAKE upload failed; network error' : 'FAKE upload', + httpStatus: faked || undefined, + }; + reject(rej); + return; + } let xhr = new XMLHttpRequest(); const cleanup = (): void => { xhr.onload = null; @@ -205,7 +224,9 @@ export const uploadFile = ( ); cleanup(); const rej: UploadFileReject = { - statusNum: httpStatus || 500, + // 0 (not 500) when we never got a response: the caller classifies this as + // UploadFailureReason.NoResponse, and 500 would claim we reached the server. + statusNum: httpStatus || 0, statusText, httpStatus: httpStatus || undefined, }; @@ -251,7 +272,12 @@ export interface NextUploadProps { offline: boolean; errorReporter: typeof bugsnagClient; uploadType: UploadType; - cb?: (n: number, success: boolean, data?: MediaFileAttributes) => void; + cb?: ( + n: number, + success: boolean, + data?: MediaFileAttributes, + failure?: UploadFailureInfo + ) => void; onTerminalFailure?: (info: UploadTerminalFailureInfo) => void; /** When retrying from the pending queue, pass the entry id to remove after a successful upload. */ pendingUploadIdToClearOnSuccess?: string; @@ -278,7 +304,12 @@ export const nextUpload = (dispatch: Dispatch) => { dispatch({ payload: n, type: UPLOAD_ITEM_PENDING }); let pendingIdToClear = pendingUploadIdToClearOnSuccess; - const sendError = (n: number, message: string, mediaid?: number): void => { + const sendError = ( + n: number, + message: string, + reason: UploadFailureReason, + mediaid?: number + ): void => { dispatch({ payload: { current: n, @@ -287,7 +318,8 @@ export const nextUpload = }, type: UPLOAD_ITEM_FAILED, }); - if (cb) cb(n, false); + // These never reached the network, so they carry no status. + if (cb) cb(n, false, undefined, { reason }); }; const { name, size, type } = files[n] as File; const isDownloadable = !isNotDownloadable(type); @@ -298,11 +330,15 @@ export const nextUpload = isDownloadable && !acceptExtPat.test(record.originalFile.split('?')[0] || '') ) { - sendError(n, `${name}:unsupported`); + sendError(n, `${name}:unsupported`, UploadFailureReason.UnsupportedType); return; } if (size > typeLimit(uploadType) * 1000000) { - sendError(n, `${name}:toobig:${(size / 1000000).toFixed(2)}`); + sendError( + n, + `${name}:toobig:${(size / 1000000).toFixed(2)}`, + UploadFailureReason.TooBig + ); return; } if (offline) { @@ -319,7 +355,11 @@ export const nextUpload = errorReporter, infoMsg(err as Error, `failed getting name: ${name}`) ); - sendError(n, `${name} failed local write`); + sendError( + n, + `${name} failed local write`, + UploadFailureReason.LocalWriteFailed + ); } return; } @@ -346,13 +386,17 @@ export const nextUpload = dispatch({ payload: { current: n, - // statusNum is undefined when the request never reached the server, - // so don't render a literal "(undefined)" at the user (TT-7583). - error: `upload ${name}: (${statusNum ?? 'no response'}) ${statusText}`, + // statusNum is undefined (POST) or 0 (PUT) when the request never + // reached the server; don't render that at the user (TT-7583). + error: `upload ${name}: (${statusNum || 'no response'}) ${statusText}`, }, type: UPLOAD_ITEM_FAILED, }); - if (cb) cb(n, false, data); + if (cb) + cb(n, false, data, { + reason: uploadFailureReasonFromStatus(statusNum), + statusNum, + }); } }; @@ -467,7 +511,11 @@ export const nextUpload = errorReporter, infoMsg(err as Error, `local staging failed: ${name}`) ); - sendError(n, `${name}: local save failed`); + sendError( + n, + `${name}: local save failed`, + UploadFailureReason.LocalWriteFailed + ); return; } } @@ -516,10 +564,8 @@ export const nextUpload = record: snapshotForPending(), }; const pendingRecord = pendingIdToClear - ? (updatePendingMediaUpload( - pendingIdToClear, - queuePatch - ) ?? appendPendingMediaUpload(queuePatch)) + ? (updatePendingMediaUpload(pendingIdToClear, queuePatch) ?? + appendPendingMediaUpload(queuePatch)) : appendPendingMediaUpload(queuePatch); onTerminalFailure?.({ localAbsolutePath: pathForQueue || pendingRecord.localAbsolutePath, @@ -534,6 +580,14 @@ export const nextUpload = let json: unknown; for (let attempt = 0; attempt < UPLOAD_MAX_ATTEMPTS; attempt++) { try { + // TEMPORARY (TT-7583 manual test) — remove with fakeUploadFailure.ts + const fakedPost = fakeUploadFailureStatus('post'); + if (fakedPost !== undefined) { + throw { + response: fakedPost ? { status: fakedPost } : undefined, + message: 'FAKE post failure', + }; + } const response = await Axios.post( API_CONFIG.host + '/api/mediafiles', vndRecord, diff --git a/src/renderer/src/store/upload/fakeUploadFailure.ts b/src/renderer/src/store/upload/fakeUploadFailure.ts new file mode 100644 index 000000000..eb34043f0 --- /dev/null +++ b/src/renderer/src/store/upload/fakeUploadFailure.ts @@ -0,0 +1,47 @@ +/** + * TEMPORARY manual-test harness for TT-7583 — DELETE BEFORE MERGING. + * + * Forces upload failures so the "possible network issue" retry banner can be + * checked against each UploadFailureReason without arranging a real fault. + * + * Drive it from the browser console (no reload needed — it is read per attempt): + * + * localStorage.apmFakeUpload = 'put:0' // PUT never reaches S3 -> NoResponse (WARNS) + * localStorage.apmFakeUpload = 'put:408' // PUT times out -> Timeout (WARNS) + * localStorage.apmFakeUpload = 'put:403' // S3 refuses the PUT -> Rejected (silent) + * localStorage.apmFakeUpload = 'put:500' // S3 blows up -> ServerError (silent) + * localStorage.apmFakeUpload = 'post:0' // POST gets no response -> NoResponse (WARNS) + * localStorage.apmFakeUpload = 'post:403' // API refuses the POST -> Rejected (silent) + * localStorage.apmFakeUpload = 'post:500' // API blows up -> ServerError (silent) + * + * delete localStorage.apmFakeUpload // back to normal + * + * "WARNS" = setOrbitRetries fires, so Sources.tsx shows the retry banner. + */ +export type FakeUploadStage = 'put' | 'post'; + +const KEY = 'apmFakeUpload'; + +/** + * The status this stage should pretend to fail with, or undefined to behave + * normally. 0 means "the request never got a response". + */ +export const fakeUploadFailureStatus = ( + stage: FakeUploadStage +): number | undefined => { + try { + const raw = window?.localStorage?.getItem(KEY); + if (!raw) return undefined; + const [wanted, status] = raw.split(':'); + if (wanted !== stage) return undefined; + const num = Number(status ?? 0); + const faked = Number.isFinite(num) ? num : 0; + + console.warn( + `[apmFakeUpload] faking ${stage} failure with status ${faked}` + ); + return faked; + } catch { + return undefined; + } +}; diff --git a/src/renderer/src/store/upload/uploadRetry.test.ts b/src/renderer/src/store/upload/uploadRetry.test.ts index d49b3a99f..75defec19 100644 --- a/src/renderer/src/store/upload/uploadRetry.test.ts +++ b/src/renderer/src/store/upload/uploadRetry.test.ts @@ -1,6 +1,9 @@ import { isRetryableUploadError, runWithUploadRetries, + suggestsConnectionProblem, + uploadFailureReasonFromStatus, + UploadFailureReason, UPLOAD_MAX_ATTEMPTS, } from './uploadRetry'; @@ -49,6 +52,46 @@ describe('isRetryableUploadError', () => { }); }); +describe('uploadFailureReasonFromStatus', () => { + it.each([ + [undefined, UploadFailureReason.NoResponse], + [0, UploadFailureReason.NoResponse], + [408, UploadFailureReason.Timeout], + [403, UploadFailureReason.Rejected], + [429, UploadFailureReason.Rejected], + [500, UploadFailureReason.ServerError], + [503, UploadFailureReason.ServerError], + ])('classifies %s as %s', (status, reason) => { + expect(uploadFailureReasonFromStatus(status)).toBe(reason); + }); +}); + +describe('suggestsConnectionProblem', () => { + it.each([[UploadFailureReason.NoResponse], [UploadFailureReason.Timeout]])( + 'is true for %s: the request never completed', + (reason) => { + expect(suggestsConnectionProblem({ reason })).toBe(true); + } + ); + + it.each([ + [UploadFailureReason.Rejected, 403], + [UploadFailureReason.ServerError, 500], + [UploadFailureReason.UnsupportedType, undefined], + [UploadFailureReason.TooBig, undefined], + [UploadFailureReason.LocalWriteFailed, undefined], + ])( + 'is false for %s: we reached the server, or never left the client', + (reason, statusNum) => { + expect(suggestsConnectionProblem({ reason, statusNum })).toBe(false); + } + ); + + it('treats missing failure info as no evidence', () => { + expect(suggestsConnectionProblem(undefined)).toBe(false); + }); +}); + describe('runWithUploadRetries', () => { beforeEach(() => { jest.useFakeTimers(); @@ -59,13 +102,11 @@ describe('runWithUploadRetries', () => { }); it('does not retry permanent upload failures', async () => { - const runAttempt = jest - .fn() - .mockRejectedValue({ - statusNum: 403, - statusText: 'Forbidden', - httpStatus: 403, - }); + const runAttempt = jest.fn().mockRejectedValue({ + statusNum: 403, + statusText: 'Forbidden', + httpStatus: 403, + }); const onRetry = jest.fn(); await expect( @@ -77,13 +118,11 @@ describe('runWithUploadRetries', () => { }); it('retries transient failures up to UPLOAD_MAX_ATTEMPTS', async () => { - const runAttempt = jest - .fn() - .mockRejectedValue({ - statusNum: 503, - statusText: 'down', - httpStatus: 503, - }); + const runAttempt = jest.fn().mockRejectedValue({ + statusNum: 503, + statusText: 'down', + httpStatus: 503, + }); const onRetry = jest.fn(); const promise = runWithUploadRetries(runAttempt, onRetry); diff --git a/src/renderer/src/store/upload/uploadRetry.ts b/src/renderer/src/store/upload/uploadRetry.ts index 4113c5486..1b47c14e2 100644 --- a/src/renderer/src/store/upload/uploadRetry.ts +++ b/src/renderer/src/store/upload/uploadRetry.ts @@ -61,6 +61,60 @@ export const isRetryableUploadStatus = ( return true; }; +/** + * Why an upload item failed. Named rather than a status number because the local + * rejections never reach the server and so have no status at all — and `undefined` + * already means "never got a response" to {@link isRetryableUploadStatus}. + */ +export enum UploadFailureReason { + /** File type we refuse to upload. */ + UnsupportedType = 'unsupportedType', + /** Over the size limit for this upload type. */ + TooBig = 'tooBig', + /** Offline staging to the local media folder failed. */ + LocalWriteFailed = 'localWriteFailed', + /** Server answered 4xx — it heard us and said no. */ + Rejected = 'rejected', + /** Server answered 5xx — reachable, but failing. */ + ServerError = 'serverError', + /** The request was sent but timed out waiting. */ + Timeout = 'timeout', + /** Never reached the server at all. */ + NoResponse = 'noResponse', +} + +/** Why an upload item failed, so callers can tell a connection problem from a rejected file. */ +export interface UploadFailureInfo { + reason: UploadFailureReason; + /** HTTP status of the last attempt; undefined when the request never reached the server. */ + statusNum?: number; +} + +/** Classify the status of a failed server round-trip. */ +export const uploadFailureReasonFromStatus = ( + status: number | undefined +): UploadFailureReason => { + if (status === undefined || status === 0) + return UploadFailureReason.NoResponse; + // 408 here is our own XHR timeout (see uploadFile), not a server-sent status. + if (status === 408) return UploadFailureReason.Timeout; + if (status >= 500) return UploadFailureReason.ServerError; + if (status >= 400 && status < 500) return UploadFailureReason.Rejected; + return UploadFailureReason.ServerError; +}; + +/** + * Does this failure look like the user's connection, rather than the file or the + * server? Only a request that never completed is evidence of that: a 4xx/5xx means + * we reached the server, and a locally rejected file never involved the network. + * No failure info at all is not evidence either, so stay quiet. + */ +export const suggestsConnectionProblem = ( + failure: UploadFailureInfo | undefined +): boolean => + failure?.reason === UploadFailureReason.NoResponse || + failure?.reason === UploadFailureReason.Timeout; + export async function runWithUploadRetries( runAttempt: (attemptIndexZeroBased: number) => Promise, onRetry?: (error: unknown, attemptIndexZeroBased: number) => void