From 158c62451aa3a508a451145ae332d971a9da3f64 Mon Sep 17 00:00:00 2001 From: Mao Nakamoto Date: Thu, 3 Sep 2026 04:05:07 +0000 Subject: [PATCH] fix(api): stop surfacing DB errors as "no surface/composition yet" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /api/surfaces and /api/compositions collapsed every Supabase error into { success: false, data: null } with no error field. useSurface and useComposition only throw when success is false AND error is set, so a real query failure — a transient outage, an RLS misconfiguration — was indistinguishable from the legitimate "you haven't created one yet" state. A user who'd already saved a surface, hitting a transient failure, was silently bounced back to "define your surface" as if the work had never happened. Switch both handlers from .single() to .maybeSingle() (already the project's convention for "zero rows is a valid outcome", per ownsProject/ownsFigure), which resolves data: null with no error on zero rows and still surfaces error.message for genuine failures — the same shape every other route in the app already uses. Added a structural test that walks every API route's NextResponse.json({ success: false, ... }) call site and fails on any missing an error field, so this class of bug can't come back by hand. --- app/src/app/api/compositions/route.ts | 4 +- app/src/app/api/error-shape.test.ts | 76 +++++++++++++++++++++++++++ app/src/app/api/surfaces/route.ts | 4 +- 3 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 app/src/app/api/error-shape.test.ts diff --git a/app/src/app/api/compositions/route.ts b/app/src/app/api/compositions/route.ts index bc4a253..36bcf5a 100644 --- a/app/src/app/api/compositions/route.ts +++ b/app/src/app/api/compositions/route.ts @@ -19,9 +19,9 @@ export async function GET(request: NextRequest) { .eq('project_id', projectId) .order('version', { ascending: false }) .limit(1) - .single(); + .maybeSingle(); - if (error) return NextResponse.json({ success: false, data: null }); + if (error) return NextResponse.json({ success: false, error: error.message }, { status: 500 }); return NextResponse.json({ success: true, data }); } diff --git a/app/src/app/api/error-shape.test.ts b/app/src/app/api/error-shape.test.ts new file mode 100644 index 0000000..0e73cda --- /dev/null +++ b/app/src/app/api/error-shape.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, it } from 'vitest'; +import { readFileSync, readdirSync } from 'node:fs'; +import { join, relative } from 'node:path'; +import ts from 'typescript'; + +/** + * /api/surfaces and /api/compositions GET handlers collapsed every Supabase + * error — a real DB outage, an RLS misconfiguration, not just "no row yet" — + * into `{ success: false, data: null }`. useSurface/useComposition only throw + * on `!success && error`, so with no `error` field the query resolved clean + * to `data: null`, indistinguishable from "you haven't saved one yet". A + * transient failure after the user saved their surface silently looked like + * they'd never defined one, and the UI sent them back to redo work that was + * already stored. + * + * Every other route in the app forwards `error.message` on failure; this + * walks each `NextResponse.json({ success: false, ... })` call site in the + * API layer and fails on any that omits `error`, so a route can't drop back + * into the swallowed shape by hand. + */ +describe('every failed API response carries an error message', () => { + const apiDir = join(process.cwd(), 'src/app/api'); + + function routeFiles(dir: string): string[] { + return readdirSync(dir, { withFileTypes: true }).flatMap((entry) => { + const path = join(dir, entry.name); + if (entry.isDirectory()) return routeFiles(path); + return entry.name === 'route.ts' ? [path] : []; + }); + } + + function isFalseLiteral(node: ts.Expression): boolean { + return node.kind === ts.SyntaxKind.FalseKeyword; + } + + function missingErrorIn(file: string): string[] { + const source = ts.createSourceFile( + file, + readFileSync(file, 'utf8'), + ts.ScriptTarget.Latest, + true, + ts.ScriptKind.TS, + ); + const missing: string[] = []; + + function visit(node: ts.Node) { + if (ts.isObjectLiteralExpression(node)) { + const successProp = node.properties.find( + (p): p is ts.PropertyAssignment => + ts.isPropertyAssignment(p) && p.name.getText(source) === 'success', + ); + const hasError = node.properties.some( + (p) => p.name?.getText(source) === 'error' || p.name?.getText(source) === 'details', + ); + if (successProp && isFalseLiteral(successProp.initializer) && !hasError) { + const line = source.getLineAndCharacterOfPosition(node.getStart()).line + 1; + missing.push(`${relative(apiDir, file)}:${line}`); + } + } + ts.forEachChild(node, visit); + } + + visit(source); + return missing; + } + + const files = routeFiles(apiDir); + + it('finds the route files', () => { + expect(files.length).toBeGreaterThan(0); + }); + + it('leaves no failed response without an error field', () => { + expect(files.flatMap(missingErrorIn)).toEqual([]); + }); +}); diff --git a/app/src/app/api/surfaces/route.ts b/app/src/app/api/surfaces/route.ts index b586346..631146a 100644 --- a/app/src/app/api/surfaces/route.ts +++ b/app/src/app/api/surfaces/route.ts @@ -17,9 +17,9 @@ export async function GET(request: NextRequest) { .from('surfaces') .select('*') .eq('project_id', projectId) - .single(); + .maybeSingle(); - if (error) return NextResponse.json({ success: false, data: null }); + if (error) return NextResponse.json({ success: false, error: error.message }, { status: 500 }); return NextResponse.json({ success: true, data }); }