diff --git a/.changeset/missing-table-must-name-the-read-table.md b/.changeset/missing-table-must-name-the-read-table.md new file mode 100644 index 0000000000..ee53abbca5 --- /dev/null +++ b/.changeset/missing-table-must-name-the-read-table.md @@ -0,0 +1,36 @@ +--- +"@objectstack/metadata": minor +"@objectstack/objectql": patch +"@objectstack/metadata-protocol": patch +--- + +fix(metadata,objectql,metadata-protocol): require a missing-table error to name the table that was READ (#13324) + +`isMissingTableError` answers the one question that licenses a fail-soft caller +to treat an empty result as the truth: "did this read fail because the table has +not been provisioned yet?". It matched the *shape* of the dialect phrase and +never asked WHICH table the phrase names. + +Measured on a real libsql database: a view whose base table is gone fails with +`no such table: main.` when the view itself is read. The phrase matches, +so a read of a relation that **exists and may be backed by rows** was classified +benign, and every fail-soft consumer on that path — `probeInstallOrganizations`, +`resolveFileReferences`, `seedAutonumber`, the cascade-delete dependents probe, +`DatabaseLoader`, `SeedLoaderService`, the `sys_metadata` overlay reads — +computed its answer from data it never read. That is a false "benign", the +direction the module's own docblock calls far more expensive than a false +"real". + +The predicate now takes the object the caller was reading and refuses the +benign verdict when the phrase names a different relation. Shape alone cannot +separate the two cases: measured, a view over a missing base table and a +genuine missing table the caller qualified produce byte-identical messages, so +the read's name is a parameter rather than another regex. + +The parameter is **optional** — omitting it reproduces the previous behaviour +exactly, so no external caller of `@objectstack/metadata/errors` changes. Every +in-repo call site now passes it. The comparison folds away schema/database +qualifiers, the legacy `namespace__short` prefix and case, so every shape +recognised before for a genuine missing table (sqlite `no such table: X`, +Postgres `relation "x" does not exist`, MySQL `table "x" doesn't exist`, +`unknown table`, the SQLSTATE and errno limbs) still answers benign. diff --git a/packages/metadata-protocol/src/protocol.metadata-store-outage.test.ts b/packages/metadata-protocol/src/protocol.metadata-store-outage.test.ts index b8f59dcebf..5a606b39e2 100644 --- a/packages/metadata-protocol/src/protocol.metadata-store-outage.test.ts +++ b/packages/metadata-protocol/src/protocol.metadata-store-outage.test.ts @@ -84,9 +84,17 @@ function emptyRegistry(items: Record = {}) { * An engine whose every read REJECTS with `error` — the shape of a metadata * store the protocol cannot reach. */ -function engineThatCannotBeRead(error: () => unknown, registryItems: Record = {}) { +function engineThatCannotBeRead( + error: (object: string) => unknown, + registryItems: Record = {}, +) { + // [#13324] The object reaches the factory, so a missing-table fault can be + // phrased for the table that was actually read. A driver never names one + // table while failing a read of another, and `isMissingTableError` now + // tells those two apart — a fixed phrase would make this fixture assert + // the benign verdict for a fault no driver produces here. const reject = vi.fn(async (object: string, query?: EngineFindOneQueryInput) => { - assertEngineFindOnePredicate(object, query); throw error(); }); + assertEngineFindOnePredicate(object, query); throw error(object); }); return { registry: emptyRegistry(registryItems), find: reject, @@ -104,8 +112,8 @@ function engineWithRows(rows: any[] = [], registryItems: Record = { } /** 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' }); +const missingTable = (object = 'sys_metadata') => + Object.assign(new Error(`SQLITE_ERROR: no such table: ${object}`), { code: 'SQLITE_ERROR' }); /** An outage: the rows may well exist and simply were not seen. */ const connectionRefused = () => diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 71c528b66a..c93ad93a89 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -5792,8 +5792,16 @@ export class ObjectStackProtocolImplementation implements * @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; + private rethrowUnlessMetadataStoreUnprovisioned(error: unknown, readObject: string): void { + // [#13324] `readObject` is REQUIRED, deliberately. This helper serves + // callers that read four different tables (`sys_metadata`, + // `sys_metadata_audit`, `sys_metadata_commit`, `sys_metadata_history`), + // so a default would silently answer about the wrong one for three of + // them — measured, not hypothetical: the first draft of this repair + // hardcoded `sys_metadata` and turned `diffMetaItem`'s genuinely + // unprovisioned `sys_metadata_history` into a loud failure. A required + // parameter makes the compiler ask the question at every new call site. + if (isMissingTableError(error, readObject)) return; // [#12536] CLASSIFY, do not assume. A read can fail because the store // is unreachable OR because a metadata app's hook refused it in its // own words — see {@link metadataReadFailureError}. @@ -6398,7 +6406,7 @@ export class ObjectStackProtocolImplementation implements // 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); + this.rethrowUnlessMetadataStoreUnprovisioned(error, 'sys_metadata'); } // ADR-0033 draft-overlay preview: when the caller opts in (admin-gated @@ -6457,7 +6465,7 @@ export class ObjectStackProtocolImplementation implements // 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); + this.rethrowUnlessMetadataStoreUnprovisioned(error, 'sys_metadata'); } } @@ -6686,7 +6694,7 @@ export class ObjectStackProtocolImplementation implements // [#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); + this.rethrowUnlessMetadataStoreUnprovisioned(error, 'sys_metadata'); } } @@ -6751,7 +6759,7 @@ export class ObjectStackProtocolImplementation implements // 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); + this.rethrowUnlessMetadataStoreUnprovisioned(error, 'sys_metadata'); } // Draft reads stop here — they intentionally do NOT fall through @@ -7176,7 +7184,7 @@ export class ObjectStackProtocolImplementation implements // overlay row, so `overlay: null` / `effective = code` IS the truth // and first boot still renders the code layer. // See {@link rethrowUnlessMetadataStoreUnprovisioned}. - this.rethrowUnlessMetadataStoreUnprovisioned(error); + this.rethrowUnlessMetadataStoreUnprovisioned(error, 'sys_metadata'); } // [#4513] `effective` is documented above as "what `getMetaItem` would @@ -7516,7 +7524,7 @@ export class ObjectStackProtocolImplementation implements // The second cause the old comment named — a host engine with no // `find` — is decided by the precondition probe above the `try`, so // it never reaches here and this arm has exactly ONE benign cause. - this.rethrowUnlessMetadataStoreUnprovisioned(err); + this.rethrowUnlessMetadataStoreUnprovisioned(err, 'sys_metadata_audit'); console.warn( `[Protocol] auditMetaItem read failed for ${request.type}/${request.name}: ${err?.message ?? err}`, ); @@ -10337,7 +10345,7 @@ export class ObjectStackProtocolImplementation implements // // No new response field and no new error code — the caller // receives the read's own failure, envelope intact. - if (isMissingTableError(error)) continue; + if (isMissingTableError(error, obj.name)) continue; throw error; } } @@ -12042,7 +12050,7 @@ export class ObjectStackProtocolImplementation implements // `rollback` / `delete` now fail with 503 when the lock state // cannot be read, instead of proceeding as if unlocked. Refusing // one uncertain write beats performing one that had to be refused. - this.rethrowUnlessMetadataStoreUnprovisioned(error); + this.rethrowUnlessMetadataStoreUnprovisioned(error, 'sys_metadata'); } return { lock: 'none', lockReason: undefined, lockSource: undefined }; } @@ -13428,7 +13436,7 @@ export class ObjectStackProtocolImplementation implements const row = await this.engine.findOne('sys_metadata', { where: { type } }); return row != null; } catch (error) { - this.rethrowUnlessMetadataStoreUnprovisioned(error); + this.rethrowUnlessMetadataStoreUnprovisioned(error, 'sys_metadata'); return false; } } @@ -16335,7 +16343,7 @@ export class ObjectStackProtocolImplementation implements // // No new error code and no new response field: the caller // receives the read's own failure, envelope intact. - if (!isMissingTableError(error)) throw error; + if (!isMissingTableError(error, 'sys_metadata')) throw error; commitItems.push({ type: d.type, name: d.name, existedBefore: false, prevVersion: null }); } } @@ -17784,7 +17792,7 @@ export class ObjectStackProtocolImplementation implements // the turn is unrevertible is a separate question (a response-field // change the #8896 ruling forbids for this family) and deliberately // NOT decided here. - if (isMissingTableError(error)) { + if (isMissingTableError(error, 'sys_metadata_commit')) { if (!this.commitStoreUnprovisionedNoted) { this.commitStoreUnprovisionedNoted = true; console.info( @@ -17935,7 +17943,7 @@ export class ObjectStackProtocolImplementation implements } catch (error) { // [#5980] Benign (the table has not been provisioned) falls through; // everything else is a read that did not happen and leaves as a 503. - this.rethrowUnlessMetadataStoreUnprovisioned(error); + this.rethrowUnlessMetadataStoreUnprovisioned(error, 'sys_metadata_commit'); return []; } } @@ -18933,7 +18941,7 @@ export class ObjectStackProtocolImplementation implements // ⛔ A `historyUnavailable: true` response key (the card's option B) was // DECLINED in the same ruling — a new published key with no consumer, on // the manual floor. Do not reintroduce it as "more informative". - this.rethrowUnlessMetadataStoreUnprovisioned(error); + this.rethrowUnlessMetadataStoreUnprovisioned(error, 'sys_metadata_history'); } const byVersion = new Map | null>(); for (const r of histRows) byVersion.set(r.version, r.body); @@ -19684,7 +19692,7 @@ export class ObjectStackProtocolImplementation implements // `error` names what the outage COSTS and how to fix it. Keeping // the technical line here at `warn` is what lets the consumer's // line stay the single loud statement of consequence. - if (!isMissingTableError(e)) { + if (!isMissingTableError(e, 'sys_metadata')) { storeUnavailable = true; console.warn( `[Protocol] DB hydration skipped: ${e instanceof Error ? e.message : String(e)}`, diff --git a/packages/metadata-protocol/src/seed-loader-existing-records-read-failure.test.ts b/packages/metadata-protocol/src/seed-loader-existing-records-read-failure.test.ts index 9ba072e876..7ee30da60e 100644 --- a/packages/metadata-protocol/src/seed-loader-existing-records-read-failure.test.ts +++ b/packages/metadata-protocol/src/seed-loader-existing-records-read-failure.test.ts @@ -65,6 +65,13 @@ function createLogger() { * re-wrap a driver error. `findCalls` records that the read really ran, which * is what turns "the seed proceeded" into "the seed proceeded AND the injected * throw fired". + * + * [#13324] It also accepts a FUNCTION of the object name, because the loader + * reads more than one table on this path (`sys_organization` for the sole-org + * probe, then the seeded object) and a missing-table fault names the table it + * was raised for. A single fixed value phrased for one of them is a fault that + * no driver produces for the other, and `isMissingTableError` now tells those + * apart — so the per-object form is what keeps this fixture faithful. */ function createEngine() { const store: Record = {}; @@ -75,7 +82,7 @@ function createEngine() { const engine = { find: vi.fn(async (objectName: string, query?: { where?: Record; limit?: number }) => { findCalls.push(objectName); - if (failFind !== null) throw failFind; + if (failFind !== null) throw typeof failFind === 'function' ? failFind(objectName) : failFind; let records = store[objectName] ?? []; if (query?.where) { const where = query.where; @@ -134,7 +141,7 @@ function createEngine() { engine, store, findCalls, - failReadsWith: (error: unknown) => { failFind = error; }, + failReadsWith: (error: unknown | ((objectName: string) => unknown)) => { failFind = error; }, stopFailingReads: () => { failFind = null; }, }; } @@ -180,8 +187,8 @@ const seedOf = (mode: string, records: Array>) => [{ /** The real driver phrasings, verbatim. */ const connectionDropped = () => Object.assign(new Error('connection terminated unexpectedly'), { code: 'ECONNRESET' }); -const tableNotProvisioned = () => - Object.assign(new Error('SQLITE_ERROR: no such table: my_app_widget'), { code: 'SQLITE_ERROR' }); +const tableNotProvisioned = (objectName = 'my_app_widget') => + Object.assign(new Error(`SQLITE_ERROR: no such table: ${objectName}`), { code: 'SQLITE_ERROR' }); /** Capture a rejection without letting a resolve pass silently. */ async function rejection(run: () => Promise): Promise<{ code?: string; message?: string } & Record> { @@ -315,7 +322,7 @@ describe('[#8896] seed loader — an existing-records read that FAILED is not "n it('an UNPROVISIONED table is truthful emptiness: the seed writes its rows', async () => { const { engine, store, findCalls, failReadsWith } = createEngine(); - failReadsWith(tableNotProvisioned()); + failReadsWith((objectName: string) => tableNotProvisioned(objectName)); const result = await new SeedLoaderService(engine, createMetadata(), createLogger()).load({ seeds: seedOf('upsert', [{ name: 'Fresh', sku: 'W-A' }]), @@ -333,8 +340,8 @@ describe('[#8896] seed loader — an existing-records read that FAILED is not "n it('an UNPROVISIONED table in the postgres phrasing (42P01) is benign too', async () => { const { engine, findCalls, failReadsWith } = createEngine(); - failReadsWith(Object.assign( - new Error('relation "my_app_widget" does not exist'), + failReadsWith((objectName: string) => Object.assign( + new Error(`relation "${objectName}" does not exist`), { code: '42P01' }, )); diff --git a/packages/metadata-protocol/src/seed-loader.ts b/packages/metadata-protocol/src/seed-loader.ts index 1790b7c3bf..b8cdc77a5f 100644 --- a/packages/metadata-protocol/src/seed-loader.ts +++ b/packages/metadata-protocol/src/seed-loader.ts @@ -1428,7 +1428,7 @@ export class SeedLoaderService implements ISeedLoaderService { // seen. It propagates, envelope intact: the seed run fails loudly instead // of writing a batch of rows nobody will be able to see. No new error code // and no new result field — the caller receives the read's own failure. - if (!isMissingTableError(error)) throw error; + if (!isMissingTableError(error, 'sys_organization')) throw error; } return undefined; } @@ -2476,7 +2476,7 @@ export class SeedLoaderService implements ISeedLoaderService { // write plan from data it never read. No new error code and no new // result field — the caller receives the read's own failure, envelope // intact, and the seed's existing error accounting reports it. - if (!isMissingTableError(error)) throw error; + if (!isMissingTableError(error, objectName)) throw error; } return map; } diff --git a/packages/metadata-protocol/src/sys-metadata-repository.ts b/packages/metadata-protocol/src/sys-metadata-repository.ts index 9343ca4bfa..17cb9b8206 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.ts @@ -1880,7 +1880,9 @@ export class SysMetadataRepository implements MetadataRepository { subject: string, ): 1 { // Benign — and only benign: a fresh DB has no row to be inconsistent with. - if (isMissingTableError(error)) return 1; + // [#13324] Both callers read `this.historyTable`, so a failure naming any + // other relation is not evidence that THIS one is empty. + if (isMissingTableError(error, this.historyTable)) return 1; if (!this.historyCounterFailureReported) { this.historyCounterFailureReported = true; diff --git a/packages/metadata/src/loaders/database-loader.ts b/packages/metadata/src/loaders/database-loader.ts index 761d1a6f17..4aa81dd82a 100644 --- a/packages/metadata/src/loaders/database-loader.ts +++ b/packages/metadata/src/loaders/database-loader.ts @@ -331,7 +331,7 @@ export class DatabaseLoader implements MetadataLoader { } catch (error) { // Benign — and ONLY benign: there is no table, therefore no row, so // numbering from 1 cannot collide with anything. - if (isMissingTableError(error)) return 1; + if (isMissingTableError(error, this.historyTableName)) return 1; throw error; } } @@ -760,7 +760,11 @@ export class DatabaseLoader implements MetadataLoader { * with its empty value. */ private rethrowUnlessTableUnprovisioned(error: unknown): void { - if (isMissingTableError(error)) return; + // [#13324] Every caller of this helper reads `this.tableName`, so that is + // the relation whose emptiness they are about to trust — a failure naming + // any OTHER relation (a view over a dropped base table) is not evidence + // about it and stays loud. + if (isMissingTableError(error, this.tableName)) return; throw error; } diff --git a/packages/metadata/src/utils/schema-sync-errors.test.ts b/packages/metadata/src/utils/schema-sync-errors.test.ts index 24e63be946..77be581545 100644 --- a/packages/metadata/src/utils/schema-sync-errors.test.ts +++ b/packages/metadata/src/utils/schema-sync-errors.test.ts @@ -457,3 +457,154 @@ describe('isMissingTableError — Postgres sub-object phrases are NOT missing ta expect(isMissingTableError(err)).toBe(false); }); }); + +describe('isMissingTableError — the phrase must name the table that was READ (#13324)', () => { + /** + * The measured defect. Reading a VIEW whose base table is gone fails with a + * phrase that answers the shape test perfectly and names something else. + * + * Both spellings libsql produces are pinned: the wrapper's `SQLITE_ERROR: ` + * prefix and the bare `cause` message underneath it, since the predicate + * walks the `cause` chain and must reach the same verdict at either node. + */ + const VIEW_OVER_MISSING_BASE = 'no such table: main.table_that_does_not_exist'; + + it('answers NOT benign for a view whose base table is missing', () => { + // Measured against a real libsql file database: `CREATE VIEW + // sys_metadata AS SELECT … FROM table_that_does_not_exist`, then + // `SELECT * FROM sys_metadata`. `sys_metadata` exists and may be backed + // by rows; the licence "there are no rows" is false about it. + const err = Object.assign(new Error(`SQLITE_ERROR: ${VIEW_OVER_MISSING_BASE}`), { + code: 'SQLITE_ERROR', + cause: Object.assign(new Error(VIEW_OVER_MISSING_BASE), { code: 'SQLITE_ERROR' }), + }); + expect(isMissingTableError(err, 'sys_metadata')).toBe(false); + }); + + it('is the ONLY thing that separates the view case from a genuine miss', () => { + // ⛔ The two are byte-identical in shape, measured on libsql — a view + // over `absent_base`, and a genuine missing table the caller qualified. + // Any repair that reads only the phrase must answer both the same way, + // which is why the read's name is a parameter and not a regex. + const viewFault = new Error('no such table: main.absent_base'); + const genuineMiss = new Error('no such table: main.orders'); + expect(viewFault.message.replace('absent_base', 'orders')).toBe(genuineMiss.message); + + expect(isMissingTableError(viewFault, 'orders')).toBe(false); + expect(isMissingTableError(genuineMiss, 'orders')).toBe(true); + }); + + /** + * ⛔ Fence: every shape recognised today for a GENUINE missing table must + * still be recognised. A suite that silently lost a true positive is the + * failure mode this repair is most likely to cause, so each limb of the + * signature is pinned with the read named. + */ + const TRUE_POSITIVES: ReadonlyArray = [ + ['sqlite `no such table: X`', new Error('no such table: sys_metadata_history'), 'sys_metadata_history'], + [ + 'sqlite schema-qualified `no such table: main.X`', + new Error('no such table: main.sys_metadata_history'), + 'sys_metadata_history', + ], + [ + 'PG `relation "x" does not exist`', + Object.assign(new Error('relation "sys_metadata_history" does not exist'), { code: '42P01' }), + 'sys_metadata_history', + ], + [ + 'MySQL `table "x" doesn\'t exist`', + new Error("Table 'app.sys_metadata_history' doesn't exist"), + 'sys_metadata_history', + ], + ['MySQL `unknown table`', new Error("Unknown table 'app.sys_metadata_history'"), 'sys_metadata_history'], + [ + 'the SQLSTATE limb, message carrying no name at all', + Object.assign(new Error('db error'), { code: '42P01' }), + 'sys_metadata_history', + ], + [ + 'the mysql2 symbolic-code limb', + Object.assign(new Error('db error'), { code: 'ER_NO_SUCH_TABLE' }), + 'sys_metadata_history', + ], + [ + 'the errno limb', + Object.assign(new Error('opaque'), { errno: 1146 }), + 'sys_metadata_history', + ], + [ + 'the driver wrapper, original attached as `cause`', + Object.assign(new Error("The database refused to run this query for object 'sys_file'."), { + code: 'DATABASE_ERROR', + cause: new Error('no such table: sys_file'), + }), + 'sys_file', + ], + ]; + + it.each(TRUE_POSITIVES)('still benign: %s', (_name, error, readObject) => { + expect(isMissingTableError(error, readObject)).toBe(true); + }); + + describe('the comparison folds away what two dialects spell differently', () => { + it('ignores case', () => { + expect(isMissingTableError(new Error('no such table: SYS_METADATA'), 'sys_metadata')).toBe(true); + }); + + it('ignores the MySQL database qualifier', () => { + expect( + isMissingTableError(new Error("Table 'app.sys_metadata' doesn't exist"), 'sys_metadata'), + ).toBe(true); + }); + + it('ignores the legacy `namespace__short` object prefix `resolveTableName` strips', () => { + // A caller naming the object `crm__account` reads the table + // `account`; a strict comparison would call that a mismatch and + // turn a genuine missing table loud. + expect(isMissingTableError(new Error('no such table: account'), 'crm__account')).toBe(true); + }); + }); + + describe('no evidence means "as we were", never "be loud"', () => { + it('keeps the pre-#13324 verdict when the caller names nothing', () => { + // The parameter is optional because this is a published export. + // Omitting it must reproduce the old behaviour EXACTLY — including + // the old behaviour on the very error this card is about. + expect(isMissingTableError(new Error(VIEW_OVER_MISSING_BASE))).toBe(true); + expect(isMissingTableError(new Error('no such table: sys_metadata'))).toBe(true); + }); + + it('ignores a non-string second argument', () => { + // Before this card the second positional parameter was the internal + // `depth` counter. A stale numeric argument must not be read as a + // table name (which would compare against `"0"` and answer loud). + expect(isMissingTableError(new Error('no such table: sys_metadata'), 0 as never)).toBe(true); + }); + + it('leaves a phrase with no extractable name benign', () => { + // `unknown table` with nothing quoted after it names nothing, so it + // cannot contradict the caller. + expect(isMissingTableError(new Error('unknown table'), 'sys_metadata')).toBe(true); + }); + }); + + it('does not let a mismatching phrase be rescued by a matching `cause`', () => { + // Same disposition #6347 gave the sub-object exclusion: recognition + // ends the question rather than descending, so a nested phrase naming + // the read table cannot restore the benign verdict. + const err = Object.assign(new Error('no such table: main.some_other_table'), { + cause: Object.assign(new Error('relation "sys_metadata" does not exist'), { code: '42P01' }), + }); + expect(isMissingTableError(err, 'sys_metadata')).toBe(false); + }); + + it('leaves the DDL predicate untouched', () => { + // The new channel is scoped to MISSING_TABLE; `isSchemaAlreadyExistsError` + // takes no read name and must not have acquired one. + const err = Object.assign(new Error('table sys_metadata already exists'), { + code: 'SQLITE_ERROR', + }); + expect(isSchemaAlreadyExistsError(err)).toBe(true); + }); +}); diff --git a/packages/metadata/src/utils/schema-sync-errors.ts b/packages/metadata/src/utils/schema-sync-errors.ts index 663bc62d2e..21a715fa0a 100644 --- a/packages/metadata/src/utils/schema-sync-errors.ts +++ b/packages/metadata/src/utils/schema-sync-errors.ts @@ -88,6 +88,92 @@ // and for why the exclusion's width deliberately differs from the extractor's. import { isRelationSubObjectPhrase } from '@objectstack/types'; +/** + * The relation name each missing-table phrase puts on display, one capture per + * dialect spelling in {@link MISSING_TABLE.message}. + * + * Extraction is deliberately partial. A phrase whose name cannot be read back + * out — `unknown table` with nothing quoted after it, a bare SQLSTATE, an + * errno — yields nothing, and yielding nothing must stay *silent* rather than + * become evidence: see {@link phraseNamesAnotherRelation}. + */ +const RELATION_IN_PHRASE: readonly RegExp[] = [ + // SQLite / libsql: `no such table: sys_metadata_history`, and the + // schema-qualified `no such table: main.orders` it uses when it resolved + // the name itself (views, triggers) or the caller qualified it. + /no such table:\s*([^\s'"`;,()]+)/i, + // PostgreSQL: `relation "sys_metadata_history" does not exist` + /relation\s+["'`]([^"'`]+)["'`]\s+does not exist/i, + // MySQL / MariaDB: `Table 'app.sys_metadata_history' doesn't exist` + /table\s+["'`]([^"'`]+)["'`]\s+doesn'?t exist/i, + // MySQL / MariaDB: `Unknown table 'app.t'` + /unknown table\s+["'`]([^"'`]+)["'`]/i, +]; + +/** + * Reduce a relation name to the part two dialects can be expected to agree on. + * + * Drops a leading qualifier (`main.orders`, `app.orders` — SQLite's schema, + * MySQL's database) and the legacy `{namespace}__{shortName}` prefix that + * `StorageNameMapping.resolveTableName` strips to get from an object name to + * its physical table, then case-folds. + * + * Every step here makes the comparison MORE likely to match, and that + * direction is chosen on purpose: a match keeps today's benign verdict, so an + * over-eager normaliser can only ever leave the gap open, while an over-strict + * one would manufacture a loud verdict for a genuine missing table — the one + * regression this repair is not allowed to cause. + */ +function normaliseRelationName(name: string): string { + const afterQualifier = name.slice(name.lastIndexOf('.') + 1); + const namespaceEnd = afterQualifier.lastIndexOf('__'); + const bare = namespaceEnd === -1 ? afterQualifier : afterQualifier.slice(namespaceEnd + 2); + return bare.toLowerCase(); +} + +/** + * Does this message name a relation OTHER than the one the caller read? + * + * The gap this closes: the message test recognises the *shape* of "no such + * table" and never asks WHICH table. A view over a dropped base table answers + * the shape perfectly — measured on libsql, reading a view named `sys_metadata` + * whose base table is gone raises `no such table: main.` — so a read of a + * relation that very much exists was classified as "not provisioned yet", and + * every fail-soft consumer took the empty answer as the truth. That is a false + * *benign*, the direction the module docblock calls far more expensive than a + * false "real". + * + * ⛔ Not answerable from the phrase's shape alone, and that is why this channel + * takes the read's name rather than a regex. Measured on libsql, a view over a + * missing base table and a genuine missing table the caller happened to qualify + * produce byte-identical spellings (`no such table: main.absent_base` vs + * `no such table: main.orders`); the only thing that separates them is whether + * the name in the phrase is the name that was asked for. + * + * Conservative in the direction the rest of the module already errs in — it + * can only ever SUBTRACT benign verdicts, never add one: + * - no name extractable, or no name supplied -> `false` (stay as we were) + * - any extracted name matches the read -> `false` (a true positive) + * - names found, none of them the read's -> `true` (the phrase is about + * something else; be loud) + */ +function phraseNamesAnotherRelation(message: string, readObject: string): boolean { + const expected = normaliseRelationName(readObject); + if (expected === '') return false; + + let named = false; + for (const pattern of RELATION_IN_PHRASE) { + const captured = pattern.exec(message)?.[1]; + if (captured === undefined) continue; + const candidate = normaliseRelationName(captured); + if (candidate === '') continue; + // One agreeing name is enough to keep the benign verdict. + if (candidate === expected) return false; + named = true; + } + return named; +} + /** One "which errors mean X?" vocabulary, in the three forms drivers use. */ interface DriverErrorSignature { /** `error.code` — Postgres SQLSTATE, or mysql2's symbolic name. */ @@ -108,6 +194,20 @@ interface DriverErrorSignature { readonly excludes?: { /** SQLSTATEs / driver codes that positively mean "**not** this case". */ readonly codes: ReadonlySet; + /** + * Message shapes whose named relation is not the one that was READ. + * + * The third exclusion channel, and the only one that needs a fact from + * the caller: the message alone cannot say whether the relation it + * names is the one the caller asked for. Given the read's own object + * name, it answers "this phrase is about something else" — which is + * the same not-X-first move {@link matchesMessage} makes, one step out. + * + * Absent (or given no object name) it never fires, so a signature that + * does not carry it, and a caller that cannot name what it read, both + * keep the pure-shape behaviour. + */ + readonly namesAnotherRelation?: (message: string, readObject: string) => boolean; /** * Message shapes that carry a legal match for this case as a substring. * @@ -235,12 +335,43 @@ const MISSING_TABLE: DriverErrorSignature = { * one, which is the direction this whole module already errs in. */ matchesMessage: isRelationSubObjectPhrase, + /** + * [#13324] "…and the relation it names is not the one you read." + * + * The sibling of the phrase above, reached one step further out. That + * one recognises a failure about something INSIDE a relation, which + * therefore says the relation is present; this one recognises a failure + * about a DIFFERENT relation, which says nothing at all about the one + * the caller read. Both end the question with `false` for the same + * reason: the licence this predicate grants — "there are no rows, so + * there is nothing to be inconsistent with" — is about the table that + * was READ, and neither phrase is evidence about it. + */ + namesAnotherRelation: phraseNamesAnotherRelation, }, }; /** How far to follow an `error.cause` chain — drivers wrap, but not deeply. */ const MAX_CAUSE_DEPTH = 4; +/** + * The {@link DriverErrorSignature.excludes.namesAnotherRelation} channel, in the + * one place both the string and the object node reach it. + * + * Guards the caller-supplied half rather than trusting it: the parameter is + * optional on the public predicate, so `undefined` (a caller that cannot name + * what it read) and a non-string (a stale positional `depth` argument from + * before this parameter existed) must both mean "no evidence", never "loud". + */ +function excludedByReadObject( + message: string, + signature: DriverErrorSignature, + readObject: string | undefined, +): boolean { + if (typeof readObject !== 'string' || readObject === '') return false; + return signature.excludes?.namesAnotherRelation?.(message, readObject) === true; +} + /** * The single matcher both predicates run on: exclusions, then code, then errno, * then message, then one step down the `cause` chain. @@ -259,11 +390,13 @@ function matchesDriverError( error: unknown, signature: DriverErrorSignature, depth: number, + readObject?: string, ): boolean { if (error === null || error === undefined || depth > MAX_CAUSE_DEPTH) return false; if (typeof error === 'string') { if (signature.excludes?.matchesMessage(error)) return false; + if (excludedByReadObject(error, signature, readObject)) return false; return signature.message.test(error); } if (typeof error !== 'object') return false; @@ -279,6 +412,8 @@ function matchesDriverError( if (excludes) { if (typeof err.code === 'string' && excludes.codes.has(err.code)) return false; if (typeof err.message === 'string' && excludes.matchesMessage(err.message)) return false; + if (typeof err.message === 'string' && excludedByReadObject(err.message, signature, readObject)) + return false; } if (typeof err.code === 'string' && signature.codes.has(err.code)) return true; @@ -286,7 +421,7 @@ function matchesDriverError( if (typeof err.message === 'string' && signature.message.test(err.message)) return true; // Drivers commonly re-throw with the original attached as `cause`. - return matchesDriverError(err.cause, signature, depth + 1); + return matchesDriverError(err.cause, signature, depth + 1, readObject); } /** @@ -317,11 +452,30 @@ export function isSchemaAlreadyExistsError(error: unknown, depth = 0): boolean { * Postgres' two phrasings — the relation is right there in the message because * it exists (#6347). See {@link MISSING_TABLE}'s `excludes`. * + * [#13324] Neither is a failure that names a **different relation**, and that + * one cannot be seen without `readObject`. The message test asks what the + * phrase LOOKS like and never which table it names, so a read of a view whose + * base table has been dropped — `no such table: main.`, measured on + * libsql for a view that itself exists — answered benign for a relation that is + * present and may be backed by rows. Naming the read closes it: the phrase must + * be about the table the caller asked for, or it is not evidence about it. + * + * Pass `readObject` from every in-repo call site. It is **optional** so that + * omitting it is exactly the pre-#13324 behaviour rather than a new loud + * failure — this is a published export (`@objectstack/metadata/errors`), and a + * required parameter would be a breaking change to it. The cost of the choice + * is that the narrowing is opt-in per call site: a new caller that forgets it + * silently gets the old, wider verdict. + * * @param error - The value thrown by a driver/engine read (`find`, `findOne`, …). + * @param readObject - The object/table whose emptiness the caller is about to + * treat as the truth — its own API name is fine, the comparison folds + * away schema qualifiers, the legacy `ns__short` prefix and case. + * Omitted (or not a string) means "cannot say", never "be loud". * @param depth - Internal `cause`-chain recursion counter; callers pass nothing. * @returns `true` only when the error positively identifies as - * table/relation-does-not-exist. + * table/relation-does-not-exist **for `readObject`**. */ -export function isMissingTableError(error: unknown, depth = 0): boolean { - return matchesDriverError(error, MISSING_TABLE, depth); +export function isMissingTableError(error: unknown, readObject?: string, depth = 0): boolean { + return matchesDriverError(error, MISSING_TABLE, depth, readObject); } diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 598d208ba2..fb1804b217 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -3720,7 +3720,7 @@ export class ObjectQL implements IObjectQLEngine { } catch (error) { // The one benign cause only — anything else is an outage, not an // emptiness, and must not be memoised as one. - if (!isMissingTableError(error)) throw error; + if (!isMissingTableError(error, ORGANIZATION_OBJECT)) throw error; ids = []; } this.organizationProbeMemo = ids; @@ -4684,7 +4684,7 @@ export class ObjectQL implements IObjectQLEngine { // [#5979] Discriminate by error TYPE. Seeding from 0 is the truth for // exactly ONE failure reason — the table has not been provisioned, so // there are genuinely no rows and number 1 collides with nothing. - if (isMissingTableError(error)) return 0; + if (isMissingTableError(error, object)) return 0; // Every other failure (connection drop, timeout, permission denial, // query error) means the rows may well exist and simply were not seen. // Answering 0 there restarts the sequence at 1 against a table already @@ -8372,7 +8372,7 @@ export class ObjectQL implements IObjectQLEngine { // is FUNCTIONAL and scoped to this response (the answer is visibly // smaller, and the next read repairs it); nothing on this path claims to // have persisted anything. - if (!isMissingTableError(error)) { + if (!isMissingTableError(error, 'sys_file')) { this.logger.warn( 'sys_file lookup failed; file fields keep their raw ids and will render as "no file" for this read — ' + 'check storage/database availability, then re-read to hydrate', @@ -8888,7 +8888,7 @@ export class ObjectQL implements IObjectQLEngine { * believes it stored is gone. */ private reportFindFailure(object: string, error: unknown): void { - if (isMissingTableError(error)) { + if (isMissingTableError(error, object)) { this.logger.debug('Find operation failed', { object, reason: 'table-not-provisioned', @@ -11650,7 +11650,7 @@ export class ObjectQL implements IObjectQLEngine { // // No new response field and no new error code: the caller receives // the probe's own failure, envelope intact. - if (isMissingTableError(error)) continue; + if (isMissingTableError(error, childName)) continue; throw error; } // [#9362] The multi-value pushdown above is a SUPERSET, so the exact diff --git a/packages/objectql/src/lifecycle/lifecycle-service.ts b/packages/objectql/src/lifecycle/lifecycle-service.ts index e7fa653556..272b74542c 100644 --- a/packages/objectql/src/lifecycle/lifecycle-service.ts +++ b/packages/objectql/src/lifecycle/lifecycle-service.ts @@ -857,7 +857,7 @@ export class LifecycleService { // incomplete evidence" is the correct failure direction: a log cannot // bring back a reaped row, and the rows this defers are still there for // the next sweep to reap once the read succeeds. - if (!isMissingTableError(error)) throw error; + if (!isMissingTableError(error, 'sys_organization')) throw error; } } @@ -924,7 +924,7 @@ export class LifecycleService { // one's), and break the documented invariant that a sweep failure is // isolated and never thrown into the scheduler. That is strictly more // damage than the defect being repaired. - if (isMissingTableError(error)) continue; + if (isMissingTableError(error, obj.name)) continue; const msg = (error as Error)?.message ?? String(error); report.errors.push({ object: obj.name,