diff --git a/db/test/live-share.test.ts b/db/test/live-share.test.ts index 6e068e9..2866574 100644 --- a/db/test/live-share.test.ts +++ b/db/test/live-share.test.ts @@ -186,7 +186,8 @@ test('live share routes', async (t) => { const answer = await refresh(deps, shareId, cardId); assert.equal(answer.status, 404, name); - assert.deepEqual(answer.body, { error: 'not_found' }, name); + assert.equal(answer.body.code, 'not_found', name); + assert.equal(answer.body.retryable, false, name); assert.equal((await download(deps, shareId, cardId)).status, 404, name); } @@ -279,7 +280,7 @@ test('live share routes', async (t) => { assert.equal(waiting.status, 503); assert.equal(waiting.headers.get('retry-after'), '2'); - assert.deepEqual(await waiting.json(), { status: 'pending', retry_after_s: 2 }); + assert.equal((await waiting.json()).retry_after_s, 2); }); await t.test('no credential: an error answer and no back-end call', async () => { @@ -295,7 +296,7 @@ test('live share routes', async (t) => { const file = await download(unset, id, 'data'); assert.equal(file.status, 502); - assert.deepEqual(await file.json(), { status: 'error', code: 'live_unavailable' }); + assert.equal((await file.json()).code, 'live_unavailable'); assert.equal(stub.calls.length, 0); assert.deepEqual(await rows(id), []); }); @@ -562,17 +563,23 @@ test('live share routes', async (t) => { assert.equal(waiting.status, 503); assert.equal(waiting.headers.get('retry-after'), '9'); - assert.deepEqual(await waiting.json(), { status: 'pending', retry_after_s: 9 }); + const pending = await waiting.json(); + + assert.equal(waiting.headers.get('content-type'), 'application/problem+json'); + assert.equal(pending.code, 'refresh_pending'); + assert.equal(pending.retryable, true); + assert.equal(pending.retry_after_s, 9); const failed = over({ refreshDerived: (call) => aiRefusal('recipe_missing', call) }); const none = await download(failed.deps, id, 'derived'); assert.equal(none.status, 502); - assert.deepEqual(await none.json(), { - status: 'error', - code: 'recipe_missing', - retry_after_s: 30, - }); + const missing = await none.json(); + + assert.equal(none.headers.get('content-type'), 'application/problem+json'); + assert.equal(missing.code, 'recipe_missing'); + assert.equal(missing.retryable, false); + assert.equal(missing.retry_after_s, 30); await age(id); @@ -587,13 +594,13 @@ test('live share routes', async (t) => { const badFormat = await download(refusing.deps, id, 'data', 'format=xlsx'); assert.equal(badFormat.status, 400); - assert.deepEqual(await badFormat.json(), { status: 'error', code: 'request_invalid' }); + assert.equal((await badFormat.json()).code, 'request_invalid'); // A refusal of the server's own credential is not the caller's error. const badCredential = await download(refusing.deps, id, 'data', 'format=csv'); assert.equal(badCredential.status, 502); - assert.deepEqual(await badCredential.json(), { status: 'error', code: 'unauthorized' }); + assert.equal((await badCredential.json()).code, 'unauthorized'); }); await t.test('stored answers go with the share and with a switch to snapshot', async () => { @@ -683,7 +690,7 @@ test('live share routes', async (t) => { ]); assert.equal(res.status, 502); - assert.deepEqual(await res.json(), { status: 'error', code: 'backend_timeout' }); + assert.equal((await res.json()).code, 'backend_timeout'); }); await t.test('a share deleted mid-refresh answers 404, not a foreign-key failure', async () => { @@ -722,7 +729,8 @@ test('live share routes', async (t) => { const answer = await refresh({ ...deps, db: racing }, id, 'data'); - assert.deepEqual(answer, { status: 404, body: { error: 'not_found' } }); + assert.equal(answer.status, 404); + assert.equal(answer.body.code, 'not_found'); assert.deepEqual(await rows(id), []); }); diff --git a/docs/development.md b/docs/development.md index 75d3b98..ccc1c54 100644 --- a/docs/development.md +++ b/docs/development.md @@ -221,7 +221,7 @@ cd pnpm install ``` -A new package version packs under a new file name. Point the `@developmentfinance/client` entry in `package.json` at it before re-locking. Commit the tarball and the lockfile together. +A new package version packs under a new file name. Point the `@developmentfinance/client` entry in `package.json` at it before re-locking. A rebuilt tarball keeps its file name, so a local re-vendor needs the client's directory under `node_modules/.pnpm` deleted before `pnpm install --frozen-lockfile` picks it up. Commit the tarball and the lockfile together. ## Conventions diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 037f6b0..f9658b9 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -945,7 +945,7 @@ packages: postcss-selector-parser: ^7.1.1 '@developmentfinance/client@file:vendor/developmentfinance-client-0.1.0.tgz': - resolution: {integrity: sha512-shJGkuZFt1Iny5g0E8wKdDdNLmyhbQpaySFVD++PcKeTm8do4igX6rCKHZGKbTgUFSQqtUXGF6TuHjsVZveFQA==, tarball: file:vendor/developmentfinance-client-0.1.0.tgz} + resolution: {integrity: sha512-Npl9G5tM6qOQyFa9vlJ9hci414gHBJAeeFdAwLe8zOqWezObmJZ2+Nc86nso0s1uZe4/XVMHfkQEjKiWt9pTCQ==, tarball: file:vendor/developmentfinance-client-0.1.0.tgz} version: 0.1.0 engines: {node: '>=22.18'} peerDependencies: diff --git a/src/app/api/conversations/[id]/route.ts b/src/app/api/conversations/[id]/route.ts index 3301d04..7d1db92 100644 --- a/src/app/api/conversations/[id]/route.ts +++ b/src/app/api/conversations/[id]/route.ts @@ -7,6 +7,7 @@ import { upsertConversation, } from '@/lib/chat/conversationStore'; import type { SavedConversation } from '@/lib/chat/history'; +import { missingProblem, requestInvalidProblem, unauthorizedProblem } from '@/lib/proxy/problem'; type Params = { params: Promise<{ id: string }> }; @@ -17,14 +18,14 @@ export const GET = async (_request: Request, { params }: Params) => { const user = await getUser(); if (!user) { - return NextResponse.json({ error: 'unauthorized' }, { status: 401 }); + return unauthorizedProblem(); } const { id } = await params; const saved = await getConversation(user.id, id); if (!saved) { - return NextResponse.json({ error: 'not_found' }, { status: 404 }); + return missingProblem('There is no such conversation.'); } return NextResponse.json({ conversation: saved }); @@ -34,7 +35,7 @@ export const PUT = async (request: Request, { params }: Params) => { const user = await getUser(); if (!user) { - return NextResponse.json({ error: 'unauthorized' }, { status: 401 }); + return unauthorizedProblem(); } const { id } = await params; @@ -46,7 +47,9 @@ export const PUT = async (request: Request, { params }: Params) => { typeof body.title !== 'string' || !Array.isArray(body.messages) ) { - return NextResponse.json({ error: 'bad_request' }, { status: 400 }); + return requestInvalidProblem( + 'The body needs the conversation id from the path, a title, and a list of messages.', + ); } await upsertConversation(user.id, body); @@ -58,7 +61,7 @@ export const DELETE = async (_request: Request, { params }: Params) => { const user = await getUser(); if (!user) { - return NextResponse.json({ error: 'unauthorized' }, { status: 401 }); + return unauthorizedProblem(); } const { id } = await params; diff --git a/src/app/api/conversations/route.ts b/src/app/api/conversations/route.ts index 8172724..a6161e1 100644 --- a/src/app/api/conversations/route.ts +++ b/src/app/api/conversations/route.ts @@ -2,6 +2,7 @@ import { NextResponse } from 'next/server'; import { getUser } from '@/lib/auth/session'; import { listConversations, searchConversations } from '@/lib/chat/conversationStore'; +import { unauthorizedProblem } from '@/lib/proxy/problem'; // GET /api/conversations → the signed-in user's history (list columns only) // GET /api/conversations?q=… → the ones whose title or transcript match @@ -9,7 +10,7 @@ export const GET = async (request: Request) => { const user = await getUser(); if (!user) { - return NextResponse.json({ error: 'unauthorized' }, { status: 401 }); + return unauthorizedProblem(); } const q = new URL(request.url).searchParams.get('q')?.trim() ?? ''; diff --git a/src/app/api/download-pin/route.ts b/src/app/api/download-pin/route.ts index fbb403d..6c4b62d 100644 --- a/src/app/api/download-pin/route.ts +++ b/src/app/api/download-pin/route.ts @@ -2,6 +2,8 @@ import type { NextRequest } from 'next/server'; import { readBoundedText } from '@/lib/proxy/boundedBody'; import { clientIp } from '@/lib/proxy/forward'; +import { problem, rateLimitedProblem, requestInvalidProblem } from '@/lib/proxy/problem'; +import { rateHeaders } from '@/lib/proxy/rateHeaders'; import { hit } from '@/lib/proxy/rateLimit'; import { MAX_DOWNLOAD_PIN_BODY, parseDownloadPin, pinVersionFrame } from '@/lib/search/snapshot'; import { needsPin, pinVersionOnce } from '@/lib/search/versionPin'; @@ -23,13 +25,13 @@ const CAPTURE_POLL_DEADLINE_MS = 50_000; // 413 as soon as the count passes it, chunked or not, and a malformed one // fails parseDownloadPin. A pin the door refuses, or cannot take right now, is // logged with the door's detail and left unpinned for a later download to -// retry. +// retry. Every refusal is a problem document (src/lib/proxy/problem). export const POST = async (req: NextRequest) => { // A pin can mint, so it has its own small allowance. const verdict = await hit('pin', clientIp(req)); if (!verdict.allowed) { - return Response.json({ error: 'rate_limited' }, { status: 429 }); + return rateLimitedProblem(verdict, rateHeaders(verdict)); } const text = await readBoundedText(req.body, MAX_DOWNLOAD_PIN_BODY).catch((error: unknown) => { @@ -42,17 +44,22 @@ export const POST = async (req: NextRequest) => { }); if (text === undefined) { - return Response.json({ error: 'bad_request' }, { status: 400 }); + return requestInvalidProblem('The request body could not be read.'); } if (text === null) { - return Response.json({ error: 'payload_too_large' }, { status: 413 }); + return problem({ + status: 413, + code: 'payload_too_large', + detail: `The request body is over ${MAX_DOWNLOAD_PIN_BODY} bytes.`, + retryable: false, + }); } const body = parseDownloadPin(text); if (!body) { - return Response.json({ error: 'bad_request' }, { status: 400 }); + return requestInvalidProblem('The request body is not a download pin.'); } if (!needsPin(body)) { @@ -86,6 +93,11 @@ export const POST = async (req: NextRequest) => { cause: cause instanceof Error ? cause.message : undefined, }); - return Response.json({ error: 'pin_refused' }, { status: 502 }); + return problem({ + status: 502, + code: 'pin_refused', + detail: 'The version could not be pinned. A later download tries again.', + retryable: false, + }); } }; diff --git a/src/app/api/events/route.ts b/src/app/api/events/route.ts index babc4ff..5792d20 100644 --- a/src/app/api/events/route.ts +++ b/src/app/api/events/route.ts @@ -1,14 +1,17 @@ import { NextResponse } from 'next/server'; import { getSession } from '@/lib/auth/session'; -import { fitsProps } from '@/lib/events/limits'; +import { fitsProps, MAX_PROPS_BYTES } from '@/lib/events/limits'; import { EVENT_NAMES, type EventName, track } from '@/lib/events/track'; +import { problem, requestInvalidProblem } from '@/lib/proxy/problem'; /** * Usage-event intake (POST /api/events) for the browser side of * src/lib/events. Body: `{ name, props?, visitorId?, page? }`. The name must * be one the site emits; props are capped so a stray payload can't bloat - * the table. Always answers 204 — the page never waits on analytics. + * the table. Answers 204 for an event taken, and a problem document + * (`request_invalid`, `payload_too_large`) for one refused; the page never + * waits on either. */ const MAX_ID = 64; const MAX_PAGE = 300; @@ -21,7 +24,7 @@ export const POST = async (request: Request) => { const name = body?.name; if (!(EVENT_NAMES as readonly unknown[]).includes(name)) { - return NextResponse.json({ error: 'unknown event' }, { status: 400 }); + return requestInvalidProblem('The event name is not one the site records.', '/name'); } const props = @@ -30,7 +33,12 @@ export const POST = async (request: Request) => { : {}; if (!fitsProps(props)) { - return NextResponse.json({ error: 'props too large' }, { status: 413 }); + return problem({ + status: 413, + code: 'payload_too_large', + detail: `The event props are over ${MAX_PROPS_BYTES} bytes.`, + retryable: false, + }); } const session = await getSession(); diff --git a/src/app/api/roadmap/votes/route.ts b/src/app/api/roadmap/votes/route.ts index 1102187..ebb90d7 100644 --- a/src/app/api/roadmap/votes/route.ts +++ b/src/app/api/roadmap/votes/route.ts @@ -4,6 +4,7 @@ import { NextResponse } from 'next/server'; import { UPCOMING } from '@/components/organisms/Roadmap/roadmapData'; import { getUser } from '@/lib/auth/session'; import { db } from '@/lib/db'; +import { requestInvalidProblem, unauthorizedProblem } from '@/lib/proxy/problem'; import { roadmapVote } from '../../../../../db/schema'; // Only entries still under consideration take votes. @@ -39,14 +40,14 @@ export const POST = async (request: Request) => { const user = await getUser(); if (!user) { - return NextResponse.json({ error: 'unauthorized' }, { status: 401 }); + return unauthorizedProblem(); } const body = (await request.json().catch(() => null)) as { entryId?: unknown } | null; const entryId = typeof body?.entryId === 'string' ? body.entryId : ''; if (!VOTABLE.has(entryId)) { - return NextResponse.json({ error: 'bad_request' }, { status: 400 }); + return requestInvalidProblem('The entry does not take votes.', '/entryId'); } const where = and(eq(roadmapVote.userId, user.id), eq(roadmapVote.entryId, entryId)); diff --git a/src/app/api/share/[id]/route.ts b/src/app/api/share/[id]/route.ts index fd4c1e8..6c739c3 100644 --- a/src/app/api/share/[id]/route.ts +++ b/src/app/api/share/[id]/route.ts @@ -3,12 +3,20 @@ import { NextResponse } from 'next/server'; import { getUser } from '@/lib/auth/session'; import { deleteShare, snapshotShare } from '@/lib/chat/shareStore'; import type { OwnerWriteResult } from '@/lib/chat/shareWrites'; +import { + missingProblem, + problem, + requestInvalidProblem, + unauthorizedProblem, +} from '@/lib/proxy/problem'; import { getSearchShare } from '@/lib/search/searchShare'; type Context = { params: Promise<{ id: string }> }; const SHARE_ID = /^[A-Za-z0-9]{8}$/; +const noSuchShare = (): Response => missingProblem('There is no such share.'); + // GET /api/share/ → a search share's references ({ mode, query, refs }). // Public, like the link itself; conversation shares render server-side on // /s/ and never come through here. @@ -17,7 +25,7 @@ export const GET = async (_request: Request, { params }: Context) => { const found = SHARE_ID.test(id) ? await getSearchShare(id) : null; if (!found) { - return NextResponse.json({ error: 'not_found' }, { status: 404 }); + return noSuchShare(); } return NextResponse.json(found, { headers: { 'Cache-Control': 'no-store' } }); @@ -29,23 +37,28 @@ export const GET = async (_request: Request, { params }: Context) => { const ownerWrite = async ( { params }: Context, write: (id: string, userId: string) => Promise, - done: (id: string) => NextResponse, + done: (id: string) => Response, ) => { const user = await getUser(); if (!user) { - return NextResponse.json({ error: 'unauthorized' }, { status: 401 }); + return unauthorizedProblem(); } const { id } = await params; const result = SHARE_ID.test(id) ? await write(id, user.id) : 'not_found'; if (result === 'not_found') { - return NextResponse.json({ error: 'not_found' }, { status: 404 }); + return noSuchShare(); } if (result === 'unsupported_kind') { - return NextResponse.json({ error: 'unsupported_kind' }, { status: 400 }); + return problem({ + status: 400, + code: 'unsupported_kind', + detail: 'Only a conversation, canvas, or card share can be changed here.', + retryable: false, + }); } return done(id); @@ -58,7 +71,7 @@ export const PATCH = async (request: Request, context: Context) => { const body = (await request.json().catch(() => null)) as { mode?: unknown } | null; if (body?.mode !== 'snapshot') { - return NextResponse.json({ error: 'invalid_mode' }, { status: 400 }); + return requestInvalidProblem('A share can only be switched to `snapshot`.', '/mode'); } return ownerWrite(context, snapshotShare, (id) => NextResponse.json({ id, mode: 'snapshot' })); diff --git a/src/app/api/share/route.ts b/src/app/api/share/route.ts index d283db3..48e4a52 100644 --- a/src/app/api/share/route.ts +++ b/src/app/api/share/route.ts @@ -2,10 +2,11 @@ import { after, NextResponse, type NextRequest } from 'next/server'; import { getUser } from '@/lib/auth/session'; -import { readStudioShareRequest, shareCreateProps } from '@/lib/chat/shareMode'; +import { GUEST_SHARES, readStudioShareRequest, shareCreateProps } from '@/lib/chat/shareMode'; import { createShare, liveShareRefusalFor, spendGuestShare } from '@/lib/chat/shareStore'; import { clientIp } from '@/lib/proxy/forward'; import { readBoundedText } from '@/lib/proxy/boundedBody'; +import { problem, requestInvalidProblem } from '@/lib/proxy/problem'; import { track } from '@/lib/events/track'; import { createSearchShare } from '@/lib/search/searchShare'; import { readSearchShareRequest } from '@/lib/search/searchShareRequest'; @@ -21,7 +22,7 @@ const shareSearch = async (body: any, headers: Headers) => { const request = readSearchShareRequest(body); if (!request) { - return NextResponse.json({ error: 'bad_request' }, { status: 400 }); + return requestInvalidProblem('The request is not a search share.'); } const user = await getUser(); @@ -47,7 +48,14 @@ const shareSearch = async (body: any, headers: Headers) => { cause: cause instanceof Error ? cause.message : undefined, }); - return NextResponse.json({ error: 'platform_refused', detail: message }, { status: 502 }); + return problem({ + status: 502, + code: 'platform_refused', + detail: 'The data platform could not capture this result.', + retryable: false, + // The platform's own code, which the share box words for the viewer. + extra: { platform_code: message }, + }); } // Recorded after the response, so the link does not wait on analytics. @@ -71,11 +79,17 @@ const shareSearch = async (body: any, headers: Headers) => { // conversation. A request that names none stores a snapshot. A live share is // refused when its data cards cannot all be refreshed (`liveShareRefusal` in // src/lib/studio/liveShare), and the refusal names the cards in the way. +// Every refusal is a problem document (src/lib/proxy/problem). export const POST = async (request: NextRequest) => { const text = await readBoundedText(request.body, MAX_SHARE_BODY).catch(() => ''); if (text === null) { - return NextResponse.json({ error: 'payload_too_large' }, { status: 413 }); + return problem({ + status: 413, + code: 'payload_too_large', + detail: `The request body is over ${MAX_SHARE_BODY} bytes.`, + retryable: false, + }); } const body = (() => { @@ -92,26 +106,36 @@ export const POST = async (request: NextRequest) => { const read = readStudioShareRequest(body); - if ('error' in read) { - return NextResponse.json(read, { status: 400 }); + if (read instanceof Response) { + return read; } const user = await getUser(); if (!user) { if (read.mode === 'live') { - return NextResponse.json({ error: 'live_needs_account' }, { status: 403 }); + return problem({ + status: 403, + code: 'live_needs_account', + detail: 'Sign in to make a live share. Without an account, a share is a snapshot.', + retryable: false, + }); } if (!(await spendGuestShare(clientIp(request)))) { - return NextResponse.json({ error: 'guest_limit' }, { status: 429 }); + return problem({ + status: 429, + code: 'guest_limit', + detail: `This network has made today's ${GUEST_SHARES} guest shares. Sign in to make more.`, + retryable: false, + }); } } const refusal = read.mode === 'live' ? await liveShareRefusalFor(read) : null; if (refusal) { - return NextResponse.json(refusal.body, { status: refusal.status }); + return refusal; } const created = await createShare({ userId: user?.id ?? null, ...read }); diff --git a/src/app/api/source-doc/[...dcid]/route.ts b/src/app/api/source-doc/[...dcid]/route.ts index 9bc716f..b11e1fa 100644 --- a/src/app/api/source-doc/[...dcid]/route.ts +++ b/src/app/api/source-doc/[...dcid]/route.ts @@ -1,3 +1,4 @@ +import { missingProblem, unreachableProblem } from '@/lib/proxy/problem'; import { loadSourceDoc } from '@/lib/sources/graph'; // The graph's own words for one source (the same documentation door the @@ -15,11 +16,11 @@ export const GET = async ( // loadSourceDoc throws on a 5xx from the graph or a failed request. console.error('source-doc: the graph read failed', error); - return Response.json({ detail: 'The graph is unavailable' }, { status: 502 }); + return unreachableProblem('The graph is unavailable.', {}); } if (!doc) { - return Response.json({ detail: 'Not found' }, { status: 404 }); + return missingProblem('The graph holds no documentation for this source.'); } return Response.json({ diff --git a/src/app/api/v1/studio/[...path]/route.ts b/src/app/api/v1/studio/[...path]/route.ts index dce4f09..d1e2ad5 100644 --- a/src/app/api/v1/studio/[...path]/route.ts +++ b/src/app/api/v1/studio/[...path]/route.ts @@ -3,6 +3,7 @@ import { NextRequest } from 'next/server'; import { getUser } from '@/lib/auth/session'; import { counterDb } from '@/lib/db'; import { clientIp, forward, STUDIO_API_BASE } from '@/lib/proxy/forward'; +import { notFoundProblem } from '@/lib/proxy/problem'; import { meterTurn, refundPrompt, @@ -29,10 +30,7 @@ const proxy = async (req: NextRequest, { params }: { params: Promise<{ path: str const route = studioRoute((await params).path); if (!route) { - return Response.json( - { error: 'not_found', retry: 'do_not_retry', detail: 'There is no such Studio route.' }, - { status: 404 }, - ); + return notFoundProblem(req.nextUrl.pathname); } const options = { diff --git a/src/app/api/version-download/[spec]/[cv]/route.ts b/src/app/api/version-download/[spec]/[cv]/route.ts index 6979dbb..bb477c2 100644 --- a/src/app/api/version-download/[spec]/[cv]/route.ts +++ b/src/app/api/version-download/[spec]/[cv]/route.ts @@ -1,14 +1,21 @@ import type { NextRequest } from 'next/server'; +import { + missingProblem, + rateLimitedProblem, + requestInvalidProblem, + unreachableProblem, +} from '@/lib/proxy/problem'; import { downloadAllowance, readVersionFile } from '@/lib/search/storedFile'; import { SPEC_SHAPE, VERSION_SHAPE } from '@/lib/search/versionLink'; // GET /api/version-download// → 302 to the version's stored file. // A bundle prints this path, which does not change; the route reads the // manifest at click time and redirects to the stable link of the file -// `storedDownload` picks (readVersionFile). 404 -// when the API serves no such version or the version lists no file to -// offer; 429 past the `api` allowance. +// `storedDownload` picks (readVersionFile). Refusals are problem documents +// (src/lib/proxy/problem): 404 when the API serves no such version or the +// version lists no file to offer; 429 past the `api` allowance, with the +// allowance headers and Retry-After; 502 when the version read fails. export const GET = async ( req: NextRequest, { params }: { params: Promise<{ spec: string; cv: string }> }, @@ -16,22 +23,21 @@ export const GET = async ( const allowance = await downloadAllowance(req); if (!allowance.allowed) { - return Response.json( - { detail: 'Too many requests' }, - { status: 429, headers: allowance.headers }, - ); + return rateLimitedProblem(allowance.verdict, allowance.headers); } const { spec, cv } = await params; if (!SPEC_SHAPE.test(spec) || !VERSION_SHAPE.test(cv)) { - return Response.json({ detail: 'Bad ref' }, { status: 400 }); + return requestInvalidProblem('The spec or content version is not a valid reference.'); } const file = await readVersionFile({ specId: spec, contentVersion: cv }); if ('status' in file) { - return Response.json({ detail: file.message }, { status: file.status }); + return file.status === 404 + ? missingProblem(file.message) + : unreachableProblem(file.message, {}); } return new Response(null, { diff --git a/src/app/codes/[code]/route.ts b/src/app/codes/[code]/route.ts index 6ef7db7..830a578 100644 --- a/src/app/codes/[code]/route.ts +++ b/src/app/codes/[code]/route.ts @@ -2,6 +2,7 @@ import { NextRequest } from 'next/server'; import { codeTarget } from '@/lib/proxy/codes'; import { forward } from '@/lib/proxy/forward'; +import { notFoundProblem } from '@/lib/proxy/problem'; import { searchApiBase } from '@/lib/searchApi'; // The API's refusal-code registry (GET /codes/{code}), served at the same @@ -12,10 +13,7 @@ export const GET = async (req: NextRequest, { params }: { params: Promise<{ code const target = codeTarget(searchApiBase(), code, req.nextUrl.search); if (!target) { - return Response.json( - { error: 'not_found', retry: 'do_not_retry', detail: 'There is no such refusal code.' }, - { status: 404 }, - ); + return notFoundProblem(req.nextUrl.pathname); } return forward(req, { diff --git a/src/lib/chat/shareMode.ts b/src/lib/chat/shareMode.ts index 2e0ae2f..abf4a98 100644 --- a/src/lib/chat/shareMode.ts +++ b/src/lib/chat/shareMode.ts @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/no-explicit-any -- request bodies are validated field by field below */ import type { SavedConversation } from './history.ts'; +import { problem, requestInvalidProblem } from '../proxy/problem.ts'; import type { ShareKind } from './sharePrune.ts'; /** @@ -48,15 +49,13 @@ export type StudioShareRequest = { cardId?: string; }; -export type StudioShareRefusal = 'bad_request' | 'invalid_mode' | 'live_unsupported_kind'; - /** - * A Studio share request (`POST /api/share`), or the code it is refused - * with. A request that names no mode asks for a snapshot. + * A Studio share request (`POST /api/share`), or the 400 problem document it + * is refused with: `request_invalid`, with a `pointer` to the mode when that + * is the fault, or `live_unsupported_kind`. A request that names no mode + * asks for a snapshot. */ -export const readStudioShareRequest = ( - body: any, -): StudioShareRequest | { error: StudioShareRefusal } => { +export const readStudioShareRequest = (body: any): StudioShareRequest | Response => { const saved = body?.conversation; if ( @@ -67,17 +66,24 @@ export const readStudioShareRequest = ( !Array.isArray(saved.widgets) || (body.kind === 'card' && typeof body.cardId !== 'string') ) { - return { error: 'bad_request' }; + return requestInvalidProblem( + 'The body needs a kind, a conversation with an id and widgets, and a cardId for a card.', + ); } const mode: unknown = body.mode ?? 'snapshot'; if (!(MODES as unknown[]).includes(mode)) { - return { error: 'invalid_mode' }; + return requestInvalidProblem('The mode is `live` or `snapshot`.', '/mode'); } if (mode === 'live' && !LIVE_KINDS.includes(body.kind)) { - return { error: 'live_unsupported_kind' }; + return problem({ + status: 400, + code: 'live_unsupported_kind', + detail: 'Only a canvas or a card can be a live share.', + retryable: false, + }); } return { kind: body.kind, mode: mode as ShareMode, saved, cardId: body.cardId }; diff --git a/src/lib/chat/shareStore.ts b/src/lib/chat/shareStore.ts index 569feb1..00a4f87 100644 --- a/src/lib/chat/shareStore.ts +++ b/src/lib/chat/shareStore.ts @@ -12,7 +12,7 @@ import { import { apisFetch } from '@/lib/apisAuth'; import { counterDb, db } from '@/lib/db'; -import { liveShareRefusal, type LiveShareRefusal } from '@/lib/studio/liveShare'; +import { liveShareRefusal } from '@/lib/studio/liveShare'; import { snapshotLiveShare } from '@/lib/studio/liveShareResults'; import { share } from '../../../db/schema'; @@ -51,7 +51,7 @@ export const liveShareRefusalFor = ({ saved, kind, cardId, -}: StudioShareRequest): Promise => +}: StudioShareRequest): Promise => liveShareRefusal({ env: process.env, fetch: apisFetch }, pruneForShare(saved, kind, cardId)); export const getShare = async (id: string) => { diff --git a/src/lib/chat/test/shareMode.test.ts b/src/lib/chat/test/shareMode.test.ts index dca6cd0..f417ff3 100644 --- a/src/lib/chat/test/shareMode.test.ts +++ b/src/lib/chat/test/shareMode.test.ts @@ -67,25 +67,39 @@ test('readStudioShareRequest: a request that names no mode asks for a snapshot', } }); -test('readStudioShareRequest: a live conversation share is refused', () => { - assert.deepEqual(readStudioShareRequest({ kind: 'conversation', mode: 'live', conversation }), { - error: 'live_unsupported_kind', - }); +// The refusal a request came to: its status and problem members. +const refusalOf = async (read: unknown) => { + assert.ok(read instanceof Response, 'a refusal is a problem document'); + assert.equal(read.headers.get('content-type'), 'application/problem+json'); + + const { status, code, pointer, retryable } = await read.json(); + + return { status, code, pointer, retryable }; +}; + +test('readStudioShareRequest: a live conversation share is refused', async () => { + assert.deepEqual( + await refusalOf( + readStudioShareRequest({ kind: 'conversation', mode: 'live', conversation }), + ), + { status: 400, code: 'live_unsupported_kind', pointer: undefined, retryable: false }, + ); assert.deepEqual( readStudioShareRequest({ kind: 'conversation', mode: 'snapshot', conversation }), { kind: 'conversation', mode: 'snapshot', saved: conversation, cardId: undefined }, ); }); -test('readStudioShareRequest: an unknown mode is refused', () => { +test('readStudioShareRequest: an unknown mode is refused, pointing at the mode', async () => { for (const mode of ['frozen', '', 1, true, ['live'], { mode: 'live' }]) { - assert.deepEqual(readStudioShareRequest({ kind: 'canvas', mode, conversation }), { - error: 'invalid_mode', - }); + assert.deepEqual( + await refusalOf(readStudioShareRequest({ kind: 'canvas', mode, conversation })), + { status: 400, code: 'request_invalid', pointer: '/mode', retryable: false }, + ); } }); -test('readStudioShareRequest: a request without a kind, a conversation, or a card id is refused', () => { +test('readStudioShareRequest: a request without a kind, a conversation, or a card id is refused', async () => { const refused = [ null, {}, @@ -98,7 +112,12 @@ test('readStudioShareRequest: a request without a kind, a conversation, or a car ]; for (const body of refused) { - assert.deepEqual(readStudioShareRequest(body), { error: 'bad_request' }); + assert.deepEqual(await refusalOf(readStudioShareRequest(body)), { + status: 400, + code: 'request_invalid', + pointer: undefined, + retryable: false, + }); } }); diff --git a/src/lib/proxy/forward.ts b/src/lib/proxy/forward.ts index 52a9638..caac5e2 100644 --- a/src/lib/proxy/forward.ts +++ b/src/lib/proxy/forward.ts @@ -7,6 +7,7 @@ import { apisFetch } from '@/lib/apisAuth'; import { limiterKeyFor, setCaller, trustedHops } from './caller'; import { relayBody } from './compress'; import { clientResponseHeaders, dropEncodedLength, pick, upstreamRequestHeaders } from './headers'; +import { rateLimitedProblem, unreachableProblem } from './problem'; import { rateHeaders } from './rateHeaders'; import { hit, type Surface } from './rateLimit'; @@ -80,28 +81,12 @@ type ForwardOptions = { rewrite?: (upstream: Response) => Promise; }; -const unreachable = (detail: string, headers: Record): Response => - Response.json( - { error: 'transient_unavailable', retry: 'retry_after_delay', detail }, - { status: 502, headers }, - ); - export const forward = async (req: NextRequest, options: ForwardOptions): Promise => { const verdict = options.surface ? await hit(options.surface, clientIp(req)) : undefined; const limitHeaders = verdict ? rateHeaders(verdict) : {}; if (verdict && !verdict.allowed) { - return Response.json( - { - error: 'rate_limited', - retry: 'retry_after_delay', - detail: `Too many requests — ${verdict.limit} per window per address. Try again in ${verdict.reset}s.`, - }, - { - status: 429, - headers: { ...limitHeaders, 'Retry-After': String(verdict.reset) }, - }, - ); + return rateLimitedProblem(verdict, limitHeaders); } const refusal = await options.admit?.(); @@ -131,7 +116,7 @@ export const forward = async (req: NextRequest, options: ForwardOptions): Promis ); } - return unreachable(options.unreachable, limitHeaders); + return unreachableProblem(options.unreachable, limitHeaders); }; const hasBody = req.method !== 'GET' && req.method !== 'HEAD'; @@ -151,7 +136,7 @@ export const forward = async (req: NextRequest, options: ForwardOptions): Promis signal: req.signal, } as RequestInit); } catch { - return unreachable(options.unreachable, limitHeaders); + return unreachableProblem(options.unreachable, limitHeaders); } if (options.rewrite) { @@ -192,7 +177,8 @@ const ASSET_RESPONSE_HEADERS = [ /** * GET a public static asset from `target` and return it with its own type * and cache headers. No token, no caller and no limit, since the asset is - * public upstream too. Gives up after ASSET_TIMEOUT_MS with a 502. + * public upstream too. Gives up after ASSET_TIMEOUT_MS with a 502 problem + * document. */ export const forwardAsset = async (req: NextRequest, target: string): Promise => { const headers = new Headers(); @@ -211,7 +197,7 @@ export const forwardAsset = async (req: NextRequest, target: string): Promise { +const TITLES = { + 400: 'Bad Request', + 401: 'Unauthorized', + 403: 'Forbidden', + 404: 'Not Found', + 413: 'Content Too Large', + 422: 'Unprocessable Content', + 429: 'Too Many Requests', + 502: 'Bad Gateway', + 503: 'Service Unavailable', +} as const; + +export type ProblemStatus = keyof typeof TITLES; + +/** + * The codes the API's refusal registry or the client contract's + * `proxy-codes.json` holds, so `/codes/` resolves to a page. Any other + * code the site refuses with is its own, and its `type` is `about:blank`. + */ +const REGISTRY_CODES = [ + 'request_invalid', + 'not_found', + 'rate_limited', + 'transient_unavailable', +] as const; + +const registryCodes: ReadonlySet = new Set(REGISTRY_CODES); + +type ProblemInit = { + status: ProblemStatus; + code: string; + detail: string; + retryable: boolean; + // Sent with the document, such as Retry-After or the X-RateLimit-* set. + headers?: Record; + // Members the code defines beyond the common set. They never replace one + // of the common members. + extra?: Record; +}; + +/** A problem document for `code`, with a fresh trace id. */ +export const problem = ({ + status, + code, + detail, + retryable, + headers, + extra, +}: ProblemInit): Response => { const traceId = crypto.randomUUID().replaceAll('-', ''); return Response.json( { - type: '/codes/not_found', - title: 'Not Found', - status: 404, - detail: `There is no route ${path}.`, - code: 'not_found', + ...extra, + type: registryCodes.has(code) ? `/codes/${code}` : 'about:blank', + title: TITLES[status], + status, + detail, + code, trace_id: traceId, - retryable: false, + retryable, }, { - status: 404, + status, headers: { + 'Cache-Control': 'no-cache', + ...headers, 'Content-Type': 'application/problem+json', 'x-dfo-trace-id': traceId, - 'Cache-Control': 'no-cache', }, }, ); }; + +/** A 404 `not_found` problem document that says what is missing. */ +export const missingProblem = (detail: string): Response => + problem({ status: 404, code: 'not_found', detail, retryable: false }); + +/** A 404 problem document for the route `path`. */ +export const notFoundProblem = (path: string): Response => + missingProblem(`There is no route ${path}.`); + +/** + * A 400 `request_invalid` problem document. `pointer` names the invalid + * member of the request body, such as `/mode`, when one is at fault. + */ +export const requestInvalidProblem = (detail: string, pointer?: string): Response => + problem({ + status: 400, + code: 'request_invalid', + detail, + retryable: false, + extra: pointer ? { pointer } : undefined, + }); + +/** A 401 `unauthorized` problem document: the call needs a signed-in account. */ +export const unauthorizedProblem = (): Response => + problem({ + status: 401, + code: 'unauthorized', + detail: 'Sign in to make this request.', + retryable: false, + }); + +/** A 429 `rate_limited` problem document; `headers` carry the rate headers. */ +export const rateLimitedProblem = ( + { limit, reset }: { limit: number; reset: number }, + headers: Record, +): Response => + problem({ + status: 429, + code: 'rate_limited', + detail: `Too many requests: ${limit} per window per address. Try again in ${reset}s.`, + retryable: true, + headers: { ...headers, 'Retry-After': String(reset) }, + }); + +/** A 502 `transient_unavailable` problem document: the upstream did not answer. */ +export const unreachableProblem = (detail: string, headers: Record): Response => + problem({ status: 502, code: 'transient_unavailable', detail, retryable: true, headers }); diff --git a/src/lib/proxy/test/problem.test.ts b/src/lib/proxy/test/problem.test.ts index c91467d..45987e4 100644 --- a/src/lib/proxy/test/problem.test.ts +++ b/src/lib/proxy/test/problem.test.ts @@ -1,10 +1,18 @@ /** - * `problem.ts`: the problem document for a route the proxy does not have. + * `problem.ts`: the problem documents the proxy answers with when it refuses a call itself. */ import assert from 'node:assert/strict'; import test from 'node:test'; -import { notFoundProblem } from '../problem.ts'; +import { + missingProblem, + notFoundProblem, + problem, + rateLimitedProblem, + requestInvalidProblem, + unauthorizedProblem, + unreachableProblem, +} from '../problem.ts'; test('notFoundProblem: answers 404 with a not_found problem document and a trace id', async () => { const res = notFoundProblem('/api/v1/nowhere'); @@ -25,3 +33,132 @@ test('notFoundProblem: each answer has its own trace id', () => { assert.notEqual(a.headers.get('x-dfo-trace-id'), b.headers.get('x-dfo-trace-id')); }); + +test('rateLimitedProblem: a retryable 429 problem that keeps the rate headers and Retry-After', async () => { + const res = rateLimitedProblem({ limit: 60, reset: 7 }, { 'X-RateLimit-Limit': '60' }); + const body = await res.json(); + + assert.equal(res.status, 429); + assert.equal(res.headers.get('content-type'), 'application/problem+json'); + assert.equal(res.headers.get('retry-after'), '7'); + assert.equal(res.headers.get('x-ratelimit-limit'), '60'); + assert.equal(body.code, 'rate_limited'); + assert.equal(body.status, 429); + assert.equal(body.retryable, true); + assert.equal(res.headers.get('x-dfo-trace-id'), body.trace_id); +}); + +test('unreachableProblem: a retryable 502 transient_unavailable problem with the detail given', async () => { + const res = unreachableProblem('The studio agent is unreachable.', { + 'X-RateLimit-Limit': '60', + }); + const body = await res.json(); + + assert.equal(res.status, 502); + assert.equal(res.headers.get('content-type'), 'application/problem+json'); + assert.equal(res.headers.get('x-ratelimit-limit'), '60'); + assert.equal(body.code, 'transient_unavailable'); + assert.equal(body.detail, 'The studio agent is unreachable.'); + assert.equal(body.retryable, true); +}); + +test('problem: titles the statuses the site refuses with and spreads the extra members', async () => { + for (const [status, title] of [ + [400, 'Bad Request'], + [401, 'Unauthorized'], + [403, 'Forbidden'], + [413, 'Content Too Large'], + [422, 'Unprocessable Content'], + [503, 'Service Unavailable'], + ] as const) { + const res = problem({ + status, + code: 'live_too_many_cards', + detail: 'Too many.', + retryable: false, + extra: { limit: 20 }, + }); + const body = await res.json(); + + assert.equal(res.status, status); + assert.equal(body.title, title); + assert.equal(body.limit, 20); + assert.equal(body.type, 'about:blank'); + } +}); + +test('problem: type links the registry codes and is about:blank for any other code', async () => { + for (const [code, type] of [ + ['request_invalid', '/codes/request_invalid'], + ['not_found', '/codes/not_found'], + ['rate_limited', '/codes/rate_limited'], + ['transient_unavailable', '/codes/transient_unavailable'], + ['unauthorized', 'about:blank'], + ['platform_refused', 'about:blank'], + ] as const) { + const body = await problem({ + status: 400, + code, + detail: 'Refused.', + retryable: false, + }).json(); + + assert.equal(body.type, type, code); + } +}); + +test('problem: an extra member cannot replace a common member', async () => { + const res = problem({ + status: 400, + code: 'request_invalid', + detail: 'Bad.', + retryable: false, + extra: { + code: 'other', + status: 200, + type: 'x', + title: 'x', + detail: 'x', + retryable: true, + trace_id: 'x', + limit: 3, + }, + }); + const body = await res.json(); + + assert.deepEqual( + { code: body.code, status: body.status, type: body.type, title: body.title }, + { + code: 'request_invalid', + status: 400, + type: '/codes/request_invalid', + title: 'Bad Request', + }, + ); + assert.equal(body.detail, 'Bad.'); + assert.equal(body.retryable, false); + assert.equal(body.trace_id, res.headers.get('x-dfo-trace-id')); + assert.equal(body.limit, 3); +}); + +test('missingProblem, requestInvalidProblem, unauthorizedProblem: the registry codes with their statuses', async () => { + for (const [res, status, code] of [ + [missingProblem('There is no such share.'), 404, 'not_found'], + [requestInvalidProblem('Bad body.'), 400, 'request_invalid'], + [unauthorizedProblem(), 401, 'unauthorized'], + ] as const) { + const body = await res.json(); + + assert.equal(res.status, status); + assert.equal(body.code, code); + assert.equal(body.retryable, false); + assert.equal(body.pointer, undefined); + } +}); + +test('requestInvalidProblem: names the invalid member of the body when given one', async () => { + const body = await requestInvalidProblem('The mode is wrong.', '/mode').json(); + + assert.equal(body.pointer, '/mode'); + assert.equal(body.detail, 'The mode is wrong.'); +}); diff --git a/src/lib/search/readings.ts b/src/lib/search/readings.ts index f73bf91..20a4438 100644 --- a/src/lib/search/readings.ts +++ b/src/lib/search/readings.ts @@ -32,6 +32,7 @@ const BASIS_NOTES: Record = { nominated: 'Another reading of your words', twin: 'An equivalent series that holds data', direction: 'The same measure, read in the other direction', + breakdown: 'The same measure, broken down another way', }; /** The line a reading card leads with, saying why the part offers it. diff --git a/src/lib/search/searchClient.ts b/src/lib/search/searchClient.ts index f456dc9..dce0935 100644 --- a/src/lib/search/searchClient.ts +++ b/src/lib/search/searchClient.ts @@ -1240,8 +1240,9 @@ export type OpenedSearchShare = { }; /** Mints a share link for the definite parts on screen: its id, or the - * platform's refusal code (`over_ceiling`, …) — `null` code when the - * request itself failed. */ + * platform's refusal code (`over_ceiling`, …), read from the + * `platform_code` of a `platform_refused` problem document — `null` code + * for any other failure of the request. */ export async function createSearchShare(req: { mode: SearchShareMode; query: string; @@ -1263,9 +1264,17 @@ export async function createSearchShare(req: { }); const body = await res.json().catch(() => null); - return res.ok && typeof body?.id === 'string' - ? { id: body.id } - : { refused: typeof body?.detail === 'string' ? body.detail : null }; + if (res.ok && typeof body?.id === 'string') { + return { id: body.id }; + } + + // Only a `platform_refused` problem carries the platform's code. + return { + refused: + body?.code === 'platform_refused' && typeof body.platform_code === 'string' + ? body.platform_code + : null, + }; } catch { return { refused: null }; } diff --git a/src/lib/search/snapshot.ts b/src/lib/search/snapshot.ts index 892e4af..545b69a 100644 --- a/src/lib/search/snapshot.ts +++ b/src/lib/search/snapshot.ts @@ -68,12 +68,13 @@ export const renderingArtifact = < ); /** The pin failed or cannot be attempted, so the snapshot would expire with - * the working tier. POST /api/share answers this code as its `detail`. */ + * the working tier. POST /api/share answers this code as the `platform_code` + * of its `platform_refused` problem. */ export const PIN_REFUSED = 'pin_refused'; /** The pin door is pinning nothing right now (429 or 503) because its daily * allowance is spent or it is busy. POST /api/share answers this code as - * its `detail`. */ + * the `platform_code` of its `platform_refused` problem. */ export const PIN_UNAVAILABLE = 'pin_unavailable'; const UNAVAILABLE_STATUSES = new Set([429, 503]); diff --git a/src/lib/search/storedFile.ts b/src/lib/search/storedFile.ts index ea806cb..a59e809 100644 --- a/src/lib/search/storedFile.ts +++ b/src/lib/search/storedFile.ts @@ -3,7 +3,7 @@ import 'server-only'; import type { NextRequest } from 'next/server'; import { clientIp } from '@/lib/proxy/forward'; -import { rateHeaders } from '@/lib/proxy/rateHeaders'; +import { rateHeaders, type RateVerdict } from '@/lib/proxy/rateHeaders'; import { hit } from '@/lib/proxy/rateLimit'; import { datasetsFetch } from '@/lib/search/datasetsFetch'; import { artifactHref } from '@/lib/search/searchClient'; @@ -28,11 +28,12 @@ const VERSION_READ_TIMEOUT_MS = 20_000; */ export const downloadAllowance = async ( req: NextRequest, -): Promise<{ allowed: boolean; headers: Record }> => { +): Promise<{ allowed: boolean; verdict: RateVerdict; headers: Record }> => { const verdict = await hit('api', clientIp(req)); return { allowed: verdict.allowed, + verdict, headers: verdict.allowed ? rateHeaders(verdict) : { ...rateHeaders(verdict), 'Retry-After': String(verdict.reset) }, diff --git a/src/lib/search/test/searchClient.test.ts b/src/lib/search/test/searchClient.test.ts index 5d41aa5..e0714e5 100644 --- a/src/lib/search/test/searchClient.test.ts +++ b/src/lib/search/test/searchClient.test.ts @@ -4,6 +4,7 @@ import { applyDoneEvent, artifactHref, buildRequest, + createSearchShare, createSuggestionSlot, entityOptions, fetchDatasetOptions, @@ -1250,6 +1251,53 @@ test('pinDownloadedVersion posts the downloaded version to the site pin route an }); }); +for (const [name, status, document, refused] of [ + [ + 'a platform_refused problem gives the platform code', + 502, + { + code: 'platform_refused', + detail: 'The data platform refused.', + platform_code: 'over_ceiling', + }, + 'over_ceiling', + ], + [ + 'a request_invalid problem with a detail gives no code', + 400, + { code: 'request_invalid', detail: 'The request is not a search share.' }, + null, + ], + [ + 'a payload_too_large problem with a detail gives no code', + 413, + { code: 'payload_too_large', detail: 'The request body is over 1 bytes.' }, + null, + ], + [ + 'a platform_code on another code is not read', + 502, + { code: 'transient_unavailable', platform_code: 'over_ceiling' }, + null, + ], +] as const) { + test(`createSearchShare: ${name}`, async (t) => { + t.mock.method( + globalThis, + 'fetch', + async () => + new Response(JSON.stringify(document), { + status, + headers: { 'content-type': 'application/problem+json' }, + }), + ); + + assert.deepEqual(await createSearchShare({ mode: 'snapshot', query: 'q', parts: [] }), { + refused, + }); + }); +} + test('artifactHref prefixes the Datasets API origin onto a root-relative artifact link', () => { const url = '/api/datasets/spec_a1/cv_b2/files/sha256:abc'; diff --git a/src/lib/search/view.ts b/src/lib/search/view.ts index 9de15d2..940dfbc 100644 --- a/src/lib/search/view.ts +++ b/src/lib/search/view.ts @@ -2406,6 +2406,8 @@ const NO_DATA_REASON_TEXT: Record, string> = { 'The matching series disagree about who gives and receives, so no single answer fits.', qualifier_not_bound: 'A category or condition in that query could not be applied to the matching series.', + timed_out: + 'The search ran out of time before it answered this. Ask again, or ask a narrower question.', }; export const noDataReasonText = (reason?: NoDataReason | null): string => diff --git a/src/lib/studio/cardDownload.ts b/src/lib/studio/cardDownload.ts index 9f500c3..45e107b 100644 --- a/src/lib/studio/cardDownload.ts +++ b/src/lib/studio/cardDownload.ts @@ -136,19 +136,19 @@ const wholeSeconds = (value: unknown): number | null => { : null; }; -// The share route's answer when it serves no file is a 503 while a refresh -// is in progress (the wait in `Retry-After` and the body's `retry_after_s`), -// and otherwise the refusal's `code`, or `error` on its own 404. +// The share route's answer when it serves no file is a problem document: 503 +// `refresh_pending` while a refresh is in progress (the wait in `Retry-After` +// and the document's `retry_after_s`), and otherwise its `code`. const liveRefusal = (res: Response, body: Record | null): CardDownloadError => { - if (res.status === 503 && body?.['status'] === 'pending') { + const code = body?.['code']; + + if (res.status === 503 && code === DOWNLOAD_PENDING) { return new CardDownloadError( DOWNLOAD_PENDING, - wholeSeconds(res.headers.get('retry-after')) ?? wholeSeconds(body['retry_after_s']), + wholeSeconds(res.headers.get('retry-after')) ?? wholeSeconds(body?.['retry_after_s']), ); } - const code = body?.['code'] ?? body?.['error']; - return new CardDownloadError(typeof code === 'string' ? code : null); }; diff --git a/src/lib/studio/liveCards.ts b/src/lib/studio/liveCards.ts index 79af9a3..67d0922 100644 --- a/src/lib/studio/liveCards.ts +++ b/src/lib/studio/liveCards.ts @@ -311,9 +311,12 @@ const readAnswer = (ok: boolean, body: unknown): Answer => { } if (!ok) { - const { error } = body; + const { code, retryable } = body; - return failure(typeof error === 'string' && error ? error : LIVE_CLIENT_CODES.failed); + return { + ...failure(typeof code === 'string' && code ? code : LIVE_CLIENT_CODES.failed), + ...(retryable === true ? { retryable: true } : {}), + }; } const { status, card, code } = body; diff --git a/src/lib/studio/liveShare.ts b/src/lib/studio/liveShare.ts index ac9d4d4..c8f4d8f 100644 --- a/src/lib/studio/liveShare.ts +++ b/src/lib/studio/liveShare.ts @@ -6,6 +6,7 @@ import { isLiveStudioShare } from '../chat/shareMode.ts'; import type { ShareDb } from '../chat/shareWrites.ts'; import type { CardRef, Widget } from '../chat/widgets.ts'; import { dropEncodedLength, pick } from '../proxy/headers.ts'; +import { problem, type ProblemStatus } from '../proxy/problem.ts'; import { gateOf } from './gate.ts'; import { BackendError, @@ -106,12 +107,6 @@ export type BlockedCard = { reason: 'missing_ref' | 'unknown_component' | 'unsupported_frame'; }; -export type LiveShareRefusal = - | { status: 503; body: { error: 'live_unavailable' } } - | { status: 400; body: { error: 'live_too_many_cards'; limit: number; count: number } } - | { status: 400; body: { error: 'live_cards_blocked'; cards: BlockedCard[] } } - | { status: 502; body: { error: 'live_check_failed' } }; - const blockOf = async (backend: LiveBackend, widget: Widget): Promise => { const ref = refOf(widget); @@ -136,32 +131,38 @@ const blockOf = async (backend: LiveBackend, widget: Widget): Promise, saved: SavedConversation, -): Promise => { +): Promise => { const backend = liveBackendFrom(env, fetcher); if (!backend) { - return { status: 503, body: { error: 'live_unavailable' } }; + return problem({ + status: 503, + code: 'live_unavailable', + detail: 'This deployment cannot refresh live cards.', + retryable: false, + }); } const cards = dataCardsOf(saved); if (cards.length > LIVE_SHARE_MAX_DATA_CARDS) { - return { + return problem({ status: 400, - body: { - error: 'live_too_many_cards', - limit: LIVE_SHARE_MAX_DATA_CARDS, - count: cards.length, - }, - }; + code: 'live_too_many_cards', + detail: `A live share holds at most ${LIVE_SHARE_MAX_DATA_CARDS} data cards; this one holds ${cards.length}.`, + retryable: false, + extra: { limit: LIVE_SHARE_MAX_DATA_CARDS, count: cards.length }, + }); } const limited = bounded(backend, CREATE_CHECK_DEADLINE_MS); @@ -173,14 +174,25 @@ export const liveShareRefusal = async ( ).filter((block) => block !== null); return blocked.length - ? { status: 400, body: { error: 'live_cards_blocked', cards: blocked } } + ? problem({ + status: 400, + code: 'live_cards_blocked', + detail: 'Some data cards cannot be refreshed, so the share cannot be live.', + retryable: false, + extra: { cards: blocked }, + }) : null; } catch (error) { console.error('live share: card check failed', { error: error instanceof BackendError ? error.code : error, }); - return { status: 502, body: { error: 'live_check_failed' } }; + return problem({ + status: 502, + code: 'transient_unavailable', + detail: 'The data cards could not be checked right now. Try again in a minute.', + retryable: true, + }); } }; @@ -271,7 +283,13 @@ const NOT_CONFIGURED: LiveResult = { status: 'error', code: 'live_unavailable' } const NO_STORE = { 'Cache-Control': 'no-store' }; const notFound = (): Response => - Response.json({ error: 'not_found' }, { status: 404, headers: NO_STORE }); + problem({ + status: 404, + code: 'not_found', + detail: 'There is no such live card.', + retryable: false, + headers: NO_STORE, + }); export type RefreshAnswer = { status: LiveResult['status']; @@ -344,19 +362,34 @@ const EXPORT_HEADERS = ['content-type', 'content-disposition', 'content-length'] // The statuses with which dfo-apis refuses an export request itself: a // format it cannot write, or rows that are gone. -const REQUEST_REFUSALS = [400, 404, 422]; - -// A download that cannot be served answers JSON under an error status, so a -// browser does not save it as the file: 503 with Retry-After while the -// card's refresh is pending, dfo-apis' own status for a refusal of the -// export request (`REQUEST_REFUSALS`), and 502 for everything else. -const noFile = (result: LiveResult & { status: 'pending' | 'error' }, status = 502): Response => +const REQUEST_REFUSALS = [400, 404, 422] as const satisfies readonly ProblemStatus[]; + +// A download that cannot be served answers a problem document under an error +// status, so a browser does not save it as the file: 503 `refresh_pending` +// with Retry-After while the card's refresh is pending, dfo-apis' own status +// for a refusal of the export request (`REQUEST_REFUSALS`), and 502 for +// everything else. `retry_after_s` is an extension member of both. +const noFile = ( + result: LiveResult & { status: 'pending' | 'error' }, + status: ProblemStatus = 502, +): Response => result.status === 'pending' - ? Response.json(answerOf(result), { + ? problem({ status: 503, + code: 'refresh_pending', + detail: 'The card is being refreshed. Ask again after the wait.', + retryable: true, headers: { ...NO_STORE, 'Retry-After': String(result.retryAfterS) }, + extra: { retry_after_s: result.retryAfterS }, }) - : Response.json(answerOf(result), { status, headers: NO_STORE }); + : problem({ + status, + code: result.code, + detail: 'The card has no data file to serve.', + retryable: result.retryable === true, + headers: NO_STORE, + extra: result.retryAfterS ? { retry_after_s: result.retryAfterS } : undefined, + }); /** * `GET /api/share/[id]/cards/[cardId]/data?format=&bundle=`. The rows of diff --git a/src/lib/studio/shareDialog.ts b/src/lib/studio/shareDialog.ts index 9344de4..4e83431 100644 --- a/src/lib/studio/shareDialog.ts +++ b/src/lib/studio/shareDialog.ts @@ -104,23 +104,22 @@ export const readShareAnswer = (ok: boolean, body: unknown): ShareAnswer => { return typeof body['id'] === 'string' && body['id'] ? { id: body['id'] } : FAILED; } - const { error, cards, limit, count } = body; + // Every refusal is a problem document; its `code` says which. + const { code, cards, limit, count } = body; - if (error === 'live_cards_blocked' && Array.isArray(cards)) { + if (code === 'live_cards_blocked' && Array.isArray(cards)) { const cardIds = cards.flatMap((card) => isRecord(card) && typeof card['id'] === 'string' ? [card['id']] : [], ); - return cardIds.length ? { refusal: { code: error, cardIds } } : FAILED; + return cardIds.length ? { refusal: { code, cardIds } } : FAILED; } - if (error === 'live_too_many_cards' && typeof limit === 'number' && typeof count === 'number') { - return { refusal: { code: error, limit, count } }; + if (code === 'live_too_many_cards' && typeof limit === 'number' && typeof count === 'number') { + return { refusal: { code, limit, count } }; } - return error === 'live_unavailable' || error === 'guest_limit' - ? { refusal: { code: error } } - : FAILED; + return code === 'live_unavailable' || code === 'guest_limit' ? { refusal: { code } } : FAILED; }; /** diff --git a/src/lib/studio/spendPrompt.ts b/src/lib/studio/spendPrompt.ts index 5a5658a..a629c5b 100644 --- a/src/lib/studio/spendPrompt.ts +++ b/src/lib/studio/spendPrompt.ts @@ -8,6 +8,7 @@ import { type CounterDb, type Reservation, } from '../proxy/limitCounter.ts'; +import { problem } from '../proxy/problem.ts'; import { GUEST_PROMPTS, USER_DAILY_PROMPTS } from './promptAllowance.ts'; import type { ToolError } from './wire.ts'; @@ -124,22 +125,25 @@ export const admitTurn = async ( } catch (error) { console.error('studio: the prompt allowance could not be read', error); - const unavailable: ToolError = { - error: 'transient_unavailable', - retry: 'retry_after_delay', + return problem({ + status: 503, + code: 'transient_unavailable', detail: 'Studio cannot check your prompt allowance right now. Try again in a minute.', - }; - - return Response.json(unavailable, { status: 503 }); + retryable: true, + }); } finally { clearTimeout(timer); } return ( refusal && - Response.json(refusal, { + problem({ status: 429, + code: refusal.error, + detail: refusal.detail, + retryable: refusal.retry === 'retry_after_delay', headers: { 'Retry-After': String(refusal.retry_after_seconds) }, + extra: { retry_after_seconds: refusal.retry_after_seconds }, }) ); }; diff --git a/src/lib/studio/test/cardDownload.test.ts b/src/lib/studio/test/cardDownload.test.ts index f04b196..c8f084f 100644 --- a/src/lib/studio/test/cardDownload.test.ts +++ b/src/lib/studio/test/cardDownload.test.ts @@ -207,7 +207,7 @@ for (const [name, response, code, retryAfterS] of [ [ 'a refresh in progress, the wait in Retry-After', () => - new Response(JSON.stringify({ status: 'pending', retry_after_s: 6 }), { + new Response(JSON.stringify({ code: 'refresh_pending', retry_after_s: 6 }), { status: 503, headers: { 'retry-after': '7' }, }), @@ -217,14 +217,16 @@ for (const [name, response, code, retryAfterS] of [ [ 'a refresh in progress, the wait in the body alone', () => - new Response(JSON.stringify({ status: 'pending', retry_after_s: 6 }), { status: 503 }), + new Response(JSON.stringify({ code: 'refresh_pending', retry_after_s: 6 }), { + status: 503, + }), DOWNLOAD_PENDING, 6, ], [ 'a refusal of the format', () => - new Response(JSON.stringify({ status: 'error', code: 'request_invalid' }), { + new Response(JSON.stringify({ code: 'request_invalid', retryable: false }), { status: 400, }), 'request_invalid', @@ -233,7 +235,7 @@ for (const [name, response, code, retryAfterS] of [ [ 'a back end out of reach', () => - new Response(JSON.stringify({ status: 'error', code: 'backend_unreachable' }), { + new Response(JSON.stringify({ code: 'backend_unreachable', retryable: true }), { status: 502, }), 'backend_unreachable', @@ -241,7 +243,8 @@ for (const [name, response, code, retryAfterS] of [ ], [ 'a share that no longer holds the card', - () => new Response(JSON.stringify({ error: 'not_found' }), { status: 404 }), + () => + new Response(JSON.stringify({ code: 'not_found', retryable: false }), { status: 404 }), 'not_found', null, ], @@ -259,12 +262,12 @@ for (const [name, response, code, retryAfterS] of [ }); } -test('fetchCardData: off a live share, a 503 or an `error` key is not read as the share route', async (t) => { +test('fetchCardData: off a live share, the wait of a pending 503 is not read as the share route', async (t) => { t.mock.method( globalThis, 'fetch', async () => - new Response(JSON.stringify({ status: 'pending', error: 'not_found' }), { + new Response(JSON.stringify({ code: 'refresh_pending', retry_after_s: 6 }), { status: 503, headers: { 'retry-after': '7' }, }), @@ -273,7 +276,9 @@ test('fetchCardData: off a live share, a 503 or an `error` key is not read as th await assert.rejects( fetchCardData(REF, { format: 'csv', bundle: true }), (error: unknown) => - error instanceof CardDownloadError && error.code === null && error.retryAfterS === null, + error instanceof CardDownloadError && + error.code === 'refresh_pending' && + error.retryAfterS === null, ); }); diff --git a/src/lib/studio/test/liveCards.test.ts b/src/lib/studio/test/liveCards.test.ts index a3dbf40..72b8703 100644 --- a/src/lib/studio/test/liveCards.test.ts +++ b/src/lib/studio/test/liveCards.test.ts @@ -399,7 +399,8 @@ test('each card settles on its own answer, and one failing card leaves the rest current: [{ body: { status: 'current', card: CARD } }], redrawn: [{ body: { status: 'redrawn', card: CARD } }], refused: [{ body: { status: 'error', code: 'recipe_missing' } }], - gone: [{ status: 404, body: { error: 'not_found' } }], + gone: [{ status: 404, body: { code: 'not_found', retryable: false } }], + unreachable: [{ status: 502, body: { code: 'transient_unavailable', retryable: true } }], offline: [new TypeError('fetch failed')], garbled: [{ body: { status: 'current' } }], broken: [{ status: 500, body: null }], @@ -409,6 +410,7 @@ test('each card settles on its own answer, and one failing card leaves the rest 'redrawn', 'refused', 'gone', + 'unreachable', 'offline', 'garbled', 'broken', @@ -419,6 +421,7 @@ test('each card settles on its own answer, and one failing card leaves the rest redrawn: { status: 'ready', outcome: 'redrawn', card: CARD }, refused: { status: 'error', code: 'recipe_missing' }, gone: { status: 'error', code: 'not_found' }, + unreachable: { status: 'error', code: 'transient_unavailable', retryable: true }, offline: { status: 'error', code: 'network_error' }, garbled: { status: 'error', code: 'refresh_invalid' }, broken: { status: 'error', code: 'refresh_failed' }, diff --git a/src/lib/studio/test/liveShare.test.ts b/src/lib/studio/test/liveShare.test.ts index 2f4ff4a..592c1ed 100644 --- a/src/lib/studio/test/liveShare.test.ts +++ b/src/lib/studio/test/liveShare.test.ts @@ -83,6 +83,21 @@ const canvas = (widgets: Widget[], removedIds: string[] = []): SavedConversation layout: {}, }) as SavedConversation; +// A refusal as the share route sends it: a problem document, reduced to its +// status and the members a reader acts on. +const refusalOf = async (res: Response | null) => { + if (!res) { + return null; + } + + assert.equal(res.headers.get('content-type'), 'application/problem+json'); + + const body = await res.json(); + const members = ['code', 'retryable', 'cards', 'limit', 'count'].filter((key) => key in body); + + return { status: res.status, ...Object.fromEntries(members.map((key) => [key, body[key]])) }; +}; + test('refreshCard: a dataset card whose frame stands is the stored revision’s read', async () => { const { stub, backend } = backendOver({ components: { cmp_data: DATASET_FRAME }, @@ -336,10 +351,14 @@ test('a stored ref outside the service’s id form is a missing ref and reaches // Creating the share refuses the card before any read. assert.deepEqual( - await liveShareRefusal({ env: ENV, fetch: stub.fetch }, canvas([chart('c', ref)])), + await refusalOf( + await liveShareRefusal({ env: ENV, fetch: stub.fetch }, canvas([chart('c', ref)])), + ), { status: 400, - body: { error: 'live_cards_blocked', cards: [{ id: 'c', reason: 'missing_ref' }] }, + code: 'live_cards_blocked', + retryable: false, + cards: [{ id: 'c', reason: 'missing_ref' }], }, JSON.stringify(ref), ); @@ -408,17 +427,19 @@ test('liveShareRefusal: names every card that blocks a live link', async () => { chart('odd-frame', { component_id: 'cmp_odd', revision: 1 }), ]); - assert.deepEqual(await liveShareRefusal({ env: ENV, fetch: stub.fetch }, saved), { - status: 400, - body: { - error: 'live_cards_blocked', + assert.deepEqual( + await refusalOf(await liveShareRefusal({ env: ENV, fetch: stub.fetch }, saved)), + { + status: 400, + code: 'live_cards_blocked', + retryable: false, cards: [ { id: 'no-ref', reason: 'missing_ref' }, { id: 'unknown', reason: 'unknown_component' }, { id: 'odd-frame', reason: 'unsupported_frame' }, ], }, - }); + ); }); test('liveShareRefusal: holds at most four component reads in flight', async () => { @@ -453,10 +474,16 @@ test('liveShareRefusal: more data cards than the cap, before any read', async () const count = LIVE_SHARE_MAX_DATA_CARDS + 1; const cards = Array.from({ length: count }, (_, index) => chart(`card-${index}`, REF)); - assert.deepEqual(await liveShareRefusal({ env: ENV, fetch: stub.fetch }, canvas(cards)), { - status: 400, - body: { error: 'live_too_many_cards', limit: LIVE_SHARE_MAX_DATA_CARDS, count }, - }); + assert.deepEqual( + await refusalOf(await liveShareRefusal({ env: ENV, fetch: stub.fetch }, canvas(cards))), + { + status: 400, + code: 'live_too_many_cards', + retryable: false, + limit: LIVE_SHARE_MAX_DATA_CARDS, + count, + }, + ); assert.equal(stub.calls.length, 0); assert.equal( await liveShareRefusal({ env: ENV, fetch: stub.fetch }, canvas(cards.slice(1))), @@ -492,13 +519,15 @@ test('liveShareRefusal: counts and checks only the cards the canvas shows', asyn pinnedIds: ['pinned-no-ref'], }; - assert.deepEqual(await liveShareRefusal({ env: ENV, fetch: stub.fetch }, withPinned), { - status: 400, - body: { - error: 'live_cards_blocked', + assert.deepEqual( + await refusalOf(await liveShareRefusal({ env: ENV, fetch: stub.fetch }, withPinned)), + { + status: 400, + code: 'live_cards_blocked', + retryable: false, cards: [{ id: 'pinned-no-ref', reason: 'missing_ref' }], }, - }); + ); }); test('liveShareRefusal: no credential, or a back end that cannot answer', async () => { @@ -506,11 +535,13 @@ test('liveShareRefusal: no credential, or a back end that cannot answer', async const saved = canvas([chart('a', REF)]); assert.deepEqual( - await liveShareRefusal( - { env: { ...ENV, LIVE_REFRESH_PRINCIPAL: '' }, fetch: stub.fetch }, - saved, + await refusalOf( + await liveShareRefusal( + { env: { ...ENV, LIVE_REFRESH_PRINCIPAL: '' }, fetch: stub.fetch }, + saved, + ), ), - { status: 503, body: { error: 'live_unavailable' } }, + { status: 503, code: 'live_unavailable', retryable: false }, ); assert.equal(stub.calls.length, 0); @@ -518,9 +549,10 @@ test('liveShareRefusal: no credential, or a back end that cannot answer', async throw new TypeError('fetch failed'); }; - assert.deepEqual(await liveShareRefusal({ env: ENV, fetch: down }, saved), { + assert.deepEqual(await refusalOf(await liveShareRefusal({ env: ENV, fetch: down }, saved)), { status: 502, - body: { error: 'live_check_failed' }, + code: 'transient_unavailable', + retryable: true, }); }); diff --git a/src/lib/studio/test/shareDialog.test.ts b/src/lib/studio/test/shareDialog.test.ts index 15bb1cc..97c104a 100644 --- a/src/lib/studio/test/shareDialog.test.ts +++ b/src/lib/studio/test/shareDialog.test.ts @@ -128,7 +128,8 @@ test("the site's answer to a new share is its id or the refusal it names", () => [ false, { - error: 'live_cards_blocked', + code: 'live_cards_blocked', + retryable: false, cards: [ { id: 'cmp_b', reason: 'missing_ref' }, { id: 'cmp_c', reason: 'unsupported_frame' }, @@ -138,15 +139,21 @@ test("the site's answer to a new share is its id or the refusal it names", () => ], [ false, - { error: 'live_too_many_cards', limit: 20, count: 23 }, + { code: 'live_too_many_cards', retryable: false, limit: 20, count: 23 }, { refusal: { code: 'live_too_many_cards', limit: 20, count: 23 } }, ], - [false, { error: 'live_unavailable' }, { refusal: { code: 'live_unavailable' } }], - [false, { error: 'guest_limit' }, { refusal: { code: 'guest_limit' } }], + [false, { code: 'live_unavailable' }, { refusal: { code: 'live_unavailable' } }], + [false, { code: 'guest_limit' }, { refusal: { code: 'guest_limit' } }], // What a second try can outlive, and every answer outside the contract. - [false, { error: 'live_check_failed' }, { refusal: { code: 'failed' } }], - [false, { error: 'unauthorized' }, { refusal: { code: 'failed' } }], - [false, { error: 'live_cards_blocked', cards: [] }, { refusal: { code: 'failed' } }], + [ + false, + { code: 'transient_unavailable', retryable: true }, + { refusal: { code: 'failed' } }, + ], + [false, { code: 'unauthorized' }, { refusal: { code: 'failed' } }], + // The old `{ error }` body is no longer read. + [false, { error: 'guest_limit' }, { refusal: { code: 'failed' } }], + [false, { code: 'live_cards_blocked', cards: [] }, { refusal: { code: 'failed' } }], [false, null, { refusal: { code: 'failed' } }], [true, { title: 'no id' }, { refusal: { code: 'failed' } }], ]; @@ -248,7 +255,7 @@ test('a card that can never refresh withdraws the live link: named, deleted, nev { cmp_a: [{ body: READY }], cmp_b: [{ body: refused('recipe_missing') }], - cmp_c: [{ status: 404, body: { error: 'not_found' } }], + cmp_c: [{ status: 404, body: { code: 'not_found', retryable: false } }], }, ); const reported = await open('canvas', 'live'); diff --git a/src/lib/studio/test/spendPrompt.test.ts b/src/lib/studio/test/spendPrompt.test.ts index f3fc4ed..de07677 100644 --- a/src/lib/studio/test/spendPrompt.test.ts +++ b/src/lib/studio/test/spendPrompt.test.ts @@ -31,9 +31,14 @@ test('admitTurn answers a turn from its allowance', async (t) => { await t.test('a spent allowance is refused with 429 and Retry-After', async () => { const response = await admitTurn(async () => REFUSAL); + const body = await response?.json(); + assert.equal(response?.status, 429); assert.equal(response?.headers.get('retry-after'), '120'); - assert.equal(((await response?.json()) as ToolError).error, 'resource_exhausted'); + assert.equal(response?.headers.get('content-type'), 'application/problem+json'); + assert.equal(body.code, 'resource_exhausted'); + assert.equal(body.retryable, false); + assert.equal(body.retry_after_seconds, 120); }); await t.test('an allowance that cannot be read refuses the turn with 503', async () => { @@ -44,8 +49,12 @@ test('admitTurn answers a turn from its allowance', async (t) => { const stalled = await admitTurn(() => new Promise(() => {}), 20); for (const response of [failed, stalled]) { + const body = await response?.json(); + assert.equal(response?.status, 503); - assert.equal(((await response?.json()) as ToolError).error, 'transient_unavailable'); + assert.equal(response?.headers.get('content-type'), 'application/problem+json'); + assert.equal(body.code, 'transient_unavailable'); + assert.equal(body.retryable, true); } assert.equal(error.mock.callCount(), 2); }); diff --git a/vendor/developmentfinance-client-0.1.0.tgz b/vendor/developmentfinance-client-0.1.0.tgz index bf2da8c..92e59e4 100644 Binary files a/vendor/developmentfinance-client-0.1.0.tgz and b/vendor/developmentfinance-client-0.1.0.tgz differ