fix(service-datasource,rest): the last three uncovered datasource routes answer their registered refusal code (#4264) (#4313)

#4249 (#4263) gave the rest surface's two introspection routes a failure
contract; three sibling routes still had no catch around their service
call, so a throw was swallowed by the adapter and surfaced as the
pre-#3675 non-envelope 500 { error: 'No response from handler' } — no
success flag, no error.message, the real cause lost.

Each now answers 400 in the declared envelope, under the code registered
(ADR-0112) for the service the route dispatches to:

- GET /api/v1/datasources → DATASOURCE_ADMIN_ERROR, matching its eight
  siblings in admin-routes.ts
- POST /datasources/:name/external/refresh-catalog and
  POST /datasources/:name/external/validate → EXTERNAL_DATASOURCE_ERROR,
  the code #4249 gave the two introspection routes above them

The issue left INTERNAL_ERROR open as an alternative; the per-service
codes win on consistency — every other catch in both modules, including
pure reads, already answers 400 with the service-attributed code, and
refreshCatalog's dominant throw class is the one #4249 already
adjudicated as a 400 refusal on listRemoteTables. A 500 would fork the
failure contract within a module — the drift #4249 removed.

No new codes (both registered by #4263). The envelope-conformance suites
and the REFUSALS pin table gain one row per route.

Closes #4264


Claude-Session: https://claude.ai/code/session_01VDgGWS97x6vuikjmgMswtk

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.changeset/datasource-routes-catch-service-throws.md

@@ -0,0 +1,38 @@
+---
+"@objectstack/service-datasource": patch
+"@objectstack/rest": patch
+---
+
+fix(service-datasource,rest): the last three uncovered datasource routes answer their registered refusal code (#4264)
+
+#4249 (fixed in #4263) gave the rest surface's two introspection routes a
+failure contract; this closes the same gap on the three sibling routes it left
+uncovered. Each had no `catch` around its service call, so a service throw was
+swallowed by the adapter and surfaced as the pre-#3675 non-envelope
+`500 { error: 'No response from handler' }` — no `success` flag, no
+`error.message`, no code to switch on, real cause lost.
+
+Wire-visible changes — each route now answers `400` in the declared envelope,
+under the refusal code registered (ADR-0112) for the service it dispatches to,
+with the service's own message at `error.message`:
+
+- `GET /api/v1/datasources` (`listDatasources` throw) →
+  `400 DATASOURCE_ADMIN_ERROR` — matching its eight siblings in
+  `service-datasource/admin-routes.ts`, which already answer their catches this
+  way.
+- `POST /api/v1/datasources/:name/external/refresh-catalog` (`refreshCatalog`
+  throw) and `POST /api/v1/datasources/:name/external/validate` (`validateAll`
+  throw) → `400 EXTERNAL_DATASOURCE_ERROR` — the same code #4249 gave the two
+  introspection routes one block above them.
+
+The issue left the code choice open (`INTERNAL_ERROR` was the alternative);
+the registered per-service codes win on consistency: every other catch in both
+modules — including pure reads — already answers 400 with the service-attributed
+code, and `refreshCatalog`'s dominant throw class (unknown datasource,
+unreachable remote, no such schema) is the one #4249 already adjudicated as a
+400 refusal on `listRemoteTables`. A 500 here would fork the failure contract
+within a module — the drift #4249 removed.
+
+No new codes: both were registered in the error-code ledger by #4263. The
+envelope-conformance suites and the `REFUSALS` pin table gain one row per
+route.

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

@@ -227,6 +227,29 @@ describe('external-datasource envelope (#3843) — error bodies', () => {
         { params: { name: 'ext', remote: 'customers' } },
       ),
     },
+    {
+      // #4264: the two rows below are the routes #4249 left uncovered — no
+      // `catch` at all, so these throws surfaced as the adapter's non-envelope
+      // `500 { error: 'No response from handler' }`, the real cause swallowed.
+      name: 'a catalog refresh the service refuses',
+      status: 400,
+      code: 'EXTERNAL_DATASOURCE_ERROR',
+      run: () => drive(
+        mount({ refreshCatalog: async () => { throw new Error('unknown datasource "ext"'); } }),
+        'POST',
+        `${EXT}/refresh-catalog`,
+      ),
+    },
+    {
+      name: 'a validation sweep the service refuses',
+      status: 400,
+      code: 'EXTERNAL_DATASOURCE_ERROR',
+      run: () => drive(
+        mount({ validateAll: async () => { throw new Error('metadata store offline'); } }),
+        'POST',
+        `${EXT}/validate`,
+      ),
+    },
   ];
 
   for (const c of CASES) {

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

@@ -43,6 +43,10 @@ import { sendOk, sendError } from '@objectstack/types';
  * 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.
+ * #4264 closed the same gap on the two routes #4249 left uncovered —
+ * `refreshCatalog` / `validateAll` — whose throws (unknown datasource,
+ * unreachable remote, metadata-store failure) took the same uncaught path to
+ * that adapter 500. Every service call this module makes is now wrapped.
  *
  * 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
@@ -69,8 +73,13 @@ 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) =>
+  /**
+   * A refusal from the external-datasource service — same code as the
+   * admin-routes path (#4249). Named after the service, not one operation:
+   * since #4264 every route here except the import (which has its own
+   * registered code) answers its catch with this.
+   */
+  const refused = (res: any, err: unknown) =>
     sendError(res, 400, 'EXTERNAL_DATASOURCE_ERROR', err instanceof Error ? err.message : String(err));
 
   // List remote tables (optionally filtered by ?schema=).
@@ -82,7 +91,7 @@ export function registerExternalDatasourceRoutes(
       const tables = await svc.listRemoteTables(req.params.name, { schema });
       sendOk(res, { tables });
     } catch (err) {
-      introspectionRefused(res, err);
+      refused(res, err);
     }
   });
 
@@ -98,7 +107,7 @@ export function registerExternalDatasourceRoutes(
       );
       sendOk(res, { draft });
     } catch (err) {
-      introspectionRefused(res, err);
+      refused(res, err);
     }
   });
 
@@ -130,16 +139,24 @@ export function registerExternalDatasourceRoutes(
   server.post(`${ext}/refresh-catalog`, async (req: any, res: any) => {
     const svc = externalService();
     if (!svc?.refreshCatalog) return unavailable(res);
-    const catalog = await svc.refreshCatalog(req.params.name);
-    sendOk(res, { catalog });
+    try {
+      const catalog = await svc.refreshCatalog(req.params.name);
+      sendOk(res, { catalog });
+    } catch (err) {
+      refused(res, err);
+    }
   });
 
   // Validate the federated objects on this datasource.
   server.post(`${ext}/validate`, async (req: any, res: any) => {
     const svc = externalService();
     if (!svc?.validateAll) return unavailable(res);
-    const report = await svc.validateAll();
-    const results = (report.results ?? []).filter((r: any) => r.datasource === req.params.name);
-    sendOk(res, { ok: results.every((r: any) => r.ok), results });
+    try {
+      const report = await svc.validateAll();
+      const results = (report.results ?? []).filter((r: any) => r.datasource === req.params.name);
+      sendOk(res, { ok: results.every((r: any) => r.ok), results });
+    } catch (err) {
+      refused(res, err);
+    }
   });
 }

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

@@ -260,6 +260,9 @@ describe('registerDatasourceAdminRoutes (real HonoHttpServer)', () => {
     svc: Record<string, unknown>;
     run: (app: any) => Promise<Response>;
   }> = [
+    // The list row is #4264: the one route that had no refusal path at all —
+    // its throw surfaced as the adapter's non-envelope 500, not any code here.
+    { route: 'GET /datasources', service: 'datasource-admin', code: 'DATASOURCE_ADMIN_ERROR', svc: { listDatasources: async () => { throw new Error('backing store offline'); } }, run: (a) => a.fetch(json('/api/v1/datasources')) },
     { 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: '{}' })) },

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

@@ -187,6 +187,15 @@ describe('datasource-admin envelope (#3843) — error bodies', () => {
       code: 'DATASOURCE_ADMIN_ERROR',
       run: () => drive(mount({ createDatasource: async () => { throw new Error('duplicate name'); } }), '/api/v1/datasources', { method: 'POST', body: JSON.stringify({ name: 'pg' }) }),
     },
+    {
+      // #4264: the one route in the module that still had no `catch`, so this
+      // throw surfaced as the adapter's non-envelope
+      // `500 { error: 'No response from handler' }` instead of the 400 below.
+      name: 'a datasource listing failure',
+      status: 400,
+      code: 'DATASOURCE_ADMIN_ERROR',
+      run: () => drive(mount({ listDatasources: async () => { throw new Error('backing store offline'); } }), '/api/v1/datasources'),
+    },
     {
       // On an external-datasource route, so the refusal carries THAT service's
       // registered code (#4249) — even though this one is raised by the route

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

@@ -170,12 +170,19 @@ export function registerDatasourceAdminRoutes(
     return { draft, secret: normalised };
   };
 
-  // List all datasources with provenance + health.
+  // List all datasources with provenance + health. The catch was missing
+  // until #4264 — the one route in this module without one, so a backing-store
+  // failure surfaced as the adapter's non-envelope 500 instead of the 400 its
+  // eight siblings answer.
   server.get(root, async (_req: any, res: any) => {
     const svc = resolve(res, 'datasource-admin', 'listDatasources');
     if (!svc) return;
-    const datasources = await svc.listDatasources();
-    sendOk(res, { datasources });
+    try {
+      const datasources = await svc.listDatasources();
+      sendOk(res, { datasources });
+    } catch (err) {
+      badRequest(res, 'datasource-admin', err);
+    }
   });
 
   // Catalog of connection drivers + their JSON-Schema config (drives the