TT-7583 fix: only warn of a network issue when the upload never completed

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);

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<boolean>,
     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<void>;
     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,16 +240,68 @@ 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.'
     );
     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<boolean>;
+
+      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<boolean>;
+
+      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);

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<void> => {
-    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));

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<typeof Axios>;
+
+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();
+  });
+});