fix(service-datasource,rest,spec)!: external-datasource refusals answer their own error code (#4249)

.changeset/external-datasource-400s-answer-their-own-code.md

@@ -0,0 +1,58 @@
+---
+"@objectstack/spec": patch
+"@objectstack/service-datasource": patch
+"@objectstack/rest": patch
+---
+
+fix(service-datasource,rest)!: external-datasource refusals answer their own error code (#4249)
+
+#4225 / #4234 fixed the 503 `message` on the three routes in
+`service-datasource/admin-routes.ts` that dispatch to `external-datasource`
+rather than `datasource-admin`. The identical mis-attribution survived one field
+over, on the 400 path — and machine-readably: one shared `badRequest` helper
+hard-coded `DATASOURCE_ADMIN_ERROR`, which the ADR-0112 ledger defines as a
+refusal *from the datasource-admin service*. So a `no such schema` raised by the
+external-datasource introspector was reported as datasource-admin's, and where
+#4225 misled a human reading prose, this misrouted a client switching on
+`error.code`.
+
+`EXTERNAL_DATASOURCE_ERROR` is now registered in the error-code ledger — under
+`@objectstack/service-datasource` and `@objectstack/rest`, the two packages that
+emit it; per the ledger's own rule the per-package rows are provenance, not
+identity — and `badRequest` takes the same `ServiceName` the route passed to
+`resolve` (#4234), so the code, like the 503 message, comes from the service the
+route actually dispatches to.
+
+Wire-visible changes:
+
+- **The three external-datasource routes' 400 `error.code`** —
+  `GET /datasources/:name/remote-tables`, `POST /datasources/:name/test`,
+  `POST /datasources/:name/object-draft` — is now `EXTERNAL_DATASOURCE_ERROR`
+  (was `DATASOURCE_ADMIN_ERROR`). Status, envelope, and `error.message` are
+  unchanged, as is everything on the six datasource-admin routes. No consumer
+  branches on the old code (grepped both repos, all the ADR-0112 sweep forms).
+- **The rest surface's two introspection routes now have a failure contract at
+  all.** `GET /datasources/:name/external/tables` and
+  `POST /datasources/:name/external/tables/:remote/draft` carried no
+  `try`/`catch`, so the very same service operations that answer 400 through
+  the admin surface surfaced here as the adapter's non-envelope
+  `500 { error: 'No response from handler' }`. They now answer
+  `400 EXTERNAL_DATASOURCE_ERROR` in the declared envelope — one operation, one
+  failure contract, on both paths. (`EXTERNAL_IMPORT_ERROR` on the import route
+  is unchanged: a refused import is a different act from a failed
+  introspection, and its name says so.)
+
+Why a new registered code rather than reusing one: ADR-0112's ledger asks
+*generic* conditions to reuse the standard catalog — that argument carried
+#4225's 503, where `SERVICE_UNAVAILABLE` is correct for all nine routes and only
+the free-text `message` named the service. A refusal specific to one service is
+exactly what registered extension codes are for, and the closed `ErrorCode`
+union means correcting the attribution had to be a ledger edit. Widening
+`EXTERNAL_IMPORT_ERROR` to cover introspection was rejected because these are
+not imports; leaving the throws uncaught was rejected because the adapter's 500
+is not the declared envelope.
+
+The conformance rows that pinned the drift move with it, and each surface now
+pins the refusal code per route the way #4234 pinned the 503 message per route.
+
+Pre-existing, like #4225: #3843 carried every code string over verbatim.

content/docs/references/api/error-code-ledger.mdx

@@ -182,6 +182,7 @@ const result = ErrorCode.parse(data);
 * `EXPIRY_TOO_LONG`
 * `EXPLAIN_FAILED`
 * `EXPORT_NOT_PERMITTED`
+* `EXTERNAL_DATASOURCE_ERROR`
 * `EXTERNAL_IMPORT_ERROR`
 * `EXTERNAL_SCHEMA_MISMATCH`
 * `EXTERNAL_SCHEMA_MODE_VIOLATION`

packages/rest/src/external-datasource-envelope.conformance.test.ts

@@ -202,6 +202,31 @@ describe('external-datasource envelope (#3843) — error bodies', () => {
         { params: { name: 'ext', remote: 'customers' } },
       ),
     },
+    {
+      // #4249: before it, the two introspection routes had no `catch`, so this
+      // throw surfaced as the adapter's non-envelope 500 — while the same
+      // service operation through `service-datasource/admin-routes.ts` answered
+      // 400. Both paths now refuse with the same registered code.
+      name: 'an introspection the service refuses',
+      status: 400,
+      code: 'EXTERNAL_DATASOURCE_ERROR',
+      run: () => drive(
+        mount({ listRemoteTables: async () => { throw new Error('no such schema'); } }),
+        'GET',
+        `${EXT}/tables`,
+      ),
+    },
+    {
+      name: 'a draft generation the service refuses',
+      status: 400,
+      code: 'EXTERNAL_DATASOURCE_ERROR',
+      run: () => drive(
+        mount({ generateObjectDraft: async () => { throw new Error('no such table'); } }),
+        'POST',
+        `${EXT}/tables/:remote/draft`,
+        { params: { name: 'ext', remote: 'customers' } },
+      ),
+    },
   ];
 
   for (const c of CASES) {

packages/rest/src/external-datasource-routes.ts

@@ -37,6 +37,13 @@ import { sendOk, sendError } from '@objectstack/types';
  * registered under this package because a refused federated import is specific
  * to it.
  *
+ * `EXTERNAL_DATASOURCE_ERROR` (#4249) is the introspection twin: a refusal from
+ * `listRemoteTables` / `generateObjectDraft`. Before #4249 those two routes had
+ * no `catch` at all, so a `no such schema` surfaced as the adapter's
+ * non-envelope `500 { error: 'No response from handler' }` — while the SAME two
+ * service operations, reached through `service-datasource/admin-routes.ts`,
+ * answered 400. One operation, one failure contract now, on both paths.
+ *
  * Note `POST /validate` keeps its `ok` — unlike the `{ ok: true, key }` #3689
  * retired from storage, that one was a private second word for `success`, while
  * this `ok` is a COMPUTED verdict over the federated objects
@@ -62,25 +69,37 @@ export function registerExternalDatasourceRoutes(
   const unavailable = (res: any) =>
     sendError(res, 503, 'SERVICE_UNAVAILABLE', 'The external-datasource service is not available.');
 
+  /** An introspection refusal — same code as the admin-routes path (#4249). */
+  const introspectionRefused = (res: any, err: unknown) =>
+    sendError(res, 400, 'EXTERNAL_DATASOURCE_ERROR', err instanceof Error ? err.message : String(err));
+
   // List remote tables (optionally filtered by ?schema=).
   server.get(`${ext}/tables`, async (req: any, res: any) => {
     const svc = externalService();
     if (!svc?.listRemoteTables) return unavailable(res);
-    const schema = typeof req.query?.schema === 'string' ? req.query.schema : undefined;
-    const tables = await svc.listRemoteTables(req.params.name, { schema });
-    sendOk(res, { tables });
+    try {
+      const schema = typeof req.query?.schema === 'string' ? req.query.schema : undefined;
+      const tables = await svc.listRemoteTables(req.params.name, { schema });
+      sendOk(res, { tables });
+    } catch (err) {
+      introspectionRefused(res, err);
+    }
   });
 
   // Generate an Object draft (structured + *.object.ts source) from a table.
   server.post(`${ext}/tables/:remote/draft`, async (req: any, res: any) => {
     const svc = externalService();
     if (!svc?.generateObjectDraft) return unavailable(res);
-    const draft = await svc.generateObjectDraft(
-      req.params.name,
-      req.params.remote,
-      (req.body as Record<string, unknown>) ?? {},
-    );
-    sendOk(res, { draft });
+    try {
+      const draft = await svc.generateObjectDraft(
+        req.params.name,
+        req.params.remote,
+        (req.body as Record<string, unknown>) ?? {},
+      );
+      sendOk(res, { draft });
+    } catch (err) {
+      introspectionRefused(res, err);
+    }
   });
 
   // Import a remote table as a live (runtime-origin) federated object so it's

packages/services/service-datasource/README.md

@@ -40,7 +40,10 @@ Mounted under `/api/v1/datasources` by `registerDatasourceAdminRoutes` (lifecycl
 degrades gracefully (`503` / "unavailable") when its service isn't wired, and the
 message names **that** service — one registrar, but two services behind it: the
 three routes marked below dispatch to `external-datasource`, the rest to
-`datasource-admin` (#4225).
+`datasource-admin` (#4225). A refusal (`400`) carries the same attribution
+machine-readably: `error.code` is `DATASOURCE_ADMIN_ERROR` from the
+datasource-admin routes and `EXTERNAL_DATASOURCE_ERROR` from the
+external-datasource ones (#4249) — both registered in the ADR-0112 ledger.
 
 **Lifecycle & connection**
 - `GET    /datasources` — list (code + runtime, with provenance/health)

packages/services/service-datasource/src/__tests__/admin-routes.test.ts

@@ -239,6 +239,50 @@ describe('registerDatasourceAdminRoutes (real HonoHttpServer)', () => {
     );
   });
 
+  /**
+   * #4249 — the 400 `error.code` names the service that refused, the way the
+   * 503 `message` above names the one that was missing (#4225). Same defect one
+   * field over: `badRequest` hard-coded `DATASOURCE_ADMIN_ERROR`, so the three
+   * external-datasource routes reported their refusals as datasource-admin's —
+   * and where #4225 misled a human reading prose, this misrouted a client
+   * switching on `error.code`.
+   *
+   * One row per route with a refusal path. The `code` column is a deliberate
+   * PIN, not derived from the route's service, so a wrong entry in the module's
+   * `SERVICE_ERROR_CODE` map has to disagree with a literal here. Each mount
+   * wires ONLY the service the route should resolve — a route dispatching to
+   * the other one answers 503, failing the row before the code is even read.
+   */
+  const REFUSALS: Array<{
+    route: string;
+    service: string;
+    code: string;
+    svc: Record<string, unknown>;
+    run: (app: any) => Promise<Response>;
+  }> = [
+    { route: 'GET /datasources/:name', service: 'datasource-admin', code: 'DATASOURCE_ADMIN_ERROR', svc: { getDatasource: async () => { throw new Error('backing store offline'); } }, run: (a) => a.fetch(json('/api/v1/datasources/pg')) },
+    { route: 'POST /datasources/test', service: 'datasource-admin', code: 'DATASOURCE_ADMIN_ERROR', svc: { testConnection: async () => { throw new Error('backing store offline'); } }, run: (a) => a.fetch(json('/api/v1/datasources/test', { method: 'POST', body: '{}' })) },
+    { route: 'POST /datasources', service: 'datasource-admin', code: 'DATASOURCE_ADMIN_ERROR', svc: { createDatasource: async () => { throw new Error('backing store offline'); } }, run: (a) => a.fetch(json('/api/v1/datasources', { method: 'POST', body: '{}' })) },
+    { route: 'PATCH /datasources/:name', service: 'datasource-admin', code: 'DATASOURCE_ADMIN_ERROR', svc: { updateDatasource: async () => { throw new Error('backing store offline'); } }, run: (a) => a.fetch(json('/api/v1/datasources/pg', { method: 'PATCH', body: '{}' })) },
+    { route: 'DELETE /datasources/:name', service: 'datasource-admin', code: 'DATASOURCE_ADMIN_ERROR', svc: { removeDatasource: async () => { throw new Error('backing store offline'); } }, run: (a) => a.fetch(json('/api/v1/datasources/pg', { method: 'DELETE' })) },
+    { route: 'GET /datasources/:name/remote-tables', service: 'external-datasource', code: 'EXTERNAL_DATASOURCE_ERROR', svc: { listRemoteTables: async () => { throw new Error('no such schema'); } }, run: (a) => a.fetch(json('/api/v1/datasources/ext/remote-tables')) },
+    { route: 'POST /datasources/:name/test', service: 'external-datasource', code: 'EXTERNAL_DATASOURCE_ERROR', svc: { testConnection: async () => { throw new Error('connection refused'); } }, run: (a) => a.fetch(json('/api/v1/datasources/ext/test', { method: 'POST', body: '{}' })) },
+    { route: 'POST /datasources/:name/object-draft', service: 'external-datasource', code: 'EXTERNAL_DATASOURCE_ERROR', svc: { generateObjectDraft: async () => { throw new Error('no such table'); } }, run: (a) => a.fetch(json('/api/v1/datasources/ext/object-draft', { method: 'POST', body: JSON.stringify({ table: 'customers' }) })) },
+  ];
+
+  for (const c of REFUSALS) {
+    it(`${c.route} refuses as 400 ${c.code} (#4249)`, async () => {
+      const res = await c.run(mountServices({ [c.service]: c.svc }));
+      expect(res.status).toBe(400);
+      const body = (await res.json()) as any;
+      expect(body.success).toBe(false);
+      expect(body.error.code).toBe(c.code);
+      // The service's own message still reads at `error.message`, as before.
+      expect(typeof body.error.message).toBe('string');
+      expect(body.error.message.length).toBeGreaterThan(0);
+    });
+  }
+
   it('surfaces lifecycle errors as 400 with the service message', async () => {
     const createDatasource = vi.fn().mockRejectedValue(new Error('duplicate name'));
     const app = mount({ createDatasource });

packages/services/service-datasource/src/__tests__/envelope.conformance.test.ts

@@ -188,9 +188,12 @@ describe('datasource-admin envelope (#3843) — error bodies', () => {
       run: () => drive(mount({ createDatasource: async () => { throw new Error('duplicate name'); } }), '/api/v1/datasources', { method: 'POST', body: JSON.stringify({ name: 'pg' }) }),
     },
     {
+      // On an external-datasource route, so the refusal carries THAT service's
+      // registered code (#4249) — even though this one is raised by the route
+      // itself before the service is called.
       name: 'a missing required body field',
       status: 400,
-      code: 'DATASOURCE_ADMIN_ERROR',
+      code: 'EXTERNAL_DATASOURCE_ERROR',
       run: () => drive(mount({ generateObjectDraft: async () => ({}) }), '/api/v1/datasources/ext/object-draft', { method: 'POST', body: '{}' }),
     },
     {
@@ -200,11 +203,20 @@ describe('datasource-admin envelope (#3843) — error bodies', () => {
       run: () => drive(mount({ getDatasource: async () => undefined }), '/api/v1/datasources/nope'),
     },
     {
+      // #4249: raised by the external-datasource introspector, so the code says
+      // so. Until then this row pinned `DATASOURCE_ADMIN_ERROR` — the same
+      // mis-attribution #4225 fixed in the 503 `message`, machine-readable here.
       name: 'a remote-table introspection failure',
       status: 400,
-      code: 'DATASOURCE_ADMIN_ERROR',
+      code: 'EXTERNAL_DATASOURCE_ERROR',
       run: () => drive(mount({ listRemoteTables: async () => { throw new Error('no such schema'); } }), '/api/v1/datasources/ext/remote-tables'),
     },
+    {
+      name: 'a saved-datasource connection test failure',
+      status: 400,
+      code: 'EXTERNAL_DATASOURCE_ERROR',
+      run: () => drive(mount({ testConnection: async () => { throw new Error('connection refused'); } }), '/api/v1/datasources/ext/test', { method: 'POST', body: '{}' }),
+    },
     {
       name: 'a removal failure',
       status: 400,