Skip to content

Commit ea41c49

Browse files
fix(rest): relay a declared service outage from the external-datasource routes as 503 (#30952)
<!-- claude-projects-attribution: {"github_login":"os-elon-musk"} --> _Requested by **Elon** · [project thread](https://claude.ai/code/project/chan_01WedDzV26nniwxv7jC88B9D?thread=cmsg_01WedDzV26nniwxv7jC88B9DUZr21b5Uwo4sS3QxyC1M1x)_ Fixes #30609 Clause-②: no Metadata-protocol: none domain:server#5 card thread · session `session_016WV4cwJiP1ALmQ1rTttNGc` · branch `claude/project-thread-ticta6` (named in the seat's re-posted claim on the card). Branched from `main` at 33043bd. ## 维护者速读 - 改了什么:外部数据源导入接口(`POST /api/v1/datasources/:name/external/tables/:remote/import`)遇到租户服务暂时不可用时,现在回 `503 SERVICE_UNAVAILABLE`(请稍后重试),不再回 `400 EXTERNAL_IMPORT_ERROR`(请求写错了)。同一组外部数据源接口共用这一处,所以它们遇到同类服务停摆也一样回 503。 - 为什么改:导入要先读租户形态来决定对象落到哪个包;租户服务读不出时抛的是「服务不可用」,但这里把它当成调用方的错误,调用方会去改请求而不是重试。数据源管理接口和 MCP `import_object` 工具早就回 503,REST 这一处没跟上。 - 风险与回滚:很低。只改了一个错误分支的状态码,只认同时带 `status 503` 和 `code SERVICE_UNAVAILABLE` 的错误;其它拒绝照旧 400,此前此后都不会保存任何东西。回滚即 revert 本 PR。 - 先看:`packages/rest/src/external-datasource-routes.ts` 的 `answerThrown`(新增的 503 分支,在 404 分支之前);`packages/rest/src/external-datasource-unknown-name.test.ts` 末尾新增的两个用例。 - 两分钟验证:`pnpm --filter @objectstack/rest exec vitest run src/external-datasource-unknown-name.test.ts`,预期 17 个用例全过,其中「answers 503 SERVICE_UNAVAILABLE and saves nothing」一条。 - 需要你做的:无,席位落地。 ## What changes `answerThrown`, the one catch every `/datasources/:name/external/*` route answers through, relayed only sandbox faults and the `404 RESOURCE_NOT_FOUND` envelope; every other throw became this family's `400`. Since PR #30602 the import's landing reads the effective tenancy posture (`classifyAdmissionTenancyPosture`), which throws `AuthzStoreUnavailableError` (`status 503`, `code SERVICE_UNAVAILABLE`) when a registered `tenancy` service cannot answer. The import therefore told the caller its request was malformed when the truth was "retry". It now relays an error carrying `status 503` and `code SERVICE_UNAVAILABLE` as that envelope, ahead of the `404` relay, exactly as `service-datasource`'s `badRequest` does. - **Choice of read: the envelope (`status` + `code`), not `isAuthzStoreUnavailableError`.** It is the read both twins make (`badRequest` in `admin-routes.ts`, and the MCP `import_object` tool's `thrownRefusalEnvelope`), so the three doors cannot drift on what counts as an outage. The brand predicate would relay only the authorization-store outage and keep answering `400` for any other service that declares the same envelope. Both fields are required, so an error carrying a stray `status` cannot re-route itself. - **The `EXTERNAL_DATASOURCE_ERROR` routes get the same relay.** They share the catch, and a declared dependency outage is the same truth on a read or a draft as on the import: `400` would tell the caller to change a request that was never wrong. This matches `badRequest`, which serves every admin route the same way. No refusal that is not a declared outage changes: a sandboxed refusal declaring a 4xx and the metadata door's `409 DESTRUCTIVE_CHANGE` keep this family's `400` and code. File surface as claimed: `answerThrown` only, its test beside it, one changeset. `refuseFederationRequest` and the route registrations are untouched (open draft PR #30702 edits those; hunks disjoint, re-checked before push). ## Tests New top-level `describe` in `packages/rest/src/external-datasource-unknown-name.test.ts`, the file that already pins `answerThrown`'s `404` relay over the real `ExternalDatasourceService` and the real `HonoHttpServer`. Its `mount()` takes config overrides now; the existing pins are unchanged. The landing hook runs the real `classifyAdmissionTenancyPosture` over a `tenancy` factory that rejects, which is the read `service-datasource`'s plugin makes. - the import on that outage answers `503 SERVICE_UNAVAILABLE`, its message names the failed `tenancy` read, carries no `data`, and `persistObject` is never called; - a plain landing refusal from the same seam still answers `400 EXTERNAL_IMPORT_ERROR` with its own sentence, nothing saved. Reproduction and ablation, one run (`node scripts/ablation-replace.mjs`, anchor `envelope?.status === 503 && envelope?.code === 'SERVICE_UNAVAILABLE'` replaced by `false`, anchor x1 -> x0, blob b99785d -> 6d0515af): the outage pin goes red with `expected { status: 400, … } to deeply equal { status: 503, … }` (1 failed, 16 passed), which is the defect as `main` answers it; restored, blob == HEAD, `git diff HEAD` empty. The test imports the routes from `src/` by relative path, so no build sits between the mutation and the run. ## Acceptance notes - None. ## Verification All local readings on head `3bccb0f74e` (the final head), worktree off `main` 33043bd; `origin/main` has since moved 6 commits, none touching `packages/rest`, `service-datasource` or `packages/core/src/security`, so not merged. <details><summary>Derived gate families: 60 derived, 59 run green, 1 NOT MEASURED</summary> - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 60 commands; each run with its exit code captured before any pipe (record: `gates-ran-3bccb0f74e.txt`). - `dispatch-gates.mjs --ran`: `60 derived famil(ies) accounted for — 59 run, 1 NOT-MEASURED`. - NOT MEASURED: `pnpm check:dual-build-cjs-loads`, reason: exit 3 `PREREQUISITE NOT MET` (reads the whole tree's `dist/`, which this worktree has not built; CI's run is the reading). </details> <details><summary>Package and repo checks</summary> - `pnpm --filter @objectstack/rest test` (under the verify lock): exit 0, `Test Files 393 passed (393)`, `Tests 7458 passed | 332 skipped (7790)`. - `pnpm --filter @objectstack/rest typecheck`: exit 0. - `pnpm lint`: exit 0. - `pnpm check:cli-command-ids`: exit 0. `pnpm check:adr-symbol-anchors`: exit 0. - `test:repo` (the `repo` vitest project) not run locally: the diff touches no file in it and the derivation does not name it; CI's `Test Core` runs it. - Full dogfood not run: no `!` in the title and no `(narrowing)` arm. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_016WV4cwJiP1ALmQ1rTttNGc)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_016WV4cwJiP1ALmQ1rTttNGc --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6910797 commit ea41c49

3 files changed

Lines changed: 90 additions & 4 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@objectstack/rest': patch
3+
---
4+
5+
fix(rest): the external-datasource routes relay a declared service outage as `503 SERVICE_UNAVAILABLE`
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
`POST /api/v1/datasources/:name/external/tables/:remote/import` answered a `tenancy` service outage as `400 EXTERNAL_IMPORT_ERROR`, telling the caller the request was malformed. The import reads the effective tenancy posture to decide where the imported object lands, and a registered `tenancy` service that cannot answer throws the `503` / `SERVICE_UNAVAILABLE` envelope there. The route now relays that envelope, as the datasource admin routes and the MCP `import_object` tool already do, so the caller is told to retry. Nothing was saved before and nothing is saved now. The other routes under `/datasources/:name/external/*` share the same catch and relay a declared outage the same way; every other refusal keeps its `400` and code.

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,19 @@ export function registerExternalDatasourceRoutes(
627627
* which told the caller to re-declare a datasource that does not exist. Both
628628
* fields are required, so an unrelated error carrying a stray `status`
629629
* cannot re-route itself.
630+
*
631+
* [#30609] A second relay of the same kind, the one `badRequest` makes
632+
* first: an error carrying the `503` / `SERVICE_UNAVAILABLE` envelope has
633+
* classified itself as a DEPENDENCY OUTAGE, and this family's `400` told the
634+
* caller the request was malformed, the opposite of "retry". The import's
635+
* landing reads the effective tenancy posture, and a registered `tenancy`
636+
* service that cannot answer throws `AuthzStoreUnavailableError` there
637+
* (`classifyAdmissionTenancyPosture`); nothing is saved, so only the status
638+
* was wrong. Read off the envelope, as `badRequest` and the MCP
639+
* `import_object` tool read it, rather than off the authorization brand, so
640+
* every route here relays any service's declared outage the same way: the
641+
* `EXTERNAL_DATASOURCE_ERROR` routes share the catch and the same truth. Both
642+
* fields are required, for the reason above.
630643
*/
631644
const answerThrown = (
632645
res: any,
@@ -640,6 +653,9 @@ export function registerExternalDatasourceRoutes(
640653
return sendError(res, nested.status, nested.code, nested.message, nested.extra);
641654
}
642655
const envelope = err as { code?: unknown; status?: unknown } | null | undefined;
656+
if (envelope?.status === 503 && envelope?.code === 'SERVICE_UNAVAILABLE') {
657+
return sendError(res, 503, 'SERVICE_UNAVAILABLE', (err as Error).message);
658+
}
643659
if (envelope?.status === 404 && envelope?.code === 'RESOURCE_NOT_FOUND') {
644660
return sendError(res, 404, 'RESOURCE_NOT_FOUND', (err as Error).message);
645661
}

‎packages/rest/src/external-datasource-unknown-name.test.ts‎

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,22 @@
2323
* Driven through the real `HonoHttpServer`.
2424
*/
2525

26-
import { describe, it, expect } from 'vitest';
26+
import { describe, it, expect, vi } from 'vitest';
27+
import { classifyAdmissionTenancyPosture } from '@objectstack/core';
2728
import { HonoHttpServer } from '@objectstack/plugin-hono-server';
28-
import { ExternalDatasourceService } from '@objectstack/service-datasource';
29+
import { ExternalDatasourceService, type ExternalDatasourceServiceConfig } from '@objectstack/service-datasource';
2930
import { registerExternalDatasourceRoutes } from './external-datasource-routes.js';
3031

31-
/** `warehouse` exists and is managed: it declares no `schemaMode`. */
32+
/**
33+
* `warehouse` exists and is managed: it declares no `schemaMode`. `lake` is a
34+
* federation source.
35+
*/
3236
const DEFINITIONS: Record<string, { name: string; schemaMode?: 'managed' | 'external' }> = {
3337
warehouse: { name: 'warehouse' },
38+
lake: { name: 'lake', schemaMode: 'external' },
3439
};
3540

36-
function mount() {
41+
function mount(overrides: Partial<ExternalDatasourceServiceConfig> = {}) {
3742
const service = new ExternalDatasourceService({
3843
introspect: async () => {
3944
throw new Error('introspect must not be reached');
@@ -44,6 +49,7 @@ function mount() {
4449
persistObject: async () => {
4550
throw new Error('persistObject must not be reached');
4651
},
52+
...overrides,
4753
});
4854
const server = new HonoHttpServer(0);
4955
registerExternalDatasourceRoutes(
@@ -161,3 +167,57 @@ describe('POST …/external/validate on a name that resolves no saved datasource
161167
});
162168
});
163169
});
170+
171+
/**
172+
* [#30609] The import on a `tenancy` service outage. The import's landing
173+
* reads the effective tenancy posture (`classifyAdmissionTenancyPosture`, the
174+
* read `service-datasource`'s plugin makes), and a registered `tenancy`
175+
* service that cannot answer throws `AuthzStoreUnavailableError` there:
176+
* declared `503` / `SERVICE_UNAVAILABLE`. The catch answered it `400
177+
* EXTERNAL_IMPORT_ERROR`, a malformed request, where the admin routes and the
178+
* MCP `import_object` tool relay the outage. The ruling these pins hold: the
179+
* outage is relayed as `503 SERVICE_UNAVAILABLE` and nothing is saved; a plain
180+
* refusal from the same landing keeps the door's `400` and code.
181+
*/
182+
describe('POST …/external/tables/:remote/import on a tenancy service outage', () => {
183+
/** A `tenancy` service registered through a factory that fails. */
184+
const outageLanding = async () => {
185+
await classifyAdmissionTenancyPosture(() => Promise.reject(new Error('tenancy factory failed')));
186+
throw new Error('the posture read must not answer');
187+
};
188+
189+
it('answers 503 SERVICE_UNAVAILABLE and saves nothing', async () => {
190+
const persistObject = vi.fn(async () => undefined);
191+
const call = mount({ resolveImportLanding: outageLanding, persistObject });
192+
193+
const answer = await call('POST', '/lake/external/tables/orders/import');
194+
195+
expect({
196+
status: answer.status,
197+
success: answer.body.success,
198+
code: answer.body.error?.code,
199+
}).toEqual({ status: 503, success: false, code: 'SERVICE_UNAVAILABLE' });
200+
expect(answer.body.error.message).toContain('(failed read: `tenancy`)');
201+
expect(answer.body).not.toHaveProperty('data');
202+
expect(persistObject).not.toHaveBeenCalled();
203+
});
204+
205+
it('still refuses a plain landing refusal 400 EXTERNAL_IMPORT_ERROR', async () => {
206+
const persistObject = vi.fn(async () => undefined);
207+
const call = mount({
208+
resolveImportLanding: async () => {
209+
throw new Error('no package to be saved into');
210+
},
211+
persistObject,
212+
});
213+
214+
const answer = await call('POST', '/lake/external/tables/orders/import');
215+
216+
expect({ status: answer.status, code: answer.body.error?.code, message: answer.body.error?.message }).toEqual({
217+
status: 400,
218+
code: 'EXTERNAL_IMPORT_ERROR',
219+
message: 'no package to be saved into',
220+
});
221+
expect(persistObject).not.toHaveBeenCalled();
222+
});
223+
});

0 commit comments

Comments
 (0)