Files
siftlode/frontend/src/components/WriteFailureReporting.dom.test.tsx
T
peter 7c0647f87a fix(uploads): finish the sweeps round 1 left half-done
Deep-review round 2, on round 1's own fix commit. It found MORE than round 1
did (28 raw, against 13), and most of it was mine -- a new piece of UI attracts
findings a backend-only diff never does. The pattern in almost all of them is
the same: a principle applied to the site that prompted it and not to its
siblings.

* `display_text` reached three of the FIVE writers of a user-supplied name.
  `POST /api/downloads` and `POST /api/downloads/edit` still handed Postgres an
  unbounded string, so a 300-character name pasted into the Add-download dialog
  or the editor was a bare 500 (main.py re-raises every DataError but the NUL
  case). Four of the seven reviewers found it independently. The bound now lives
  in `models.py`, the three columns are DECLARED with it, and the guard for the
  next writer is parsed rather than remembered: every `display_name` /
  `display_uploader` assignment under `app/routes/` must go through the helper.
  That guard immediately found two more -- the Google sign-in callback writes
  the profile name into the same 255-wide column -- so it is six, not five.

* The connection release reached one of the THREE routes that read their own
  body. The screenshot route (10 MiB, from a phone) and the client log sink both
  still held a pooled connection idle-in-transaction for the length of the
  upload, which is the invariant db.py sizes the pool on. All three now call
  `db.release_read_connection`, whose precondition -- nothing pending on the
  shared session -- is CHECKED rather than described, because a whole-session
  rollback discarding a caller's staged write is a mistake this repo has already
  made once. `test_body_limit.py` holds the rule for the next such route.

* The AST drift registry keyed its sites by (module, limit expression), so a
  second route copied from the first collapsed onto the same entry and went
  uncovered -- the exact hole the registry exists to close. Keyed per call site
  now, and it carries the release requirement too.

* The dialog's own refusal was announced once and never again: two over-cap
  picks leave an identical node with identical text, and an untouched live
  region says nothing. It is keyed per pick now, the drop zone reads as invalid
  and points at the sentence, and the reason is reachable from the control
  rather than only from a one-shot announcement.

* Two tests rebuilt the blind spot they were written to remove: the dialog's cap
  test sat nowhere near the boundary (a `>` loosened to `>=` survived it, which
  is the mutation round 1 fixed on the server side), and the header-precedence
  test only covered an exact-case duplicate while a differently-cased one would
  have been COMBINED onto the wire rather than replaced. Both fixed, and the
  merge now dedupes case-insensitively because header names are.

Also: the upload dialog's unstubbed usage query was firing a real request from
jsdom in a neighbouring suite and leaking a delayed "connection lost" toast
across test boundaries; `GET /api/downloads/usage` had no test at all though the
whole client-side check hangs off it; and the comment claiming the edge 413 is
readable now says what was measured -- readable to 200 MiB over loopback, a bare
`onerror` through a proxy -- so the pre-send check is named as the mechanism and
that branch as the backstop it is.

Every test added or changed here was mutation-checked. One mutation SURVIVED and
is recorded rather than papered over: the `tooBig` term inside `submit()` cannot
be reached, because `ui/Button` swallows the activation itself, so it is marked
as belt and braces instead of being described as the guard.
2026-09-22 23:31:55 +02:00

178 lines
7.4 KiB
TypeScript

// @vitest-environment jsdom
//
// A write that fails is named, wherever it fails. `req`/`uploadReq` deliberately leave 403/404/429
// to the caller (see reportFailure) and answer a 502/503/504 with only the transient "connection
// lost" notice — so a handler with no `catch` reports nothing at all. Round 4 swept a dozen such
// call sites and missed these two: the collection editor's three siblings, in the very file it
// edited, and the one `uploadReq` caller, whose empty catch still claimed the global error modal had
// it (deep-review round 5).
import { describe, it, expect, vi, beforeAll, beforeEach, afterEach } from "vitest";
import { render, cleanup, screen, fireEvent, waitFor } from "@testing-library/react";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
vi.mock("react-i18next", async (importOriginal) => {
const actual = await importOriginal<typeof import("react-i18next")>();
return {
...actual,
useTranslation: () => ({ t: (key: string) => key, i18n: { language: "en" } }),
};
});
vi.mock("./confirmContext", () => ({ useConfirm: () => async () => true }));
const { notify } = vi.hoisted(() => ({
notify: vi.fn((_n: { level: string; title?: string; message: string }) => 0),
}));
vi.mock("../lib/notifications", async (importOriginal) => ({
...(await importOriginal<typeof import("../lib/notifications")>()),
notify,
}));
import PlexCollectionEditor from "./PlexCollectionEditor";
import UploadDialog from "./UploadDialog";
import DownloadDialog from "./DownloadDialog";
import { api, HttpError } from "../lib/api";
beforeAll(() => {
class Stub {
observe() {}
unobserve() {}
disconnect() {}
takeRecords() {
return [];
}
}
globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver;
globalThis.IntersectionObserver ??= Stub as unknown as typeof IntersectionObserver;
});
beforeEach(() => notify.mockClear());
afterEach(() => {
cleanup();
vi.restoreAllMocks();
});
// What failed, as the toast names it: the title (E11 S2b — the title says WHAT, the line under it WHY).
const errors = () =>
notify.mock.calls.filter(([n]) => n.level === "error").map(([n]) => n.title ?? n.message);
const client = () => new QueryClient({ defaultOptions: { queries: { retry: false } } });
describe("the Plex collection editor", () => {
const collections = [
{ id: "c1", title: "Marathon", can_edit: true, child_count: 3, smart: false },
// Not marked "mine to edit" yet, so it offers the take-over button.
{
id: "c2",
title: "Theirs",
can_edit: false,
child_count: 9,
smart: false,
source: "collection",
},
];
const mount = () =>
render(
<QueryClientProvider client={client()}>
<PlexCollectionEditor
item={{ id: "m1", title: "A Movie" }}
library="1"
memberOf={[]}
onClose={() => {}}
onChanged={() => {}}
/>
</QueryClientProvider>,
);
it("says so when adding to a collection that is gone, instead of nothing happening", async () => {
vi.spyOn(api, "plexCollections").mockResolvedValue({ collections } as never);
vi.spyOn(api, "plexCollectionAddItem").mockRejectedValue(
new HttpError(404, "Unknown collection"),
);
mount();
fireEvent.click(await screen.findByText("Marathon"));
await waitFor(() => expect(errors()).toEqual(["plex.collection.editFailed"]));
});
it("says so when enabling editing is refused", async () => {
vi.spyOn(api, "plexCollections").mockResolvedValue({ collections } as never);
vi.spyOn(api, "plexSetCollectionEditable").mockRejectedValue(
new HttpError(403, "This collection is read-only"),
);
mount();
fireEvent.click(await screen.findByText("plex.collEditor.makeEditable"));
await waitFor(() => expect(errors()).toEqual(["plex.collection.editFailed"]));
});
it("says so when a CONFIRMED delete does not happen", async () => {
// The worst of the three to lose: the user has just confirmed a destructive dialog.
vi.spyOn(api, "plexCollections").mockResolvedValue({ collections } as never);
vi.spyOn(api, "plexDeleteCollection").mockRejectedValue(
new HttpError(404, "Unknown collection"),
);
mount();
fireEvent.click(await screen.findByText("plex.collEditor.delete"));
await waitFor(() => expect(errors()).toEqual(["plex.collection.deleteFailed"]));
});
});
describe("the upload dialog", () => {
const pick = () => {
const input = document.querySelector('input[type="file"]') as HTMLInputElement;
const file = new File(["x"], "clip.mp4", { type: "video/mp4" });
Object.defineProperty(input, "files", { value: [file], configurable: true });
fireEvent.change(input);
};
const mount = () =>
render(
<QueryClientProvider client={client()}>
<UploadDialog onClose={() => {}} />
</QueryClientProvider>,
);
beforeEach(() => {
// The dialog reads the per-file cap on mount. Unstubbed it is a REAL fetch from jsdom, which
// fails, retries once after 250 ms and then raises the "connection lost" notice — a delayed
// toast that outlives the test that spawned it and lands as an unexpected error in whichever
// test is running by then. The file is fast enough today that it always lands after the suite,
// which is a coincidence, not a guarantee (deep-review round 2).
vi.spyOn(api, "downloadUsage").mockResolvedValue({
upload_max_bytes: 1_000_000,
} as Awaited<ReturnType<typeof api.downloadUsage>>);
});
it("names an upload the server refused", async () => {
// A 502 from the proxy raises only the self-clearing connection notice; the next successful poll
// wipes it, and the dialog snapped back with no error and no job in the list.
vi.spyOn(api, "uploadDownload").mockRejectedValue(new HttpError(502, "Bad Gateway"));
mount();
pick();
fireEvent.click(screen.getByText("downloads.upload.submit"));
await waitFor(() => expect(errors()).toEqual(["downloads.upload.failed"]));
});
it("says nothing when the user cancelled it themselves", async () => {
// A cancel is not a failure — and it rejects with the same status 0 a genuine network drop does,
// which is why the difference is a named sentinel (`isAborted`) and not a status check.
const upload = vi.spyOn(api, "uploadDownload").mockRejectedValue(new HttpError(0, "aborted"));
mount();
pick();
fireEvent.click(screen.getByText("downloads.upload.submit"));
await waitFor(() => expect(upload).toHaveBeenCalled());
expect(errors()).toEqual([]);
});
});
describe("the download dialog", () => {
it("names a queueing the server refused with no dialog of its own", async () => {
// Its catch said "the global error dialog already surfaced the reason" — true for a quota 422,
// not for a 403/404/429 or a lost connection, where the dialog just stayed open (E11 S2b).
vi.spyOn(api, "downloadProfiles").mockResolvedValue([{ id: 1, name: "Best" }] as never);
vi.spyOn(api, "enqueueDownload").mockRejectedValue(new HttpError(403, "nope"));
render(
<QueryClientProvider client={client()}>
<DownloadDialog source="https://example.com/v" onClose={() => {}} />
</QueryClientProvider>,
);
await screen.findByText("Best");
fireEvent.click(screen.getByText("downloads.dialog.submit"));
await waitFor(() => expect(errors()).toEqual(["errors.writeFailed.enqueueDownload"]));
});
});