Repository navigation
The same three datasource-admin routes answer 400 with the wrong service's error code (#4225 follow-up, one field over) #4249
Description
#4225 / #4234 fixed the 503 message on the three routes in packages/services/service-datasource/src/admin-routes.ts that resolve external-datasource rather than datasource-admin. The identical mis-attribution survives one field over, on the 400 path — and this one is machine-readable.
The finding
admin-routes.ts has one shared 400 helper:
const badRequest = (res: any, err: unknown) =>
sendError(res, 400, 'DATASOURCE_ADMIN_ERROR', err instanceof Error ? err.message : String(err));All three external-datasource routes route their failures through it:
| Route | resolves | 400 error.code |
|---|---|---|
GET /:name/remote-tables |
external-datasource |
DATASOURCE_ADMIN_ERROR |
POST /:name/test |
external-datasource |
DATASOURCE_ADMIN_ERROR |
POST /:name/object-draft |
external-datasource |
DATASOURCE_ADMIN_ERROR |
The ledger says what that code means, in as many words:
// packages/spec/src/api/error-code-ledger.zod.ts
'@objectstack/service-datasource': [
'DATASOURCE_ADMIN_ERROR', // lifecycle/validation refusal from the datasource-admin service
],So a no such schema raised by the external-datasource introspector is reported as a lifecycle/validation refusal from datasource-admin. Same class as #4225, but where that one misled a human reading prose, this misroutes a client switching on error.code.
Why #4225 deliberately did not fix it
#4225's argument was that SERVICE_UNAVAILABLE is correct for all nine routes — ADR-0112's ledger asks generic conditions to reuse the standard catalog rather than register a per-service synonym — so only the message was wrong, and message is free text. DATASOURCE_ADMIN_ERROR is neither generic nor free: ApiErrorSchema.code is the closed ErrorCode union, so correcting it means editing the ledger. That is a spec change, and #4234 kept error.code byte-identical on all nine routes on purpose.
What the fix has to decide
The sibling surface does not settle it either. packages/rest/src/external-datasource-routes.ts catches on the import route only:
sendError(res, 400, 'EXTERNAL_IMPORT_ERROR', ...) // import onlyIts GET /external/tables and POST /external/tables/:remote/draft carry no try/catch at all. So the same two service operations — listRemoteTables, generateObjectDraft — reach REST by two paths with different failure contracts: a 400 DATASOURCE_ADMIN_ERROR through admin-routes.ts, and an uncaught throw through external-datasource-routes.ts. Worth settling in one pass rather than fixing the code string alone.
Roughly:
- Register
EXTERNAL_DATASOURCE_ERRORand use it on the three routes.EXTERNAL_IMPORT_ERRORis already registered next door under@objectstack/rest, so the owner package for a new one needs a decision too. - Widen
EXTERNAL_IMPORT_ERRORto cover introspection — but the name says import, and these are not imports, so this is only honest with a rename. - Decide these are not 400s at all, and let an introspection failure surface the way the rest surface currently lets it: uncaught.
The current drift is pinned by a test
packages/services/service-datasource/src/__tests__/envelope.conformance.test.ts asserts the wrong code today:
{
name: 'a remote-table introspection failure',
status: 400,
code: 'DATASOURCE_ADMIN_ERROR',
run: () => drive(mount({ listRemoteTables: async () => { throw new Error('no such schema'); } }), '/api/v1/datasources/ext/remote-tables'),
},That row moves with the fix, whichever option wins.
Pre-existing, like #4225: #3843 carried every code string over verbatim.
Activity
Merge origin/main into claude/datasource-admin-error-code-y1eels
fix(service-datasource,rest,spec)!: external-datasource refusals answ…
fix(service-datasource,rest): the last three uncovered datasource rou…
fix(service-datasource): forward `?schema=` on the admin remote-table…
fix(security): the external-datasource federation HTTP family require…
fix(security): the external-datasource federation HTTP family require…
docs(service-datasource): re-anchor the dead tracker citations to the…