fix(upload): tell the operator why an asset upload failed

src/anthias_server/app/static/src/home.test.ts

@@ -0,0 +1,161 @@
+// Wiring tests for the upload batch. Run with
+// `bun test src/anthias_server/app/static/src/home.test.ts`.
+//
+// home/upload-error.test pins the status → message table; this drives
+// the real `uploadFiles` through a stubbed XMLHttpRequest so a
+// regression in the plumbing — a status dropped on the way up, a batch
+// that fails to abort — cannot pass just because the table still holds.
+
+import { afterEach, beforeEach, describe, expect, test } from 'bun:test'
+
+import './home'
+
+type Toast = { kind: string; message: string; ttlMs?: number }
+type Outcome = { status: number } | { transport: true }
+
+const realXhr = globalThis.XMLHttpRequest
+
+let sends: number
+
+// Each send() consumes the next outcome, so a batch can be given a
+// different fate per file.
+function stubXhr(outcomes: Outcome[]): void {
+  let index = 0
+  sends = 0
+  ;(globalThis as unknown as { XMLHttpRequest: unknown }).XMLHttpRequest =
+    function XMLHttpRequestStub() {
+      const outcome = outcomes[Math.min(index++, outcomes.length - 1)]
+      const handlers: Record<string, (() => void)[]> = {}
+      return {
+        status: 'status' in outcome ? outcome.status : 0,
+        upload: { addEventListener: () => {} },
+        open: () => {},
+        setRequestHeader: () => {},
+        getResponseHeader: () => null,
+        addEventListener(type: string, fn: () => void) {
+          ;(handlers[type] ??= []).push(fn)
+        },
+        send() {
+          sends += 1
+          const type = 'status' in outcome ? 'load' : 'error'
+          queueMicrotask(() => handlers[type]?.forEach((fn) => fn()))
+        },
+      }
+    }
+}
+
+// `uploadFiles` only reaches for `files`, `form` and `value`, so a
+// literal is steadier here than building a real FileList in happy-dom.
+function fileInput(...names: string[]): HTMLInputElement {
+  return {
+    value: '',
+    files: names.map((n) => new File(['x'], n, { type: 'video/mp4' })),
+    form: {
+      getAttribute: () => '/assets/upload/',
+      querySelector: () => ({ value: 'test-csrf' }),
+    },
+  } as unknown as HTMLInputElement
+}
+
+let toasts: Toast[]
+let refreshes: string[]
+
+beforeEach(() => {
+  toasts = []
+  refreshes = []
+  ;(window as unknown as { Alpine: unknown }).Alpine = {
+    store: () => ({
+      push: (kind: string, message: string, ttlMs?: number) =>
+        toasts.push({ kind, message, ttlMs }),
+    }),
+  }
+  ;(window as unknown as { htmx: unknown }).htmx = {
+    trigger: (_target: string, event: string) => refreshes.push(event),
+  }
+})
+
+// bun runs every test file in one process, so leaving these replaced
+// would hand the stubs to any later file that touches them.
+afterEach(() => {
+  globalThis.XMLHttpRequest = realXhr
+  delete (window as unknown as { Alpine?: unknown }).Alpine
+  delete (window as unknown as { htmx?: unknown }).htmx
+})
+
+describe('uploadFiles error reporting', () => {
+  test('a proxy 413 surfaces the size-limit message', async () => {
+    stubXhr([{ status: 413 }])
+    await window.homeApp().uploadFiles(fileInput('big-video.mp4'))
+
+    expect(toasts[0]?.kind).toBe('error')
+    expect(toasts[0]?.message).toBe(
+      'File too large — it exceeds the upload size limit of the server ' +
+        'or a proxy in front of it',
+    )
+  })
+
+  test('a dead socket names both causes without asserting one', async () => {
+    stubXhr([{ transport: true }])
+    await window.homeApp().uploadFiles(fileInput('big-video.mp4'))
+
+    expect(toasts[0]?.message).toBe(
+      'Upload failed mid-transfer — check your connection, or try a ' +
+        'smaller file',
+    )
+  })
+
+  test('an unremarkable 400 keeps the original wording', async () => {
+    stubXhr([{ status: 400 }])
+    await window.homeApp().uploadFiles(fileInput('bad.mp4'))
+
+    expect(toasts[0]?.message).toBe(
+      'Upload failed — check the file and try again',
+    )
+  })
+
+  // These are the longest strings the store carries, so they outlast
+  // the 4s default a shorter toast gets.
+  test('an upload error stays on screen longer than the default', async () => {
+    stubXhr([{ status: 413 }])
+    await window.homeApp().uploadFiles(fileInput('big-video.mp4'))
+
+    expect(toasts[0]?.ttlMs).toBe(8000)
+  })
+})
+
+describe('uploadFiles batch behaviour', () => {
+  test('a successful upload closes the modal and refreshes the table', async () => {
+    stubXhr([{ status: 200 }])
+    const app = window.homeApp()
+    app.mode = 'add'
+    await app.uploadFiles(fileInput('fine.mp4'))
+
+    expect(toasts).toEqual([])
+    expect(app.mode).toBeNull()
+    expect(refreshes).toEqual(['refresh-assets'])
+  })
+
+  // Whatever went wrong applies to the rest of the selection too, so
+  // the batch stops rather than hammering on — and reports once.
+  test('a transport failure aborts the rest of the batch', async () => {
+    stubXhr([{ transport: true }])
+    await window.homeApp().uploadFiles(fileInput('a.mp4', 'b.mp4', 'c.mp4'))
+
+    expect(toasts).toHaveLength(1)
+    // The real assertion: files b and c were never even attempted.
+    expect(sends).toBe(1)
+  })
+
+  // A batch that fails partway still lands the rows that made it, so
+  // the operator does not re-upload files that are already stored.
+  test('a partial batch commits its successes and still reports', async () => {
+    stubXhr([{ status: 200 }, { transport: true }])
+    const app = window.homeApp()
+    app.mode = 'add'
+    await app.uploadFiles(fileInput('good.mp4', 'doomed.mp4'))
+
+    expect(refreshes).toEqual(['refresh-assets'])
+    expect(app.mode).toBeNull()
+    expect(toasts).toHaveLength(1)
+  })
+})

src/anthias_server/app/static/src/home.ts

@@ -13,6 +13,7 @@
   type AppsTabData,
   type EditAsset as AppEditAsset,
 } from './apps'
+import { uploadErrorMessage, type UploadFailure } from './home/upload-error'
 
 declare global {
   interface Window {
@@ -50,9 +51,18 @@
 type UploadState = null | 'sending' | 'processing'
 
 interface ToastStoreLike {
-  push(kind: 'success' | 'error' | 'info', message: string): number
+  push(
+    kind: 'success' | 'error' | 'info',
+    message: string,
+    ttlMs?: number,
+  ): number
 }
 
+// Longer than the 4s default in vendor.ts: these are the longest
+// strings the store carries and a dismissed toast cannot be brought
+// back.
+const UPLOAD_ERROR_TOAST_MS = 8000
+
 type SectionKey = 'active' | 'inactive'
 
 interface HomeAppData {
@@ -100,8 +110,13 @@
 // 'rejected' — server reached, but it refused this file (HTTP 200 +
 //              error toast, e.g. invalid type). The toast already
 //              informed the user; the batch skips it and carries on.
-// 'error'    — transport failure / non-2xx. Aborts the batch.
-type UploadResult = 'ok' | 'rejected' | 'error'
+// 'error'    — transport failure / non-2xx. Aborts the batch, and
+//              carries the failure so the toast can say why — see
+//              home/upload-error.
+type UploadResult =
+  | { status: 'ok' }
+  | { status: 'rejected' }
+  | { status: 'error'; failure: UploadFailure }
 
 const DATE_FMT_MAP: Record<string, string> = {
   'mm/dd/yyyy': 'm/d/Y',
@@ -376,20 +391,20 @@
 
       this.uploadTotal = files.length
       let succeeded = 0
-      let aborted = false
+      let failure: UploadFailure | null = null
       for (let i = 0; i < files.length; i++) {
         this.uploadIndex = i + 1
         this.uploadFileName = files[i].name
         const result = await this.uploadOne(url, csrf, files[i])
-        if (result === 'error') {
+        if (result.status === 'error') {
           // Transport failure — something's wrong with the request
           // itself, so stop the batch rather than hammering on.
-          aborted = true
+          failure = result.failure
           break
         }
         // 'rejected' files already surfaced their own server toast;
         // skip them and keep uploading the rest of the selection.
-        if (result === 'ok') succeeded += 1
+        if (result.status === 'ok') succeeded += 1
       }
 
       // Clear the input so re-selecting the same file(s) fires change
@@ -401,11 +416,15 @@
       this.uploadIndex = 0
       this.uploadTotal = 0
 
-      if (aborted) {
+      if (failure) {
         const store = window.Alpine.store('toasts') as
           | ToastStoreLike
           | undefined
-        store?.push('error', 'Upload failed — check the file and try again')
+        store?.push(
+          'error',
+          uploadErrorMessage(failure),
+          UPLOAD_ERROR_TOAST_MS,
+        )
       }
       if (succeeded > 0) {
         this.mode = null
@@ -470,16 +489,27 @@
         xhr.addEventListener('load', () => {
           const kind = fireToastFromHeader(xhr.getResponseHeader('HX-Trigger'))
           if (xhr.status < 200 || xhr.status >= 300) {
-            resolve('error')
+            // Pass the status up so the batch can name the cause. A
+            // proxy-generated 413 never reaches Django, so there is no
+            // HX-Trigger toast to replay and the status is all the
+            // information there is.
+            resolve({
+              status: 'error',
+              failure: { kind: 'http', status: xhr.status },
+            })
             return
           }
           // The server validates and may refuse a file with a 200 +
           // error toast (invalid type, missing file). Treat that as a
           // rejected file, not a silent success.
-          resolve(kind === 'error' ? 'rejected' : 'ok')
+          resolve({ status: kind === 'error' ? 'rejected' : 'ok' })
         })
-        xhr.addEventListener('error', () => resolve('error'))
-        xhr.addEventListener('abort', () => resolve('error'))
+        // No response at all (dropped connection, DNS, TLS): status is
+        // 0 here, so it is not an HTTP failure.
+        const networkFailure = () =>
+          resolve({ status: 'error', failure: { kind: 'network' } })
+        xhr.addEventListener('error', networkFailure)
+        xhr.addEventListener('abort', networkFailure)
         const fd = new FormData()
         fd.append('csrfmiddlewaretoken', csrf)
         fd.append('file_upload', file)

src/anthias_server/app/static/src/home/upload-error.test.ts

@@ -0,0 +1,68 @@
+// Behavioural tests for the upload failure → toast message mapping.
+// Run with
+// `bun test src/anthias_server/app/static/src/home/upload-error.test.ts`.
+//
+// These pin the status the UI previously swallowed — 413, which a
+// proxy in front of Anthias returns when the body exceeds its limit
+// (Cloudflare's 100 MB cap on Free and Pro being the common one).
+
+import { describe, expect, test } from 'bun:test'
+
+import { uploadErrorMessage } from './upload-error'
+
+describe('uploadErrorMessage', () => {
+  test('413 names the size limit rather than blaming the file', () => {
+    expect(uploadErrorMessage({ kind: 'http', status: 413 })).toBe(
+      'File too large — it exceeds the upload size limit of the server ' +
+        'or a proxy in front of it',
+    )
+  })
+
+  test('403 points at the stale page rather than the file', () => {
+    expect(uploadErrorMessage({ kind: 'http', status: 403 })).toBe(
+      'Upload rejected — reload the page and try again',
+    )
+  })
+
+  test('5xx points at the device logs', () => {
+    const expected =
+      'The server failed while handling the upload — check the device logs'
+    expect(uploadErrorMessage({ kind: 'http', status: 500 })).toBe(expected)
+    expect(uploadErrorMessage({ kind: 'http', status: 502 })).toBe(expected)
+  })
+
+  // 507 is intentionally not special-cased: the browser upload path
+  // answers a full disk with 200 + an HX-Trigger toast, so this only
+  // arrives from an API caller and the generic 5xx line is right.
+  test('507 falls through to the generic server message', () => {
+    expect(uploadErrorMessage({ kind: 'http', status: 507 })).toBe(
+      'The server failed while handling the upload — check the device logs',
+    )
+  })
+
+  test('other 4xx keep the original generic wording', () => {
+    const expected = 'Upload failed — check the file and try again'
+    expect(uploadErrorMessage({ kind: 'http', status: 400 })).toBe(expected)
+    expect(uploadErrorMessage({ kind: 'http', status: 404 })).toBe(expected)
+    expect(uploadErrorMessage({ kind: 'http', status: 415 })).toBe(expected)
+  })
+
+  // A lost connection and a proxy rejecting an oversized body are
+  // indistinguishable here — the browser does not always salvage the
+  // 413 — so the message names both and asserts neither.
+  test('a transport failure names both causes', () => {
+    expect(uploadErrorMessage({ kind: 'network' })).toBe(
+      'Upload failed mid-transfer — check your connection, or try a ' +
+        'smaller file',
+    )
+  })
+
+  // A stray status 0 should never reach here (the caller maps it to
+  // `network`), but pin the fallback so a leak can't render an empty
+  // or nonsensical toast.
+  test('a stray status 0 still yields the generic message', () => {
+    expect(uploadErrorMessage({ kind: 'http', status: 0 })).toBe(
+      'Upload failed — check the file and try again',
+    )
+  })
+})