Skip to content

The same three datasource-admin routes answer 400 with the wrong service's error code (#4225 follow-up, one field over) #4249

Description

@os-zhuang

#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:

constbadRequest=(res: any,err: unknown)=>sendError(res,400,'DATASOURCE_ADMIN_ERROR',errinstanceofError ? err.message : String(err));

All three external-datasource routes route their failures through it:

Routeresolves400 error.code
GET /:name/remote-tablesexternal-datasourceDATASOURCE_ADMIN_ERROR
POST /:name/testexternal-datasourceDATASOURCE_ADMIN_ERROR
POST /:name/object-draftexternal-datasourceDATASOURCE_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 only

Its 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:

  1. Register EXTERNAL_DATASOURCE_ERROR and use it on the three routes. EXTERNAL_IMPORT_ERROR is already registered next door under @objectstack/rest, so the owner package for a new one needs a decision too.
  2. Widen EXTERNAL_IMPORT_ERROR to cover introspection — but the name says import, and these are not imports, so this is only honest with a rename.
  3. 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()=>{thrownewError('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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions