diff --git a/.changeset/metadata-store-outage-is-not-a-miss.md b/.changeset/metadata-store-outage-is-not-a-miss.md new file mode 100644 index 0000000000..f4f089581b --- /dev/null +++ b/.changeset/metadata-store-outage-is-not-a-miss.md @@ -0,0 +1,45 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(metadata-protocol): 元数据存储读不到不再被讲成「这一项不存在」(#5532) + +`sys_metadata` 整体不可达时,`GET /api/v1/meta/object/acct` 会回一个「不存在」—— +真相是「读不到」。两个事实的处置方向完全相反(去建一个 / 去修后端),而 Studio、 +Setup 在元数据库故障期就是照前者渲染的:每一个对象都显示成「不存在」。 + +根因在产出方:`getMetaItems` / `getMetaItem` 的四处 customization-overlay 读各自 +裹着一个裸 `catch {}`,注释写着 "DB not available" 然后照 miss 处理。空值一路穿过 +读链,每个消费方给它起了一个不同却同样错的名字: + +- `getMetaItemCached` → `Metadata item / not found` +- `?state=draft` → `NO_DRAFT` / 404「没有待发布的草稿」(发布流程读作「没什么可发的」) +- `getMetaItems` → `items: []`「这个环境一个都没声明」 + +ADR-0110 D3 已经为这件事立过规矩:miss 与 outage 是两个不同的事实、安全含义相反。 +#5108 按这条修掉了 `DatabaseLoader` 的复数读,#5089 修掉了 `listForIndex`;本次是 +同一条规矩在协议自己的 overlay 读上,单数与复数一并覆盖。 + +**改了什么** + +1. **区分按错误类型判定,不按异常猜。** 唯一良性的读失败是「`sys_metadata` 还没被 + 创建」——那时确实没有 overlay 行,落回 registry 就是真相,首次启动也不该爆炸。 + 判定走 `isMissingTableError`,与 `DatabaseLoader`(#5108)、本包 + `SysMetadataRepository`(#4867)同一个谓词,一个驱动怪癖只教给平台一次。其余 + 一律视为故障。 +2. **故障照实上报。** 上抛 `status: 503` / `code: SERVICE_UNAVAILABLE` + (`HttpStatusErrorCodeMap[503]`,ADR-0112 的标准目录码,不新造词汇),驱动原始 + 错误挂在 `cause` 上。REST 层现有的 #5437 / #5464 消毒与日志口原样接住:客户端拿 + 到 503 + code(文案按 5xx 规则withheld),运维在日志里拿到完整的驱动报文。 +3. **终末 not found 结构化。** 真 miss 现在带 `status: 404` / + `code: RESOURCE_NOT_FOUND`。 + +**wire 可见变化**(把错误答案改成对的答案): + +| 场景 | 之前 | 之后 | +|---|---|---| +| 元数据存储不可达 | `404`/`400`/`500` 说「不存在」「没有草稿」「什么都没声明」 | `503` + `SERVICE_UNAVAILABLE`,可重试 | +| 真的没有这一项 | `500` + `INTERNAL_ERROR`(#5489 之前是 `400` 且内部措辞逐字上线) | `404` + `RESOURCE_NOT_FOUND` | + +`sys_metadata` 尚未建表这一路径行为不变:仍旧落回 registry / MetadataService, +真查不到时回结构化 404。 diff --git a/packages/metadata-protocol/src/protocol.metadata-store-outage.test.ts b/packages/metadata-protocol/src/protocol.metadata-store-outage.test.ts new file mode 100644 index 0000000000..98982752c4 --- /dev/null +++ b/packages/metadata-protocol/src/protocol.metadata-store-outage.test.ts @@ -0,0 +1,279 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#5532] A `sys_metadata` read that FAILED is not a metadata item that does +// not exist. +// +// --------------------------------------------------------------------------- +// The defect +// --------------------------------------------------------------------------- +// Every customization-overlay read in `getMetaItems` / `getMetaItem` was +// wrapped in a bare `catch {}` whose comment named the reason it was swallowing +// ("DB not available") and then answered as if the row simply was not there. +// The emptiness travelled the whole read chain unremarked and each consumer +// gave it a different, equally wrong name: +// +// GET /meta/object/acct → "Metadata item object/acct not found" +// GET /meta/object/acct?state=draft→ NO_DRAFT / 404 "no pending draft exists" +// GET /meta/object → `items: []` — "this env declares none" +// +// Measured on `origin/main` before the fix, with an engine whose reads reject +// with `connect ECONNREFUSED 10.0.0.5:5432`: +// +// RESOLVE getMetaItem(econnrefused) -> { type, name, ...no item } +// THROW getMetaItemCached(econnrefused) status=undefined code=undefined +// msg=Metadata item object/acct not found +// THROW getMetaItem(state=draft, …) status=404 code=NO_DRAFT +// RESOLVE getMetaItems(econnrefused) -> { items: [] } +// +// ADR-0110 D3 is the rule those answers break: a miss and an outage are +// different facts with opposite meanings, and the dispositions they call for +// are opposite too — "create it / fix your link" vs. "the backend is down, +// retry". #5108 fixed exactly this in `DatabaseLoader`'s plural read and #5089 +// in `listForIndex`; this is the same rule one layer up, on the protocol's own +// overlay reads, singular and plural. +// +// --------------------------------------------------------------------------- +// The one benign reason, and why the discrimination is by error TYPE +// --------------------------------------------------------------------------- +// `sys_metadata` not provisioned yet: there are then genuinely no overlay rows, +// so falling through to the registry IS the truth and first boot must not +// explode. That is `isMissingTableError` — the same predicate `DatabaseLoader` +// (#5108) and this package's `SysMetadataRepository` (#4867) ask, so a driver +// quirk is taught to the platform once. Everything else is an outage. +// +// --------------------------------------------------------------------------- +// Reverse verification, direction predicted BEFORE running +// --------------------------------------------------------------------------- +// Ordinary red, on both halves, and they fail differently — which is the point: +// +// * Restore `} catch { /* DB not available */ }` at the four overlay reads → +// 7 red / 5 green, and they go red in exactly the shape the issue reported: +// the singular and preview reads RESOLVE with no item, the plural reads +// resolve `{ items: [] }`, the draft read throws 404. (Predicted 6 — the +// six outage cases; the seventh is the miss-vs-outage comparison, whose +// OUTAGE half is one of the same six. Recorded rather than rounded off.) +// * Restore `throw new Error(\`Metadata item …/… not found\`)` → 3 red / +// 9 green: the two "a real miss is a structured 404" cases plus the benign +// first-boot miss, all on `status`/`code` being `undefined`, while every +// 503 case stays GREEN. That separation is deliberate: it is what proves +// the 404 is fix C's own contribution and not an artifact of the outage +// split. +// +// The "benign / working store" describe is the opposite guard — it exists to +// catch the overreach where a fix starts calling first boot, or a plain +// unreferenced item, an outage. + +import { describe, it, expect, vi } from 'vitest'; +import { ErrorCode } from '@objectstack/spec/api'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +/** A registry with nothing in it — the overlay read is the only source. */ +function emptyRegistry(items: Record = {}) { + return { + getObject: () => undefined, + getItem: (_type: string, name: string) => items[name], + listItems: () => [], + applyNavContributions: (x: any) => x, + isPackageDisabled: () => false, + getObjectOwner: () => undefined, + }; +} + +/** + * An engine whose every read REJECTS with `error` — the shape of a metadata + * store the protocol cannot reach. + */ +function engineThatCannotBeRead(error: () => unknown, registryItems: Record = {}) { + const reject = vi.fn(async () => { throw error(); }); + return { + registry: emptyRegistry(registryItems), + find: reject, + findOne: reject, + } as any; +} + +/** An engine that answers reads normally, from `rows`. */ +function engineWithRows(rows: any[] = [], registryItems: Record = {}) { + return { + registry: emptyRegistry(registryItems), + find: vi.fn(async () => rows), + findOne: vi.fn(async () => rows[0] ?? null), + } as any; +} + +/** The real driver phrasings for "the table has not been provisioned yet". */ +const missingTable = () => + Object.assign(new Error('SQLITE_ERROR: no such table: sys_metadata'), { code: 'SQLITE_ERROR' }); + +/** An outage: the rows may well exist and simply were not seen. */ +const connectionRefused = () => + Object.assign(new Error('connect ECONNREFUSED 10.0.0.5:5432'), { code: 'ECONNREFUSED' }); + +/** Capture a rejection without letting a resolve pass silently. */ +async function rejection(run: () => Promise): Promise { + let caught: any; + let resolved: unknown; + let didResolve = false; + try { + resolved = await run(); + didResolve = true; + } catch (e) { + caught = e; + } + expect( + didResolve, + `expected a rejection, but the call resolved with ${JSON.stringify(resolved)}`, + ).toBe(false); + return caught; +} + +/** Every assertion the outage envelope owes a caller. */ +function expectStoreUnavailable(caught: any, cause: unknown) { + expect(caught?.status).toBe(503); + expect(caught?.code).toBe('SERVICE_UNAVAILABLE'); + // ADR-0112: the wire code must be in the declared vocabulary, or the + // envelope fails `ApiErrorSchema.parse` at the boundary that ships it. + expect(ErrorCode.safeParse(caught?.code).success).toBe(true); + // The words a client reads say "unknown", never "does not exist". + expect(caught.message).toContain('unknown'); + expect(caught.message.toLowerCase()).not.toContain('not found'); + // The driver's own error is not lost — it rides as `cause`, which is what + // `logWithheldServerFault` prints for the operator (#5437). + expect(caught.cause).toBe(cause); +} + +describe('[#5532] an unreadable sys_metadata is a 503, not "that item does not exist"', () => { + it('the singular active read no longer answers a miss it never verified', async () => { + const err = connectionRefused(); + const p = new ObjectStackProtocolImplementation(engineThatCannotBeRead(() => err)); + + const caught = await rejection(() => p.getMetaItem({ type: 'object', name: 'acct' } as any)); + expectStoreUnavailable(caught, err); + }); + + it('getMetaItemCached propagates the outage instead of relabelling it "not found"', async () => { + const err = connectionRefused(); + const p = new ObjectStackProtocolImplementation(engineThatCannotBeRead(() => err)); + + const caught = await rejection(() => p.getMetaItemCached({ type: 'object', name: 'acct' } as any)); + expectStoreUnavailable(caught, err); + // The regression this replaces, verbatim. + expect(caught.message).not.toContain('Metadata item object/acct not found'); + }); + + it('the draft read stops reporting an outage as "there is no pending draft"', async () => { + const err = connectionRefused(); + const p = new ObjectStackProtocolImplementation(engineThatCannotBeRead(() => err)); + + const caught = await rejection( + () => p.getMetaItem({ type: 'object', name: 'acct', state: 'draft' } as any), + ); + expectStoreUnavailable(caught, err); + // NO_DRAFT is a lifecycle fact ("nobody is editing this"). A publish + // flow reads it as "nothing to publish" and moves on. + expect(caught.code).not.toBe('NO_DRAFT'); + }); + + it('the ?preview=draft overlay stops silently serving the published world', async () => { + const err = connectionRefused(); + const p = new ObjectStackProtocolImplementation(engineThatCannotBeRead(() => err)); + + const caught = await rejection( + () => p.getMetaItem({ type: 'object', name: 'acct', previewDrafts: true } as any), + ); + expectStoreUnavailable(caught, err); + }); + + it('the PLURAL read stops answering "this environment declares none of these"', async () => { + const err = connectionRefused(); + const p = new ObjectStackProtocolImplementation(engineThatCannotBeRead(() => err)); + + const caught = await rejection(() => p.getMetaItems({ type: 'object' } as any)); + expectStoreUnavailable(caught, err); + }); + + it('the plural draft-preview overlay is held to the same rule', async () => { + const err = connectionRefused(); + // The active overlay read must succeed so control actually reaches the + // draft-preview block: only its own read fails. + const engine = engineWithRows([]); + let call = 0; + engine.find = vi.fn(async (_o: string, opts: any) => { + call += 1; + if (opts?.where?.state === 'draft') throw err; + return []; + }); + + const p = new ObjectStackProtocolImplementation(engine); + const caught = await rejection( + () => p.getMetaItems({ type: 'object', previewDrafts: true } as any), + ); + expectStoreUnavailable(caught, err); + expect(call).toBeGreaterThan(1); // the active read really did run first + }); +}); + +describe('[#5532 / fix C] a REAL miss is a structured 404, not an unattributable throw', () => { + it('getMetaItemCached carries status 404 + the catalog code', async () => { + const p = new ObjectStackProtocolImplementation(engineWithRows([])); + + const caught = await rejection(() => p.getMetaItemCached({ type: 'object', name: 'ghost' } as any)); + expect(caught.status).toBe(404); + expect(caught.code).toBe('RESOURCE_NOT_FOUND'); + expect(ErrorCode.safeParse(caught.code).success).toBe(true); + expect(caught.message).toBe('Metadata item object/ghost not found'); + }); + + it('is distinguishable from the outage by code alone — which is the whole point', async () => { + const missP = new ObjectStackProtocolImplementation(engineWithRows([])); + const outageP = new ObjectStackProtocolImplementation( + engineThatCannotBeRead(connectionRefused), + ); + + const miss = await rejection(() => missP.getMetaItemCached({ type: 'object', name: 'ghost' } as any)); + const outage = await rejection(() => outageP.getMetaItemCached({ type: 'object', name: 'ghost' } as any)); + + expect([miss.status, miss.code]).toEqual([404, 'RESOURCE_NOT_FOUND']); + expect([outage.status, outage.code]).toEqual([503, 'SERVICE_UNAVAILABLE']); + }); +}); + +describe('[#5532] the benign case and the healthy case are untouched', () => { + it('an unprovisioned sys_metadata still falls through to the registry', async () => { + // First boot: the table does not exist, so "no overlay row" IS the + // truth and the code-authored item must still be served. + const p = new ObjectStackProtocolImplementation( + engineThatCannotBeRead(missingTable, { acct: { name: 'acct', label: 'Account' } }), + ); + + const res: any = await p.getMetaItem({ type: 'object', name: 'acct' } as any); + expect(res.item?.name).toBe('acct'); + expect(res.item?.label).toBe('Account'); + }); + + it('an unprovisioned sys_metadata + nothing anywhere is a 404 miss, not a 503', async () => { + const p = new ObjectStackProtocolImplementation(engineThatCannotBeRead(missingTable)); + + const caught = await rejection(() => p.getMetaItemCached({ type: 'object', name: 'acct' } as any)); + expect(caught.status).toBe(404); + expect(caught.code).toBe('RESOURCE_NOT_FOUND'); + }); + + it('an unprovisioned sys_metadata still lists the registry items (plural)', async () => { + const p = new ObjectStackProtocolImplementation(engineThatCannotBeRead(missingTable)); + + const res: any = await p.getMetaItems({ type: 'object' } as any); + expect(res.items).toEqual([]); + }); + + it('a healthy store still serves the overlay row it holds', async () => { + const p = new ObjectStackProtocolImplementation( + engineWithRows([ + { type: 'object', name: 'acct', state: 'active', metadata: JSON.stringify({ name: 'acct', label: 'Overlaid' }) }, + ]), + ); + + const res: any = await p.getMetaItem({ type: 'object', name: 'acct' } as any); + expect(res.item?.label).toBe('Overlaid'); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index a06020aeba..40aa03d738 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -9,6 +9,10 @@ import type { MetadataHostEngine } from './host-engine.js'; import { evaluateRuntimeAuthoringGate } from './runtime-authoring-gate.js'; import { SysMetadataRepository, type SysMetadataEngine } from './sys-metadata-repository.js'; import { ConflictError, assertProtocolCompat, type MetadataItem } from '@objectstack/metadata-core'; +// [#5532] One vocabulary of "which driver read errors are benign", shared with +// `sys-metadata-repository.ts` in this package and with `DatabaseLoader` in +// `@objectstack/metadata` (#5108). See `rethrowUnlessMetadataStoreUnprovisioned`. +import { isMissingTableError } from '@objectstack/metadata/errors'; import type { BatchUpdateRequest, BatchUpdateResponse, @@ -1118,6 +1122,71 @@ function rowRequiredIdError(operation: 'update' | 'delete'): Error { return err; } +/** + * [#5532] The client-facing sentence for "the `sys_metadata` overlay read + * failed, so I do not know whether this item exists". + * + * Deliberately does NOT interpolate the driver's own message. #5437 records + * why: the REST boundary drops a 5xx's prose unconditionally, and the two + * write-side 500s in this very file (`Failed to persist customization overlay + * to sys_metadata: ${dbError.message}`) are the specimen that made it drop — + * a driver line is nowhere near the length bound, so it arrived intact. The + * driver error still reaches the operator: it rides as `cause` on the thrown + * error, and `handleRouteError` / `logWithheldServerFault` print the whole + * object. + */ +const METADATA_STORE_UNAVAILABLE_MESSAGE = + 'The metadata store could not be read, so whether this item exists is unknown. ' + + 'Retry once the metadata database is reachable.'; + +/** + * [#5532] A `sys_metadata` READ that failed for a reason that is NOT "the table + * has not been provisioned yet" — i.e. the rows may well exist and simply were + * not seen. + * + * 503, not 500: nothing about the REQUEST is wrong, the condition is a + * dependency outage that may clear, and a caller/proxy SHOULD retry. That is + * the same verdict `mapDataError` already gives `ERR_DATASOURCE_UNAVAILABLE`. + * `SERVICE_UNAVAILABLE` is the standard catalog's own code for 503 + * (`HttpStatusErrorCodeMap[503]`, ADR-0112) — a catalogued code rather than an + * invented string, and no new ledger vocabulary for a distinction no consumer + * measures today. + */ +function metadataStoreUnavailableError(cause: unknown): Error { + const err = new Error(METADATA_STORE_UNAVAILABLE_MESSAGE) as Error & { + code?: string; + status?: number; + cause?: unknown; + }; + err.code = 'SERVICE_UNAVAILABLE'; + err.status = 503; + err.cause = cause; + return err; +} + +/** + * [#5532] The terminal "this metadata item does not exist" — structured, so it + * stops falling out of `mapDataError`'s catch-all. + * + * A miss is a 404 with the catalog's own not-found code + * (`HttpStatusErrorCodeMap[404] === 'RESOURCE_NOT_FOUND'`, and the spelling + * `GET /meta/:type/:name` already emits from its app-visibility gate). Before + * this, the throw was a bare `Error`: no `status`, no `code`, so it reached the + * REST boundary's terminal branch and was answered — verbatim as a 400 before + * #5489, as a sanitised `500 INTERNAL_ERROR` after it. Both are wrong answers + * for a plain miss, in opposite directions; neither could be told apart from a + * genuine fault by any client. + */ +function metadataItemNotFoundError(type: string, name: string): Error { + const err = new Error(`Metadata item ${type}/${name} not found`) as Error & { + code?: string; + status?: number; + }; + err.code = 'RESOURCE_NOT_FOUND'; + err.status = 404; + return err; +} + /** What one pass of the `batchData` record loop produced (ADR-0119 D4). */ type BatchDataLoopOutcome = { results: BatchDataRowResult[]; succeeded: number; failed: number }; @@ -2984,6 +3053,57 @@ export class ObjectStackProtocolImplementation implements }; } + /** + * [#5532] Decide what a failed READ against `sys_metadata` means, and + * rethrow unless it is the ONE benign reason. + * + * ## The defect + * + * Every overlay read in `getMetaItems`/`getMetaItem` used to `catch {}` into + * its own empty value — `items` left as-is, `item` left `undefined`, the + * draft lookup falling through to the active read. That made a metadata + * store the protocol cannot reach **indistinguishable** from an environment + * where the item was never customised, and the emptiness then travelled + * the whole read chain unremarked: + * + * - `getMetaItemCached` turned it into `not found` — an outage answered + * as "that item does not exist", which is the opposite disposition + * (retry the backend vs. create the item / fix the link); + * - the `state='draft'` read turned it into `NO_DRAFT` / 404 — an outage + * answered as "there is no pending edit", which a publish flow reads as + * "nothing to publish"; + * - `getMetaItems` turned it into `items: []` — an outage answered as + * "this environment declares none of these", the exact shape #5108 fixed + * one layer down in `DatabaseLoader` and #5089 in `listForIndex`. + * + * ADR-0110 D3 is the rule: a miss and an outage are different facts with + * opposite meanings, and a consumer must never read one as the other. + * + * ## The one benign reason + * + * `sys_metadata` has not been provisioned yet. There are then genuinely no + * overlay rows, so falling through to the registry / MetadataService IS the + * truth, and a first boot must not explode. Classification is by error TYPE + * through {@link isMissingTableError} — the same predicate `DatabaseLoader` + * (#5108) and this package's own `SysMetadataRepository` (#4867) ask, so a + * driver quirk is taught to the platform once. Conservative in the same + * direction: an unrecognised error is NOT benign, because a false "benign" + * silently mis-answers "does this exist?" while a false "real" costs one + * 503 the caller can retry. + * + * @throws {@link metadataStoreUnavailableError} — a 503 carrying the driver + * error as `cause`. Not the driver error itself: unwrapped, it has + * no status, so the REST boundary would have to guess from the + * message text — and `mapDataError` guesses `no such table` into + * `404 OBJECT_NOT_FOUND`, i.e. straight back into a miss. + * @returns normally ONLY for the benign case, licensing the caller to treat + * the overlay as absent. + */ + private rethrowUnlessMetadataStoreUnprovisioned(error: unknown): void { + if (isMissingTableError(error)) return; + throw metadataStoreUnavailableError(error); + } + async getMetaItems(request: { type: string; packageId?: string; organizationId?: string; previewDrafts?: boolean }) { // #4432 — CANONICAL TYPE KEY. See {@link canonicalMetaType}. This one // is load-bearing twice over: the SchemaRegistry indexes code-authored @@ -3114,8 +3234,12 @@ export class ObjectStackProtocolImplementation implements } } } - } catch { - // DB not available — fall through with whatever we already have. + } catch (error) { + // [#5532] Only "sys_metadata not provisioned yet" licenses us to + // answer with whatever we already have. Any other read failure + // means overlay rows may exist and were not seen — serving the + // registry-only set would report them as never declared. + this.rethrowUnlessMetadataStoreUnprovisioned(error); } // ADR-0033 draft-overlay preview: when the caller opts in (admin-gated @@ -3166,8 +3290,12 @@ export class ObjectStackProtocolImplementation implements return data; }); } - } catch { - // DB unavailable — serve the active result unchanged. + } catch (error) { + // [#5532] Same rule as the active-overlay read above. Serving + // the active result "unchanged" is a lie to a caller that asked + // for a draft preview: it renders the published world while the + // pending edits it asked to see were never read. + this.rethrowUnlessMetadataStoreUnprovisioned(error); } } @@ -3338,8 +3466,11 @@ export class ObjectStackProtocolImplementation implements } return { type: request.type, name: request.name, item: decorateMetadataItem(request.type, draftItem) }; } - } catch { - // DB unavailable — fall through to the active read. + } catch (error) { + // [#5532] Falling through to the active read here would answer + // "there is no draft for this item" from a read that never + // reached the table the drafts live in. + this.rethrowUnlessMetadataStoreUnprovisioned(error); } } @@ -3398,8 +3529,13 @@ export class ObjectStackProtocolImplementation implements (item as any)._packageId = recPkg; } } - } catch { - // DB not available — fall through to registry / MetadataService + } catch (error) { + // [#5532] THE site this issue was raised on. Falling through to the + // registry / MetadataService with `item` still `undefined` is what + // let a storage outage arrive at the client as `not found` (active + // read) or `NO_DRAFT` (draft read) — both of them claims about + // authorship, made from a read that never happened. + this.rethrowUnlessMetadataStoreUnprovisioned(error); } // Draft reads stop here — they intentionally do NOT fall through @@ -5677,7 +5813,13 @@ export class ObjectStackProtocolImplementation implements const item = (result as any)?.item; if (!item) { - throw new Error(`Metadata item ${request.type}/${request.name} not found`); + // [#5532] Structured: 404 + the catalog's `RESOURCE_NOT_FOUND`. + // Reaching here now means a real miss — `getMetaItem` throws + // 503 rather than answering `undefined` when the store could + // not be read (see + // {@link rethrowUnlessMetadataStoreUnprovisioned}) — so the + // 404 is a claim this layer is finally entitled to make. + throw metadataItemNotFoundError(request.type, request.name); } // Calculate ETag (simple hash of the stringified metadata). diff --git a/packages/objectql/src/protocol-meta.test.ts b/packages/objectql/src/protocol-meta.test.ts index 075fbadbac..db23269a72 100644 --- a/packages/objectql/src/protocol-meta.test.ts +++ b/packages/objectql/src/protocol-meta.test.ts @@ -628,12 +628,33 @@ describe('ObjectStackProtocolImplementation - Metadata Persistence', () => { expect(result.item).toBeUndefined(); }); - it('should handle DB errors gracefully and return undefined item', async () => { - mockEngine.findOne.mockRejectedValue(new Error('DB down')); + // [#5532] REPLACED, not re-spelled. This slot used to read "should + // handle DB errors gracefully and return undefined item": it fed + // `new Error('DB down')` to the overlay read and asserted the answer + // was an item-shaped `undefined`. That assertion passed because the + // protocol swallowed the failure, and the emptiness then became + // "Metadata item app/test_app not found" at `getMetaItemCached` — + // an outage answered as a miss, which ADR-0110 D3 forbids and #5108 + // already fixed one layer down. "Gracefully" now means the ONE benign + // reason; everything else is reported. + it('an unreadable store is REPORTED, not answered as an absent item (#5532)', async () => { + const outage = new Error('DB down'); + mockEngine.findOne.mockRejectedValue(outage); + + await expect(protocol.getMetaItem({ type: 'app', name: 'test_app' })) + .rejects.toMatchObject({ status: 503, code: 'SERVICE_UNAVAILABLE' }); + }); + + it('an UNPROVISIONED sys_metadata still degrades gracefully (#5532)', async () => { + // The one benign read failure: no table means genuinely no overlay + // rows, so the registry answer is the truth and first boot must not + // explode. This is the half of the old test that was right. + registry.registerItem('app', sampleApp, 'name' as any); + mockEngine.findOne.mockRejectedValue(new Error('SQLITE_ERROR: no such table: sys_metadata')); const result = await protocol.getMetaItem({ type: 'app', name: 'test_app' }); - expect(result.item).toBeUndefined(); + expect(result.item).toMatchObject(sampleApp); expect(result.type).toBe('app'); expect(result.name).toBe('test_app'); }); @@ -1026,12 +1047,25 @@ describe('ObjectStackProtocolImplementation - Metadata Persistence', () => { expect(result.items).toHaveLength(0); }); - it('should handle DB errors gracefully and return empty items', async () => { + // [#5532] REPLACED — the plural twin of the singular case above. The old + // slot ("should handle DB errors gracefully and return empty items") + // asserted that an unreadable store lists nothing, which is the exact + // answer #5089/#5108 call dangerous: every consumer that gates on a + // DECLARED SET reads "zero declarations" as "the author declared none". + it('an unreadable store is REPORTED, not listed as an empty type (#5532)', async () => { mockEngine.find.mockRejectedValue(new Error('DB down')); + await expect(protocol.getMetaItems({ type: 'app' })) + .rejects.toMatchObject({ status: 503, code: 'SERVICE_UNAVAILABLE' }); + }); + + it('an UNPROVISIONED sys_metadata still lists what the registry holds (#5532)', async () => { + registry.registerItem('app', sampleApp, 'name' as any); + mockEngine.find.mockRejectedValue(new Error('SQLITE_ERROR: no such table: sys_metadata')); + const result = await protocol.getMetaItems({ type: 'app' }); - expect(result.items).toHaveLength(0); + expect(result.items).toHaveLength(1); expect(result.type).toBe('app'); }); diff --git a/packages/rest/src/rest-meta-outage-vs-miss.test.ts b/packages/rest/src/rest-meta-outage-vs-miss.test.ts new file mode 100644 index 0000000000..c681276cb0 --- /dev/null +++ b/packages/rest/src/rest-meta-outage-vs-miss.test.ts @@ -0,0 +1,183 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#5532] What `GET /meta/:type/:name` puts on the wire once the PRODUCER stops +// conflating "the metadata store could not be read" with "that item does not +// exist". +// +// This file changes NOTHING in `packages/rest`. The sanitizing and logging +// boundary (#5437 / #5464 / #5489) was already correct; the defect was upstream +// in `@objectstack/metadata-protocol`, which handed this layer an error whose +// truth had already been destroyed. These are the assertions that prove the +// receiving half really does the right thing with the two envelopes the +// producer now emits — the "verify with the rest side's existing behaviour" +// half of the fix, and the regression net if either envelope drifts. +// +// The protocol is a stub here on purpose: it lets the two envelopes be stated +// literally, side by side, instead of being reconstructed through a real +// engine. The producer's own end is pinned in +// `@objectstack/metadata-protocol`'s `protocol.metadata-store-outage.test.ts`. + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { INTERNAL_ERROR_MESSAGE } from '@objectstack/types'; +import { RestServer } from './rest-server'; + +const META_ITEM = '/api/v1/meta/:type/:name'; + +function createMockServer() { + return { + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), use: vi.fn(), + listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined), + }; +} + +function makeRes() { + const res: any = { statusCode: 200, body: undefined }; + res.status = vi.fn((c: number) => { res.statusCode = c; return res; }); + res.json = vi.fn((b: any) => { res.body = b; return res; }); + res.header = vi.fn(() => res); + res.setHeader = vi.fn(); res.write = vi.fn(); res.end = vi.fn(); res.send = vi.fn(); + return res; +} + +function setup(protocolOverrides: Record = {}) { + const protocol: any = { + getDiscovery: vi.fn().mockResolvedValue({ + version: 'v0', endpoints: { data: '', metadata: '', ui: '', auth: '/auth' }, + }), + getMetaTypes: vi.fn().mockResolvedValue([]), + getMetaItems: vi.fn().mockResolvedValue([]), + getMetaItem: vi.fn().mockResolvedValue({}), + findData: vi.fn().mockResolvedValue([]), + ...protocolOverrides, + }; + const rest = new RestServer( + createMockServer() as any, + protocol, + { api: { requireAuth: false } } as any, + ); + (rest as any).resolveExecCtx = async () => ({ userId: 'u1' }); + rest.registerRoutes(); + return { rest, protocol }; +} + +async function callMetaItem(rest: any, params: any) { + const res = makeRes(); + const route = rest.getRoutes().find((r: any) => r.method === 'GET' && r.path === META_ITEM); + if (!route) throw new Error(`GET ${META_ITEM} route not registered`); + await route.handler({ method: 'GET', params, query: {}, headers: {} }, res); + return res; +} + +/** The producer's outage envelope (metadata-protocol `getMetaItem`). */ +function storeUnavailable() { + return Object.assign( + new Error( + 'The metadata store could not be read, so whether this item exists is unknown. ' + + 'Retry once the metadata database is reachable.', + ), + { + code: 'SERVICE_UNAVAILABLE', + status: 503, + cause: new Error('connect ECONNREFUSED 10.0.0.5:5432'), + }, + ); +} + +/** The producer's miss envelope (metadata-protocol `getMetaItemCached`). */ +function itemNotFound() { + return Object.assign(new Error('Metadata item object/acct not found'), { + code: 'RESOURCE_NOT_FOUND', + status: 404, + }); +} + +let errorSpy: ReturnType; +beforeEach(() => { errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); }); +afterEach(() => { errorSpy.mockRestore(); }); + +const loggedText = () => errorSpy.mock.calls.map((c) => JSON.stringify(c.map(String))).join('\n'); + +describe('[#5532] an unreadable metadata store reaches the client as a retryable 503', () => { + it('503 + SERVICE_UNAVAILABLE, and the prose is withheld', async () => { + const { rest } = setup({ getMetaItem: vi.fn().mockRejectedValue(storeUnavailable()) }); + + const res = await callMetaItem(rest, { type: 'object', name: 'acct' }); + + // `resolveErrorResponse`'s declared-status passthrough keeps the + // producer's 503 and its `code`, and drops the sentence (#5437). The + // code is what a client branches on, and 503 is what tells an SDK / + // proxy this is worth retrying. + expect(res.statusCode).toBe(503); + expect(res.body).toEqual({ error: INTERNAL_ERROR_MESSAGE, code: 'SERVICE_UNAVAILABLE' }); + }, 60_000); + + it('the client is never told the item does not exist', async () => { + const { rest } = setup({ getMetaItem: vi.fn().mockRejectedValue(storeUnavailable()) }); + + const res = await callMetaItem(rest, { type: 'object', name: 'acct' }); + + // The regression, in the words it used to ship: before this fix the + // producer swallowed the driver error and the wire said "not found" + // (400, verbatim, pre-#5489) or `500 INTERNAL_ERROR` (post-#5489). + // Either way a console rendered "this object does not exist" during a + // metadata-plane outage. + const body = JSON.stringify(res.body).toLowerCase(); + expect(body).not.toContain('not found'); + expect(res.body.code).not.toBe('RESOURCE_NOT_FOUND'); + expect(res.statusCode).not.toBe(404); + }, 60_000); + + it('the operator still gets the driver error, cause chain and all', async () => { + const { rest } = setup({ getMetaItem: vi.fn().mockRejectedValue(storeUnavailable()) }); + + await callMetaItem(rest, { type: 'object', name: 'acct' }); + + // 503 is an *expected* lifecycle status, so `handleRouteError` does not + // print "[REST] Unhandled error" — `logWithheldServerFault` prints the + // withheld message instead (#5437). Withholding is only free of cost + // while the words are still somewhere an operator can find them, and + // "the metadata database is unreachable" is precisely the fault that + // must be diagnosable. + expect(loggedText()).toContain('5xx message withheld'); + expect(loggedText()).toContain('metadata store could not be read'); + }, 60_000); +}); + +describe('[#5532 / fix C] a real miss reaches the client as a coded 404', () => { + it('404 + RESOURCE_NOT_FOUND, with the caller-facing message intact', async () => { + const { rest } = setup({ getMetaItem: vi.fn().mockRejectedValue(itemNotFound()) }); + + const res = await callMetaItem(rest, { type: 'object', name: 'acct' }); + + expect(res.statusCode).toBe(404); + expect(res.body.code).toBe('RESOURCE_NOT_FOUND'); + expect(res.body.error).toBe('Metadata item object/acct not found'); + }, 60_000); + + it('and is NOT logged as an unhandled fault — a miss is a normal outcome', async () => { + const { rest } = setup({ getMetaItem: vi.fn().mockRejectedValue(itemNotFound()) }); + + await callMetaItem(rest, { type: 'object', name: 'acct' }); + + // `isExpectedDataStatus(404)` is true, so the stack trace that used to + // accompany every stale Studio link stops printing. + expect(loggedText()).not.toContain('[REST] Unhandled error'); + }, 60_000); + + it('the un-coded shape this replaces would have been an unattributable 500', async () => { + // Documents WHY fix C was needed rather than asserting today's code: + // the bare `new Error('Metadata item …/… not found')` the producer used + // to throw carries neither `status` nor `code`, so it matched no branch + // and fell out of `mapDataError`'s terminal `UNCLASSIFIED_FAULT` + // (#5489). A plain miss answered as a server fault — and, before + // #5489, as a 400 shipping the internal wording verbatim. + const { rest } = setup({ + getMetaItem: vi.fn().mockRejectedValue(new Error('Metadata item object/acct not found')), + }); + + const res = await callMetaItem(rest, { type: 'object', name: 'acct' }); + + expect(res.statusCode).toBe(500); + expect(res.body).toEqual({ error: INTERNAL_ERROR_MESSAGE, code: 'INTERNAL_ERROR' }); + }, 60_000); +});