diff --git a/.changeset/database-loader-read-outage-is-not-a-miss.md b/.changeset/database-loader-read-outage-is-not-a-miss.md new file mode 100644 index 0000000000..b81c3b875d --- /dev/null +++ b/.changeset/database-loader-read-outage-is-not-a-miss.md @@ -0,0 +1,39 @@ +--- +"@objectstack/metadata": patch +--- + +fix(metadata): `DatabaseLoader` 的读故障不再被吞成「什么都没声明」(#5108) + +`DatabaseLoader` 的五个读方法此前都把**任何**存储异常 `catch {}` 成各自的空值 —— +`load` → `null`、`loadMany` → `[]`、`exists` → `false`、`stat` → `null`、 +`list` → `[]`。于是 `sys_metadata` 所在库不可达时,`loadMany('permission')` 与 +「这个环境一条 permission 都没声明」返回**完全一样的值**,而且异常是在 loader 内部 +就被抹掉的:`MetadataManager` 那几个 `try/catch` 降级分支拿到的是一次「成功的空读」, +根本不会触发,整条链上没有任何一处会说出「读失败了」。 + +现在按**错误类型**判决(#4632 立的规矩,#4728 / #4825 已经在同一个文件里用过两次的 +形状,判据复用现成的 `isMissingTableError`): + +- 唯一良性的失败原因是 `sys_metadata` 尚未 provisioned —— 那时确实没有行, + 「什么都没声明」就是事实,首次启动照旧返回空值、不报错、不缓存; +- 其余全部原因(连接断开、超时、权限不足、查询出错)意味着行还在、只是这次没读到, + 一律把驱动原始异常**原样抛出**,由调用方决定降级姿态。判据保守:无法正面识别为 + 「表不存在」的错误一律当作真故障。 + +由此上层三个已有的机制第一次真的生效: + +- `MetadataManager.list()` 的降级分支会真的进,并且**升级到 `error`** + (AGENTS.md「Degradation log levels」:系统看着正常、它声称掌握的清单其实是残缺的), + 日志写明后果与修法,每次故障只说一次、恢复时再说一次;`list()` 仍然尽力返回可读 + loader 的内容 —— 这个 best-effort 姿态是刻意保留的。兄弟方法 + `MetadataManager.loadMany()` 的同一条缝走同一个判决,不让同一次故障在同一个文件里 + 报出两个级别; +- `MetadataManager.loadDiagnosed()`(ADR-0110 D3)对 `DatabaseLoader` 终于能报出 + `degraded` / `errors`,而不是把 outage 报成 miss; +- `listForIndex()` / `matchEndpoint`(#5089)契约要求「读不到存储必须抛出,不得伪装成 + miss(miss 会变成 404)」—— 这条此前对 `MemoryLoader` / `RemoteLoader` 有效、对 + `DatabaseLoader` 无效,现在对真实的 datasource loader 也成立了。 + +**行为变化**:`MetadataManager.exists()` 与 `listNames()` 本来就没有 `try/catch`, +所以存储故障现在会从它们抛出,而不再静默答「不存在」/「空清单」。这正是本次修复要的 +姿态 —— 可用性故障不是一次「没有」。 diff --git a/packages/metadata/src/loaders/database-loader.test.ts b/packages/metadata/src/loaders/database-loader.test.ts index b64a6c3847..1579c1d23c 100644 --- a/packages/metadata/src/loaders/database-loader.test.ts +++ b/packages/metadata/src/loaders/database-loader.test.ts @@ -6,14 +6,18 @@ import type { IDataDriver } from '@objectstack/spec/contracts'; import { MetadataManager } from '../metadata-manager'; import { MemoryLoader } from './memory-loader'; -// Suppress logger output during tests +// Suppress logger output during tests. Stable object (not a fresh one per +// `createLogger()` call) so the #5108 block can assert on what `list()` says +// when a loader cannot be read — the whole point of that fix is the log line. +const logger = vi.hoisted(() => ({ + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), +})); + vi.mock('@objectstack/core', () => ({ - createLogger: () => ({ - info: vi.fn(), - warn: vi.fn(), - error: vi.fn(), - debug: vi.fn(), - }), + createLogger: () => logger, })); /** @@ -378,46 +382,49 @@ describe('DatabaseLoader', () => { }); describe('error handling', () => { - it('should return null data on load failure', async () => { + // [#5108] These five used to assert the opposite — that a read failure is + // answered with the method's empty value. That behaviour WAS the defect: + // it made an unreachable `sys_metadata` byte-identical to "nothing of this + // type was ever declared". The read seam now discriminates by error type + // (see the dedicated #5108 block below for the benign half). + it('should rethrow a read failure from load — an outage is not a miss', async () => { const failingDriver = createMockDriver(); failingDriver.findOne = vi.fn().mockRejectedValue(new Error('DB error')); const failLoader = new DatabaseLoader({ driver: failingDriver }); - const result = await failLoader.load('object', 'account'); - expect(result.data).toBeNull(); + await expect(failLoader.load('object', 'account')).rejects.toThrow('DB error'); }); - it('should return empty array on loadMany failure', async () => { + it('should rethrow a read failure from loadMany', async () => { const failingDriver = createMockDriver(); failingDriver.find = vi.fn().mockRejectedValue(new Error('DB error')); const failLoader = new DatabaseLoader({ driver: failingDriver }); - const result = await failLoader.loadMany('object'); - expect(result).toEqual([]); + await expect(failLoader.loadMany('object')).rejects.toThrow('DB error'); }); - it('should return false on exists failure', async () => { + it('should rethrow a read failure from exists', async () => { const failingDriver = createMockDriver(); failingDriver.count = vi.fn().mockRejectedValue(new Error('DB error')); const failLoader = new DatabaseLoader({ driver: failingDriver }); - expect(await failLoader.exists('object', 'account')).toBe(false); + await expect(failLoader.exists('object', 'account')).rejects.toThrow('DB error'); }); - it('should return null on stat failure', async () => { + it('should rethrow a read failure from stat', async () => { const failingDriver = createMockDriver(); failingDriver.findOne = vi.fn().mockRejectedValue(new Error('DB error')); const failLoader = new DatabaseLoader({ driver: failingDriver }); - expect(await failLoader.stat('object', 'account')).toBeNull(); + await expect(failLoader.stat('object', 'account')).rejects.toThrow('DB error'); }); - it('should return empty array on list failure', async () => { + it('should rethrow a read failure from list', async () => { const failingDriver = createMockDriver(); failingDriver.find = vi.fn().mockRejectedValue(new Error('DB error')); const failLoader = new DatabaseLoader({ driver: failingDriver }); - expect(await failLoader.list('object')).toEqual([]); + await expect(failLoader.list('object')).rejects.toThrow('DB error'); }); it('should throw descriptive error on save failure', async () => { @@ -836,6 +843,263 @@ describe('DatabaseLoader event_seq on a failed history read (#4825)', () => { }); }); +// ---------- A storage read failure is never answered as "nothing declared" ---------- + +/** + * #5108 (rule: #4632; same shape one layer up from #4825, ADR-0110 D3). + * + * Every read method used to `catch {}` into its own empty value, so a + * `sys_metadata` the metadata plane could not reach returned EXACTLY what "this + * environment declares nothing of that type" returns — `[]`, `false`, `null` — + * with not one line logged anywhere on the chain. `MetadataManager`'s own + * degradation branches could not fire either, because the loader handed them a + * *successful* empty read rather than an exception. + * + * Both directions are pinned, deliberately, exactly as #4728/#4825 pin theirs: + * proving the outage is loud is not enough, because "always throw" would pass + * that alone while making a first boot against an unprovisioned table explode. + * The point is that the two are DISTINGUISHED. + */ +describe('DatabaseLoader read failures are outages, not misses (#5108)', () => { + /** Benign: nothing provisioned yet, so "nothing declared" really is true. */ + const noSuchTable = () => + Object.assign(new Error('no such table: sys_metadata'), { code: 'SQLITE_ERROR' }); + + /** NOT benign: the rows are there, this read simply did not see them. */ + const connectionReset = () => + Object.assign(new Error('read ECONNRESET'), { code: 'ECONNRESET' }); + + /** + * A driver whose reads of `sys_metadata` fail the way a real outage does. + * `syncSchema` still succeeds — the table was provisioned at boot and the + * datasource fell over afterwards, which is what makes the defect invisible. + */ + function driverWithFailingReads(makeError: () => unknown): IDataDriver { + const driver = createMockDriver(); + driver.find = vi.fn().mockImplementation(() => Promise.reject(makeError())); + driver.findOne = vi.fn().mockImplementation(() => Promise.reject(makeError())); + driver.count = vi.fn().mockImplementation(() => Promise.reject(makeError())); + return driver; + } + + describe('a REAL outage — the driver is reachable but the read failed', () => { + let loader: DatabaseLoader; + + beforeEach(() => { + loader = new DatabaseLoader({ driver: driverWithFailingReads(connectionReset) }); + }); + + it('loadMany rethrows instead of answering []', async () => { + await expect(loader.loadMany('permission')).rejects.toThrow('read ECONNRESET'); + }); + + it('exists rethrows instead of answering false', async () => { + await expect(loader.exists('permission', 'admin_all')).rejects.toThrow('read ECONNRESET'); + }); + + it('stat rethrows instead of answering null', async () => { + await expect(loader.stat('permission', 'admin_all')).rejects.toThrow('read ECONNRESET'); + }); + + it('list rethrows instead of answering []', async () => { + await expect(loader.list('permission')).rejects.toThrow('read ECONNRESET'); + }); + + it('load rethrows too — ADR-0110 D3 needs the singular read to fail loudly', async () => { + await expect(loader.load('permission', 'admin_all')).rejects.toThrow('read ECONNRESET'); + }); + + it('carries the driver error unchanged, so the cause is diagnosable', async () => { + const thrown = await loader.loadMany('permission').catch((e: unknown) => e); + expect((thrown as { code?: string }).code).toBe('ECONNRESET'); + }); + + it('does not poison the cache with the failed read', async () => { + await expect(loader.loadMany('permission')).rejects.toThrow(); + // A retry must hit the driver again rather than serve a memoized []. + await expect(loader.loadMany('permission')).rejects.toThrow(); + }); + }); + + describe('the benign case — `sys_metadata` has not been provisioned yet', () => { + let loader: DatabaseLoader; + + beforeEach(() => { + loader = new DatabaseLoader({ driver: driverWithFailingReads(noSuchTable) }); + }); + + it('answers empty rather than exploding on a first boot', async () => { + await expect(loader.loadMany('permission')).resolves.toEqual([]); + await expect(loader.list('permission')).resolves.toEqual([]); + await expect(loader.exists('permission', 'admin_all')).resolves.toBe(false); + await expect(loader.stat('permission', 'admin_all')).resolves.toBeNull(); + await expect(loader.load('permission', 'admin_all')).resolves.toMatchObject({ data: null }); + }); + + it('does not memoize the empty answer — the table may appear next call', async () => { + const driver = createMockDriver(); + let provisioned = false; + const realFind = driver.find as unknown as (t: string, q: unknown) => Promise[]>; + driver.find = vi.fn().mockImplementation((table: string, query: unknown) => { + if (!provisioned) return Promise.reject(noSuchTable()); + return realFind(table, query); + }); + const healing = new DatabaseLoader({ driver }); + + expect(await healing.list('permission')).toEqual([]); + + provisioned = true; + await healing.save('permission', 'admin_all', { name: 'admin_all' }); + expect(await healing.list('permission')).toEqual(['admin_all']); + }); + }); + + it('DISTINGUISHES the two: same call site, opposite verdicts', async () => { + const benign = new DatabaseLoader({ driver: driverWithFailingReads(noSuchTable) }); + const real = new DatabaseLoader({ driver: driverWithFailingReads(connectionReset) }); + + expect(await benign.loadMany('permission')).toEqual([]); + await expect(real.loadMany('permission')).rejects.toThrow('read ECONNRESET'); + }); + + describe('what the manager on top of it can finally say', () => { + beforeEach(() => { + logger.error.mockClear(); + logger.info.mockClear(); + logger.warn.mockClear(); + }); + + function managerOverBrokenDb(): MetadataManager { + const manager = new MetadataManager({ formats: ['json'], loaders: [new MemoryLoader()] }); + manager.registerLoader( + new DatabaseLoader({ driver: driverWithFailingReads(connectionReset) }), + ); + return manager; + } + + it('list() keeps serving what it can, and reports the outage at `error`', async () => { + const manager = managerOverBrokenDb(); + manager.registerInMemory('permission', 'from_code', { name: 'from_code' }); + + const items = await manager.list('permission'); + // Best-effort listing survives — that posture is deliberate. + expect(items).toEqual([{ name: 'from_code' }]); + + // …but it is no longer silent. AGENTS.md → "Degradation log levels": + // the system looks normal while the set it gates on is short → `error`. + expect(logger.error).toHaveBeenCalledTimes(1); + expect(logger.warn).not.toHaveBeenCalled(); + const [message] = logger.error.mock.calls[0] as [string]; + expect(message).toContain('database'); + expect(message).toContain('permission'); + expect(message).toMatch(/PARTIAL/); + expect(message).toMatch(/never declared/i); + expect(message).toMatch(/reporting healthy/i); + expect(message).toMatch(/Fix:/); + }); + + it('says it once per outage, not once per read', async () => { + const manager = managerOverBrokenDb(); + + await manager.list('permission'); + await manager.list('view'); + await manager.list('flow'); + + expect(logger.error).toHaveBeenCalledTimes(1); + }); + + it('the sibling plural read, loadMany(), reports at the same level', async () => { + const manager = managerOverBrokenDb(); + + await expect(manager.loadMany('permission')).resolves.toEqual([]); + expect(logger.error).toHaveBeenCalledTimes(1); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it('un-says it when the loader becomes readable again', async () => { + const driver = createMockDriver(); + let broken = true; + const realFind = driver.find as unknown as (t: string, q: unknown) => Promise[]>; + driver.find = vi.fn().mockImplementation((table: string, query: unknown) => { + if (broken) return Promise.reject(connectionReset()); + return realFind(table, query); + }); + const manager = new MetadataManager({ formats: ['json'], loaders: [] }); + manager.registerLoader(new DatabaseLoader({ driver, cache: { enabled: false } })); + + await manager.loadMany('permission'); + expect(logger.error).toHaveBeenCalledTimes(1); + + broken = false; + await manager.loadMany('permission'); + expect(logger.error).toHaveBeenCalledTimes(1); + expect(logger.info.mock.calls.map(c => c[0]).join()).toMatch(/readable again/i); + }); + + it('loadDiagnosed reports `degraded` — ADR-0110 D3 now holds for the DB loader', async () => { + const manager = managerOverBrokenDb(); + + const diagnosed = await manager.loadDiagnosed('permission', 'admin_all'); + expect(diagnosed.data).toBeNull(); + expect(diagnosed.degraded).toBe(true); + expect(diagnosed.errors.join()).toContain('read ECONNRESET'); + }); + + it('a clean miss is still NOT degraded — the distinction is the point', async () => { + const manager = new MetadataManager({ formats: ['json'], loaders: [new MemoryLoader()] }); + manager.registerLoader(new DatabaseLoader({ driver: createMockDriver() })); + + const diagnosed = await manager.loadDiagnosed('permission', 'never_declared'); + expect(diagnosed.data).toBeNull(); + expect(diagnosed.degraded).toBe(false); + expect(logger.error).not.toHaveBeenCalled(); + }); + + /** + * The reason #5108 was split out of #5089. `listForIndex()` was written + * without a `try/catch` precisely so an unreadable store could not be + * served as "no endpoint declares this route" — but that only worked for + * loaders that actually report their failures. Against a real + * `DatabaseLoader` the seam was inert: the loader swallowed first, so + * `matchEndpoint` answered `undefined`, which the REST layer turns into a + * 404 — an availability failure rendered as a semantic "not declared". + */ + it('matchEndpoint REJECTS on a broken DatabaseLoader instead of 404-shaped undefined', async () => { + const manager = managerOverBrokenDb(); + + await expect( + manager.matchEndpoint({ method: 'GET', path: '/api/v1/apps/showcase/tasks' }), + ).rejects.toThrow('read ECONNRESET'); + }); + + it('…even when the endpoint IS declared in another, healthy loader', async () => { + const manager = managerOverBrokenDb(); + manager.registerInMemory('api', 'list_tasks', { + name: 'list_tasks', + path: '/api/v1/apps/showcase/tasks', + method: 'GET', + type: 'object_operation', + target: 'showcase_task', + }); + + // A partial read cannot prove the match it found is the right one. + await expect( + manager.matchEndpoint({ method: 'GET', path: '/api/v1/apps/showcase/tasks' }), + ).rejects.toThrow('read ECONNRESET'); + }); + + it('an unprovisioned table is NOT an outage — matchEndpoint still answers a clean miss', async () => { + const manager = new MetadataManager({ formats: ['json'], loaders: [new MemoryLoader()] }); + manager.registerLoader(new DatabaseLoader({ driver: driverWithFailingReads(noSuchTable) })); + + await expect( + manager.matchEndpoint({ method: 'GET', path: '/api/v1/apps/showcase/tasks' }), + ).resolves.toBeUndefined(); + expect(logger.error).not.toHaveBeenCalled(); + }); + }); +}); + // ---------- DatabaseLoader read-through cache ---------- describe('DatabaseLoader read-through cache', () => { diff --git a/packages/metadata/src/loaders/database-loader.ts b/packages/metadata/src/loaders/database-loader.ts index dbb22b338c..4b32e28320 100644 --- a/packages/metadata/src/loaders/database-loader.ts +++ b/packages/metadata/src/loaders/database-loader.ts @@ -664,6 +664,59 @@ export class DatabaseLoader implements MetadataLoader { }; } + // ========================================== + // Read-failure classification (#5108) + // ========================================== + + /** + * Decide what a failed READ against {@link tableName} means, and rethrow + * unless it is the ONE benign reason. + * + * #5108 (rule from #4632; same shape as #4728 and #4825) — discriminate by + * error TYPE. Every read method below used to `catch {}` into its own empty + * value: `load` → `null`, `loadMany` → `[]`, `exists` → `false`, `stat` → + * `null`, `list` → `[]`. That made a database the metadata plane cannot + * reach **indistinguishable** from an environment where nothing of that type + * was ever declared — and it erased the failure *inside the loader*, so + * neither `MetadataManager`'s own `try/catch` degradation branches nor + * {@link import('../metadata-manager.js').MetadataManager.loadDiagnosed} + * (ADR-0110 D3, whose whole purpose is to tell a miss from an outage) could + * report anything. Nowhere on the chain was there a line saying the read + * failed. + * + * Why that is worse than a noisy error: every consumer that gates on a + * *declared set* — permissions, sharing rules, policies, endpoint + * declarations — reads the empty answer as "the author declared none". Some + * then fail open (grant), some fail closed (lock out); both look healthy + * from outside. This is the AGENTS.md → "Degradation log levels" shape the + * repo has already paid for twice, one layer up from #4825. + * + * Exactly one failure reason is benign: `sys_metadata` has not been + * provisioned yet. There are then genuinely no rows, so "nothing declared" + * IS the truth, and a first boot must not explode. Every other reason — + * connection drop, timeout, insufficient privileges, malformed query — means + * the rows may well be there and simply were not seen. + * + * Classification is conservative in the same direction as + * {@link isMissingTableError} itself: an unrecognised error is NOT benign. + * A false "benign" silently mis-answers a security question; a false "real" + * costs one loud error. + * + * @param error The value thrown by `_find` / `_findOne` / `_count`. + * @throws The underlying driver error, unchanged — deliberately, matching + * {@link nextEventSeq}. The loader does not log it: the caller owns + * the consequence and is the only layer that knows what an + * incomplete answer costs it (`MetadataManager.list()` reports it at + * `error`; `listForIndex()`/`matchEndpoint` let it propagate so an + * outage can never be served as a 404). + * @returns normally ONLY for the benign case, licensing the caller to answer + * with its empty value. + */ + private rethrowUnlessTableUnprovisioned(error: unknown): void { + if (isMissingTableError(error)) return; + throw error; + } + // ========================================== // MetadataLoader Interface Implementation // ========================================== @@ -718,7 +771,11 @@ export class DatabaseLoader implements MetadataLoader { etag: record.checksum, loadTime: Date.now() - startTime, }; - } catch { + } catch (error) { + this.rethrowUnlessTableUnprovisioned(error); + // Benign only: the table is not provisioned, so there is no row. Not + // cached — `ensureSchema()` retries, and a `null` memoized here would + // outlive the provisioning that fixes it. return { data: null, loadTime: Date.now() - startTime, @@ -748,7 +805,9 @@ export class DatabaseLoader implements MetadataLoader { this.loadManyCache?.set(type, result); return result; - } catch { + } catch (error) { + this.rethrowUnlessTableUnprovisioned(error); + // Benign only: no table, therefore no items of this type. Not cached. return []; } } @@ -768,7 +827,9 @@ export class DatabaseLoader implements MetadataLoader { }); return count > 0; - } catch { + } catch (error) { + this.rethrowUnlessTableUnprovisioned(error); + // Benign only: no table, therefore the item genuinely does not exist. return false; } } @@ -805,7 +866,9 @@ export class DatabaseLoader implements MetadataLoader { }; this.statCache?.set(key, stats); return stats; - } catch { + } catch (error) { + this.rethrowUnlessTableUnprovisioned(error); + // Benign only: no table, therefore nothing to stat. Not cached. return null; } } @@ -830,7 +893,9 @@ export class DatabaseLoader implements MetadataLoader { this.listCache?.set(type, names); return names; - } catch { + } catch (error) { + this.rethrowUnlessTableUnprovisioned(error); + // Benign only: no table, therefore no names. Not cached. return []; } } diff --git a/packages/metadata/src/metadata-manager.ts b/packages/metadata/src/metadata-manager.ts index c794e8a601..434448ff2d 100644 --- a/packages/metadata/src/metadata-manager.ts +++ b/packages/metadata/src/metadata-manager.ts @@ -149,6 +149,14 @@ export class MetadataManager implements IMetadataService { private listCache = new Map(); private static readonly LIST_CACHE_TTL_MS = 30_000; + // [#5108] Loader names whose read failure has already been reported at + // `error` by `list()`. AGENTS.md → "Degradation log levels": say it once, at + // the first degradation — `list()` is hot enough that one line per failed + // read would bury the one line that matters. Cleared when the loader answers + // again, so a second outage is reported again. Same once-only discipline as + // `DatabaseLoader.schemaFailureReported`. + private readonly loaderReadFailureReported = new Set(); + // Realtime service for event publishing private realtimeService?: IRealtimeService; @@ -536,8 +544,9 @@ export class MetadataManager implements IMetadataService { items.set(itemAny.name, item); } } + this.reportLoaderReadRecovered(loader.contract.name); } catch (e) { - this.logger.warn(`Loader ${loader.contract.name} failed to loadMany ${type}`, { error: e }); + this.reportLoaderReadFailure(loader.contract.name, type, e); } } @@ -545,6 +554,60 @@ export class MetadataManager implements IMetadataService { this.cacheListResult(type, result); return result; } + + /** + * Report — at `error`, once per outage episode — that a loader could not be + * read while serving {@link list}. + * + * [#5108] This branch used to be dead for the loader that matters. Before + * #5108 `DatabaseLoader` caught its own read failures and answered `[]`, so + * `list()` received a *successful empty read* and never entered this `catch` + * at all: an unreachable `sys_metadata` and "this environment declares no + * `permission`" produced byte-identical results with not one line logged. + * With the loader rethrowing everything but the benign not-provisioned case, + * this is where the outage finally becomes speakable. + * + * `error`, not `warn`, per AGENTS.md → "Degradation log levels". Apply its + * one question — *does the system still look normal from outside while + * something it claims to know has not actually landed?* — and the answer is + * yes: `list()` still returns, callers still get an array, nothing 500s, and + * the set they gate on is quietly short. Which way that cuts depends on the + * consumer, and both ways are silent (#3935 is the fail-open precedent). + * + * Said **once** per loader, and un-said on recovery, because `list()` is a + * hot path — one line per outage, not one per read. + * + * Deliberately does NOT rethrow: `list()` is the best-effort listing seam and + * must keep serving what the reachable loaders hold. The strict counterpart + * for callers whose answer is a security decision is `listForIndex()` (no + * `catch`, feeding `matchEndpoint`) and {@link loadDiagnosed} (ADR-0110 D3) + * for the singular read — both of which only became honest for + * `DatabaseLoader` with the same #5108 change. + */ + private reportLoaderReadFailure(loaderName: string, type: string, error: unknown): void { + if (this.loaderReadFailureReported.has(loaderName)) return; + this.loaderReadFailureReported.add(loaderName); + this.logger.error( + `[MetadataManager] Loader \`${loaderName}\` could NOT be read (first failure seen while listing \`${type}\`) — ` + + `every list served from now on is a PARTIAL set presented as a complete one, and the server keeps reporting healthy. ` + + `Consumers that gate on a declared set (permissions, sharing rules, policies, api endpoints) will read the ` + + `declarations this loader holds as "never declared" — which grants or locks out depending on the consumer, silently either way. ` + + `Fix: check the datasource behind \`${loaderName}\` — connection, credentials, and that its metadata table exists. ` + + `The read is retried on the next list once the ${MetadataManager.LIST_CACHE_TTL_MS}ms list cache lapses, so a transient ` + + `cause recovers on its own and the recovery is logged.`, + error instanceof Error ? error : undefined, + { loader: loaderName, type, error }, + ); + } + + /** Un-say {@link reportLoaderReadFailure} once the loader answers again. */ + private reportLoaderReadRecovered(loaderName: string): void { + if (!this.loaderReadFailureReported.delete(loaderName)) return; + this.logger.info( + `[MetadataManager] Loader \`${loaderName}\` is readable again — listings are complete once more.`, + ); + } + private cacheListResult(type: string, items: unknown[]): void { this.listCache.set(type, { ts: Date.now(), items }); } @@ -565,7 +628,7 @@ export class MetadataManager implements IMetadataService { * Enumerate stored items of `type` for an index build — like {@link list}, * but a store that cannot be read THROWS instead of contributing nothing. * - * [#5089] `list()` deliberately warn-logs and skips a failing loader so a + * [#5089] `list()` deliberately logs a failing loader and skips it so a * partially-available metadata plane still serves what it can. That posture * is wrong for `matchEndpoint`: its `undefined` becomes an HTTP 404, and a * store outage that silently yields "zero declarations" would turn every @@ -577,10 +640,14 @@ export class MetadataManager implements IMetadataService { * is `list()`'s failure posture inverted for the one caller whose answer is * a security/availability decision rather than a best-effort listing. * - * ⚠️ This surfaces only failures a loader actually reports. `DatabaseLoader` - * currently swallows its own read errors into `[]` (#5108), so a DB outage is - * invisible even here — that is a defect in the loader, not a reason to - * soften this seam. + * This surfaces only failures a loader actually reports — which, since + * #5108, includes `DatabaseLoader`: it used to swallow its own read errors + * into `[]`, making a DB outage invisible even here. It now rethrows every + * read failure except the benign "table not provisioned yet", so this seam + * holds against the real datasource-backed loader and not just the memory / + * remote ones. (`database-loader.test.ts` pins that end to end: a broken + * driver behind a real `DatabaseLoader` makes `matchEndpoint` reject rather + * than answer a 404-shaped `undefined`.) */ private async listForIndex(type: string): Promise { const items = new Map(); @@ -1634,8 +1701,13 @@ export class MetadataManager implements IMetadataService { } results.push(item); } + this.reportLoaderReadRecovered(loader.contract.name); } catch (e) { - this.logger.warn(`Loader ${loader.contract.name} failed to loadMany ${type}`, { error: e }); + // [#5108] Same seam, same verdict as `list()` — see + // {@link reportLoaderReadFailure}. Two adjacent plural reads + // reporting one storage outage at two different levels is how the + // wrong one gets copied. + this.reportLoaderReadFailure(loader.contract.name, type, e); } } return results;