diff --git a/.changeset/rest-map-schema-errors.md b/.changeset/rest-map-schema-errors.md new file mode 100644 index 000000000..a94e6a8bc --- /dev/null +++ b/.changeset/rest-map-schema-errors.md @@ -0,0 +1,15 @@ +--- +'@objectstack/rest': patch +--- + +fix(rest): map schema-mismatch & not-null driver errors to structured 4xx + +`mapDataError` collapsed any SQL-looking driver error into a generic +`500 DATABASE_ERROR`, so a bad write payload to the data API leaked a 500 +instead of a fixable 4xx (e.g. `POST /data/sys_team` with an unknown field, +or omitting a required column). It now maps unknown-column errors to +`400 INVALID_FIELD { field }` and not-null violations to +`400 VALIDATION_FAILED { fields:[{required}] }` across SQLite/Postgres/MySQL +phrasings, placed before the unknown-object branch so Postgres +`column … of relation … does not exist` is not mis-mapped to 404. Genuine +driver faults still return 500; unique violations still return 409. diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 89f22712e..ba7569e44 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -29,7 +29,7 @@ const TRANSLATABLE_META_TYPES = new Set(['view', 'action', 'object', 'app', 'das * not permitted …" — trips the `'' … not` substring check and * returns a misleading 404. */ -function mapDataError(error: any, object?: string): { status: number; body: Record } { +export function mapDataError(error: any, object?: string): { status: number; body: Record } { // Optimistic-Concurrency-Control mismatch → 409 with current state. // Surfaced FIRST so the structured fields (`currentVersion`, // `currentRecord`) are preserved instead of being squashed into the @@ -122,6 +122,59 @@ function mapDataError(error: any, object?: string): { status: number; body: Reco }; } + // Schema-mismatch & required-field violations are CLIENT errors (a bad + // payload the caller can fix), not server faults — so map them to a + // structured 4xx BEFORE the unknown-object / SQL-leak branches, which + // would otherwise bury them in a generic 404 or 500. Driver phrasing + // varies by dialect; cover SQLite / Postgres / MySQL: + // unknown column → SQLite "table X has no column named c" / + // "no such column: c"; Postgres 'column "c" of + // relation "X" does not exist'; MySQL "Unknown + // column 'c' in 'field list'". + // not-null → SQLite "NOT NULL constraint failed: X.c"; + // Postgres 'null value in column "c" ... violates + // not-null constraint'; MySQL "Column 'c' cannot + // be null". + // NOTE: this is a last-resort safety net — the validation layer should + // ideally reject these before they reach the driver (see follow-ups on + // unknown-field rejection + provenance-aware required checks). + const unknownColumn = + /has no column named\s+["'`]?([a-z0-9_]+)/i.exec(raw) || + /no such column:\s*["'`]?([a-z0-9_.]+)/i.exec(raw) || + /unknown column\s+["'`]([a-z0-9_]+)["'`]/i.exec(raw) || + /column\s+["'`]([a-z0-9_]+)["'`]\s+of relation\s+\S+\s+does not exist/i.exec(raw); + if (unknownColumn) { + const field = unknownColumn[1]?.split('.').pop(); + return { + status: 400, + body: { + error: field + ? `Unknown field '${field}'${object ? ` on object '${object}'` : ''}` + : 'Request references a field that does not exist', + code: 'INVALID_FIELD', + ...(field ? { field } : {}), + ...(object ? { object } : {}), + }, + }; + } + + const notNull = + /not null constraint failed:\s*\S*?\.([a-z0-9_]+)/i.exec(raw) || + /null value in column\s+["'`]([a-z0-9_]+)["'`]/i.exec(raw) || + /column\s+["'`]([a-z0-9_]+)["'`]\s+cannot be null/i.exec(raw); + if (notNull) { + const field = notNull[1]; + return { + status: 400, + body: { + error: `${field} is required`, + code: 'VALIDATION_FAILED', + fields: [{ field, code: 'required', message: `${field} is required` }], + ...(object ? { object } : {}), + }, + }; + } + const looksLikeUnknownObject = lower.includes('no such table') || lower.includes('relation') && lower.includes('does not exist') || diff --git a/packages/rest/src/rest.test.ts b/packages/rest/src/rest.test.ts index f68b6e342..37e0546e4 100644 --- a/packages/rest/src/rest.test.ts +++ b/packages/rest/src/rest.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import { RouteManager, RouteGroupBuilder } from './route-manager'; -import { RestServer } from './rest-server'; +import { RestServer, mapDataError } from './rest-server'; import { createRestApiPlugin } from './rest-api-plugin'; import type { RestApiPluginConfig } from './rest-api-plugin'; @@ -1426,3 +1426,92 @@ describe('RestServer.resolveProtocol', () => { expect(f.kernelManager.getOrCreate).not.toHaveBeenCalled(); }); }); + +// --------------------------------------------------------------------------- +// mapDataError — schema-mismatch & required-field violations must surface as +// structured 4xx, never a leaked 500 DATABASE_ERROR (repro: B7 POST +// /data/sys_team with a body whose fields don't match the table). +// --------------------------------------------------------------------------- +describe('mapDataError — schema/constraint envelopes', () => { + const sqliteError = (message: string, code = 'SQLITE_ERROR') => + Object.assign(new Error(message), { code }); + + it('maps SQLite "has no column named" → 400 INVALID_FIELD with the field', () => { + const r = mapDataError( + sqliteError( + "insert into `sys_team` (`id`, `label`, `name`) values (?, ?, ?) returning * - table sys_team has no column named label", + ), + 'sys_team', + ); + expect(r.status).toBe(400); + expect(r.body.code).toBe('INVALID_FIELD'); + expect(r.body.field).toBe('label'); + expect(r.body.object).toBe('sys_team'); + expect(String(r.body.error)).not.toMatch(/insert into|sqlite/i); + }); + + it('maps SQLite "no such column" → 400 INVALID_FIELD', () => { + const r = mapDataError(sqliteError('no such column: bogus'), 'widget'); + expect(r.status).toBe(400); + expect(r.body.code).toBe('INVALID_FIELD'); + expect(r.body.field).toBe('bogus'); + }); + + it('maps MySQL "Unknown column" → 400 INVALID_FIELD', () => { + const r = mapDataError(sqliteError("Unknown column 'label' in 'field list'", 'ER_BAD_FIELD_ERROR'), 'sys_team'); + expect(r.status).toBe(400); + expect(r.body.code).toBe('INVALID_FIELD'); + expect(r.body.field).toBe('label'); + }); + + it('maps Postgres "column ... of relation ... does not exist" → 400 INVALID_FIELD (not 404 object_not_found)', () => { + const r = mapDataError( + sqliteError('column "label" of relation "sys_team" does not exist', '42703'), + 'sys_team', + ); + expect(r.status).toBe(400); + expect(r.body.code).toBe('INVALID_FIELD'); + expect(r.body.field).toBe('label'); + }); + + it('maps SQLite NOT NULL constraint → 400 VALIDATION_FAILED with required field', () => { + const r = mapDataError( + sqliteError( + "insert into `sys_team` ... - NOT NULL constraint failed: sys_team.organization_id", + 'SQLITE_CONSTRAINT_NOTNULL', + ), + 'sys_team', + ); + expect(r.status).toBe(400); + expect(r.body.code).toBe('VALIDATION_FAILED'); + expect(r.body.fields).toEqual([ + { field: 'organization_id', code: 'required', message: 'organization_id is required' }, + ]); + }); + + it('maps Postgres not-null violation → 400 VALIDATION_FAILED', () => { + const r = mapDataError( + sqliteError('null value in column "organization_id" of relation "sys_team" violates not-null constraint', '23502'), + 'sys_team', + ); + expect(r.status).toBe(400); + expect(r.body.code).toBe('VALIDATION_FAILED'); + expect((r.body.fields as any[])[0].field).toBe('organization_id'); + }); + + it('still hides genuine SQL leaks (unrecognized driver dump) behind a generic 500', () => { + const r = mapDataError(sqliteError('SQLITE_IOERR: disk I/O error', 'SQLITE_IOERR'), 'sys_team'); + expect(r.status).toBe(500); + expect(r.body.code).toBe('DATABASE_ERROR'); + expect(String(r.body.error)).not.toMatch(/disk i\/o/i); + }); + + it('still maps unique-constraint violations to 409 (unchanged)', () => { + const r = mapDataError( + sqliteError('UNIQUE constraint failed: sys_team.name, sys_team.organization_id', 'SQLITE_CONSTRAINT_UNIQUE'), + 'sys_team', + ); + expect(r.status).toBe(409); + expect(r.body.code).toBe('UNIQUE_VIOLATION'); + }); +});