diff --git a/docs/architecture/02-persistence.md b/docs/architecture/02-persistence.md index aa9fdf8..5eaa647 100644 --- a/docs/architecture/02-persistence.md +++ b/docs/architecture/02-persistence.md @@ -40,6 +40,8 @@ Most adapters are driven by **background subscribers** (arch 01 §5, _Effects_): **URL-dataset snapshot lifecycle** (the files a change to it touches): add / Refresh → component fetches (`infrastructure/remote-data.ts`) → `core/dataset.snapshotFromText` shapes the body by sniffed format → `DatasetStore.commitUrlSnapshot` / `refreshDataset` snapshots + profiles it _exactly like inline data_ → render resolves it cached-first in `core/rendering.resolvedData` (a live-URL fallback applies only while a URL dataset is still unfetched). See spec §05 for the behavior. +A fetched body is held whole in memory and stored whole in IndexedDB, so `remote-data.ts` caps it at a fixed ceiling (`MAX_REMOTE_BYTES`): it rejects on the declared `Content-Length` _before_ reading, so an oversized download never lands in memory, with a body-length backstop for chunked responses that declare no length. The classified `too-large` reason maps to its own copy in `services/remote-data-errors.ts` — neither the inline-paste nor the retry recovery helps an oversized file, so that message points at using a smaller or pre-aggregated source instead. + --- ## 2. IndexedDB Wrapper diff --git a/docs/spec/05-datasets.md b/docs/spec/05-datasets.md index c73b0e7..528dafa 100644 --- a/docs/spec/05-datasets.md +++ b/docs/spec/05-datasets.md @@ -90,7 +90,7 @@ A destructive or off-screen outcome raises a confirming toast; an action whose r - **Copy Reference** — copies the by-name reference object to the clipboard, ready to paste into a spec: `{ "data": { "name": "MyDataset" } }` The clipboard write is invisible, so it is confirmed _inline on the control_ ("Copied"), announced politely to assistive technology — not a toast. -- **New / Create New** — opens the create form in the detail pane with fields: **name** (required, unique), **source** toggle (Inline / URL), the **data** (a paste area for inline, a URL field for URL source), and an optional **comment**. Save is disabled until a name and valid data/URL are present. For a **URL** dataset, saving fetches and snapshots the address; the Save control shows a busy state while fetching. If the fetch fails — offline, blocked by the host (cross-origin), not found, empty, or timed out — the form shows a readable, cause-specific error and offers a one-click **"Paste data inline instead"** that switches the form to an inline paste (keeping the name and comment), rather than saving a broken record. On success the new dataset is shown selected in the detail pane — that visible result is the confirmation, so no toast is raised (a dataset created _off-screen_ via Extract does toast; see _Spec Editor_). +- **New / Create New** — opens the create form in the detail pane with fields: **name** (required, unique), **source** toggle (Inline / URL), the **data** (a paste area for inline, a URL field for URL source), and an optional **comment**. Save is disabled until a name and valid data/URL are present. For a **URL** dataset, saving fetches and snapshots the address; the Save control shows a busy state while fetching. If the fetch fails — offline, blocked by the host (cross-origin), not found, empty, timed out, or larger than the fetch size limit — the form shows a readable, cause-specific error and offers a one-click **"Paste data inline instead"** that switches the form to an inline paste (keeping the name and comment), rather than saving a broken record. The size-limit case is the exception: because pasting an oversized file inline would not help, its message points at using a smaller or pre-aggregated source rather than the inline fallback. On success the new dataset is shown selected in the detail pane — that visible result is the confirmation, so no toast is raised (a dataset created _off-screen_ via Extract does toast; see _Spec Editor_). - **Refresh** (URL datasets) — re-fetches the dataset's address and re-snapshots it, re-profiling the data and advancing the last-fetched time. The updated figures are the visible confirmation, so success raises no toast; a failed refresh raises an error toast. Refresh is also how a URL dataset that has not yet been fetched (e.g. one migrated from an older version) acquires its snapshot. - **Edit** — rename, edit the comment, change the source data, and (for URL datasets) change the address. Updating inline data re-profiles it; changing a URL dataset's address re-fetches and re-snapshots it; editing only a URL dataset's name or comment does **not** re-fetch. The modified timestamp advances. - **Delete** — asks for confirmation ("Delete \"Name\"? This cannot be undone."), then removes the dataset and clears the selection. diff --git a/src/app/infrastructure/remote-data.test.ts b/src/app/infrastructure/remote-data.test.ts index 804fd8f..9bb044c 100644 --- a/src/app/infrastructure/remote-data.test.ts +++ b/src/app/infrastructure/remote-data.test.ts @@ -8,15 +8,18 @@ function fakeResponse(opts: { statusText?: string; body?: string; contentType?: string | null; + contentLength?: number; }): Response { + const headers: Record = { + 'content-type': opts.contentType ?? null, + 'content-length': opts.contentLength !== undefined ? String(opts.contentLength) : null, + }; return { ok: opts.ok, status: opts.status ?? (opts.ok ? 200 : 500), statusText: opts.statusText ?? '', text: () => Promise.resolve(opts.body ?? ''), - headers: { - get: (k: string) => (k.toLowerCase() === 'content-type' ? (opts.contentType ?? null) : null), - }, + headers: { get: (k: string) => headers[k.toLowerCase()] ?? null }, } as unknown as Response; } @@ -84,6 +87,36 @@ describe('fetchRemoteData', () => { }); }); + test('rejects an oversized body declared by Content-Length, before reading it', async () => { + const text = vi.fn(() => Promise.resolve('x')); + vi.stubGlobal( + 'fetch', + vi.fn(() => + Promise.resolve({ + ok: true, + status: 200, + statusText: '', + text, + headers: { get: (k: string) => (k.toLowerCase() === 'content-length' ? '999' : null) }, + } as unknown as Response), + ), + ); + await expect( + fetchRemoteData('https://example.com/huge.csv', { maxBytes: 100 }), + ).rejects.toMatchObject({ reason: 'too-large' }); + expect(text).not.toHaveBeenCalled(); // never read the body + }); + + test('rejects an oversized body as a backstop when no length is declared', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(() => Promise.resolve(fakeResponse({ ok: true, body: 'abcdefghij' }))), + ); + await expect( + fetchRemoteData('https://example.com/chunked.csv', { maxBytes: 5 }), + ).rejects.toMatchObject({ reason: 'too-large' }); + }); + test('the thrown error is a RemoteFetchError', async () => { vi.stubGlobal( 'fetch', diff --git a/src/app/infrastructure/remote-data.ts b/src/app/infrastructure/remote-data.ts index 96548c4..a42fc88 100644 --- a/src/app/infrastructure/remote-data.ts +++ b/src/app/infrastructure/remote-data.ts @@ -16,7 +16,7 @@ */ /** Why a remote fetch failed, in terms the UI maps to copy + a recovery path. */ -export type RemoteFetchReason = 'network' | 'http' | 'empty' | 'timeout'; +export type RemoteFetchReason = 'network' | 'http' | 'empty' | 'timeout' | 'too-large'; /** A classified remote-fetch failure. `status` is set only for `http`. */ export class RemoteFetchError extends Error { @@ -42,9 +42,20 @@ export interface RemoteData { /** Default ceiling on a single fetch before it is aborted as a timeout. */ const DEFAULT_TIMEOUT_MS = 30_000; +/** + * Hard ceiling on a snapshotted remote body. A URL dataset is held whole in memory + * while profiling and then stored whole in IndexedDB, so an unbounded download + * (a multi-hundred-MB file) would jank or exhaust the tab. Checked against the + * declared `Content-Length` before reading, and against the body as a backstop for + * chunked responses that declare no length. + */ +export const MAX_REMOTE_BYTES = 50 * 1024 * 1024; + export interface FetchRemoteDataOptions { /** Abort the fetch after this many ms (default 30 s). */ timeoutMs?: number; + /** Reject a body larger than this many bytes (default {@link MAX_REMOTE_BYTES}). */ + maxBytes?: number; } function describeNetworkError(err: unknown): string { @@ -52,6 +63,11 @@ function describeNetworkError(err: unknown): string { return `Couldn't reach the URL (${reason}).`; } +/** A byte count as a rounded MB string for diagnostic messages (e.g. `120 MB`). */ +function megabytes(bytes: number): string { + return `${Math.round(bytes / (1024 * 1024))} MB`; +} + /** * Fetch a remote dataset resource once and return its raw body. Throws * `RemoteFetchError` on any failure — the caller distinguishes recovery by @@ -63,6 +79,7 @@ export async function fetchRemoteData( options: FetchRemoteDataOptions = {}, ): Promise { const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; + const maxBytes = options.maxBytes ?? MAX_REMOTE_BYTES; const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), timeoutMs); @@ -88,12 +105,24 @@ export async function fetchRemoteData( ); } + // Reject an oversized body before reading it, when the server declares the size — + // so a huge download never lands in memory in the first place. + const declared = Number(response.headers.get('content-length')); + if (Number.isFinite(declared) && declared > maxBytes) { + throw new RemoteFetchError('too-large', `The file is about ${megabytes(declared)}.`); + } + const text = await response.text(); if (text.trim() === '') { throw new RemoteFetchError('empty', 'The URL returned no data.'); } - // TODO: guard very large responses (cap or warn) before snapshotting into - // IndexedDB — a multi-hundred-MB file would load fully into memory + storage. + // Backstop for a chunked response that declared no Content-Length: the body is + // already in memory, but this still keeps it out of IndexedDB. `text.length` + // (UTF-16 units) slightly under-counts multibyte bytes — fine for a ceiling. + if (text.length > maxBytes) { + throw new RemoteFetchError('too-large', `The file is over ${megabytes(maxBytes)}.`); + } + return { text, contentType: response.headers.get('content-type') }; } diff --git a/src/app/services/remote-data-errors.test.ts b/src/app/services/remote-data-errors.test.ts index 9c15723..4dfc9d0 100644 --- a/src/app/services/remote-data-errors.test.ts +++ b/src/app/services/remote-data-errors.test.ts @@ -32,6 +32,13 @@ describe('remoteFetchErrorMessage', () => { expect(msg).not.toMatch(/inline/i); }); + test('a too-large body names the size limit and a real fix, not the inline fallback', () => { + const msg = remoteFetchErrorMessage(new RemoteFetchError('too-large', 'x')); + expect(msg).toMatch(/too large/i); + expect(msg).toMatch(/MB/); + expect(msg).not.toMatch(/inline/i); + }); + test('an unknown error still produces a sensible fallback message', () => { expect(remoteFetchErrorMessage(new Error('boom'))).toMatch(/couldn't fetch this url/i); }); diff --git a/src/app/services/remote-data-errors.ts b/src/app/services/remote-data-errors.ts index 716635a..97ad34d 100644 --- a/src/app/services/remote-data-errors.ts +++ b/src/app/services/remote-data-errors.ts @@ -10,7 +10,7 @@ * create/edit form can fall back to pasting inline; Refresh can only retry. */ -import { RemoteFetchError } from '../infrastructure/remote-data'; +import { MAX_REMOTE_BYTES, RemoteFetchError } from '../infrastructure/remote-data'; /** Where the error surfaces, which decides the recovery the copy points at. */ export type FetchRecovery = 'inline' | 'retry'; @@ -45,6 +45,10 @@ export function remoteFetchErrorMessage(err: unknown, recovery: FetchRecovery = return `This URL returned no data. Make sure it points directly at a data file. ${next}`; case 'timeout': return `This URL took too long to respond. ${next}`; + case 'too-large': + // Neither inline-paste nor a retry helps an oversized file, so this points + // at the only real fix instead of the shared next step. + return `This file is too large to store as a dataset (over ${Math.round(MAX_REMOTE_BYTES / (1024 * 1024))} MB). Use a smaller file, or pre-aggregate the data before loading it.`; } } return `Couldn't fetch this URL. ${next}`;