From 238ff4a712621c48c4b1f02cba4e654865f14943 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 15:32:20 +0000 Subject: [PATCH] fix(service-analytics): the dataset degradation path reads the ADR-0112 envelope before the message (#5717) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `queryDataset`'s catch degrades to an empty result for the #5033 case (a widget whose backing object is not mounted in this kernel). Its criterion was `isMissingSourceError` — a substring match over the error MESSAGE — so the leniency was available to any error phrased like a driver, and the outcome was not a wrong status code but a silent empty result: no exception, no 4xx, no 5xx, one warn and a confident empty chart. `dataset-compiler.ts`'s "…includes relationship "R" which does not exist on object "O"." already matched (both `relation`, inside "relationship", and `does not exist`). It has never gone off only because the compile point sits outside the try — a mine, wired and unarmed. Two independent defences: - an error carrying an ADR-0112 envelope (numeric `status` + non-empty `code`, the same structural fact `rest-server.ts`'s analytics catch reads) is re-thrown untouched, ahead of any message inspection. Status range is deliberately not part of the test: a declared 5xx is if anything worse to swallow than a 400; - the postgres limb is anchored to postgres's actual wording (`relation "x" does not exist`) instead of "any sentence with both words" — the pattern the sibling `missingSourceRelation` already used. Measured over the 13 real wordings this repo carries, exactly one verdict moves and it is that compiler refusal; no driver wording changes, so #5033's leniency for bare driver errors is untouched (asserted in all four reverse-verification states, not merely claimed). The compile point deliberately stays outside the try — moving it in would newly expose the compiler's bare invariants and the host-supplied relationship resolver to this degradation path. Residue filed as #6035 (postgres's write-path missing-COLUMN wording carries a whole missing-relation phrase inside it; dormant on a read-only face). Fixes #5717 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015a5qkLzpGXhLL2F5gvJ7dD --- ...tics-dataset-degradation-envelope-first.md | 66 ++++ .../dataset-degradation-envelope.test.ts | 335 ++++++++++++++++++ .../src/analytics-service.ts | 72 +++- 3 files changed, 471 insertions(+), 2 deletions(-) create mode 100644 .changeset/analytics-dataset-degradation-envelope-first.md create mode 100644 packages/services/service-analytics/src/__tests__/dataset-degradation-envelope.test.ts diff --git a/.changeset/analytics-dataset-degradation-envelope-first.md b/.changeset/analytics-dataset-degradation-envelope-first.md new file mode 100644 index 0000000000..e61e9acedc --- /dev/null +++ b/.changeset/analytics-dataset-degradation-envelope-first.md @@ -0,0 +1,66 @@ +--- +"@objectstack/service-analytics": patch +--- + +fix(analytics): a dataset refusal that declares an ADR-0112 envelope is never degraded to an empty result (#5717) + +`queryDataset` wraps execution in a catch that exists for one deliberate reason +(#5033): a widget whose backing object is not mounted in this kernel renders +"no data" instead of failing with a 500. The criterion for "not mounted" was +`isMissingSourceError` — a substring match over the error MESSAGE. So the +leniency was available to any error that happened to phrase itself like a +driver, and #5352 / #5367's finding on the REST face — "the wire shape of an +error family must not be a property of its wording" — applied here one level +worse: the outcome was not a wrong status code but a **silent empty result**. +No exception, no 4xx, no 5xx; one `warn` line and a confident empty chart, which +is the "populated table, Total Spend: 0" symptom #5033 was filed about. + +One refusal already matched. `dataset-compiler.ts` refuses an `include` naming a +relationship the object graph does not have with + +> `[dataset-compiler] dataset "X" includes relationship "R" which does not exist on object "O".` + +which carries both `relation` (inside "relationship") and `does not exist` — and +that conjunction was the postgres limb. It has never gone off for one reason: +`queryDataset` compiles **before** the try, so that throw has never been inside +the catch's reach. A mine, wired and unarmed. + +**Two independent defences, so the disarming does not depend on either one.** + +- **The criterion (main change).** An error carrying an ADR-0112 envelope — + numeric `status` + non-empty `code`, the same structural fact + `rest-server.ts`'s `/analytics/dataset/query` catch reads — is re-thrown + untouched, ahead of any message inspection. Its producer already answered the + classification question. The status RANGE is deliberately not part of the + test: a `DATASET_INVALID` / 400 rendered as an empty grid is the loud case, + but a declared 5xx (`READ_SCOPE_COMPILE_FAILED` — an RLS lowering that failed + closed) is if anything worse to swallow, since nobody is told at all. +- **The sniffer.** Its postgres limb is now anchored to postgres's actual + wording (`relation "x" does not exist`) instead of "any sentence containing + both words" — the same pattern the sibling `missingSourceRelation` already + used, so "is something missing" and "what is missing" can no longer disagree. + +**Observable behaviour change — read this if you alert on empty widgets.** The +guarantee is new, not the status of any shipped message: measured over the 13 +real wordings this repo carries (three driver families including sql-prefixed +and schema-qualified forms, the framework's not-registered signals, and this +package's own refusals), exactly one verdict moves — the compiler refusal above, +which reaches callers as `400 DATASET_INVALID` either way because its throw site +sits outside the try. What changes is that a caller-shaped refusal raised +**during execution** can no longer become `{rows: [], fields: [], totals: []}` +by phrasing alone: it now propagates and the route answers its declared code +(4xx as itself, declared 5xx through `ANALYTICS_QUERY_FAILED`). A dashboard that +silently rendered an empty chart for such a refusal will now surface the error. + +**#5033's leniency is untouched, and that is asserted rather than claimed.** A +bare driver error is still classified by its words and still degrades: `no such +table` (sqlite/libsql), postgres's real `relation "x" does not exist`, mysql's +`doesn't exist`, the framework's not-registered signals — and a bare error +naming a JOINED table still fails loudly as a cross-datasource dataset. Those +cases are green in all four states of the reverse verification +(`dataset-degradation-envelope.test.ts`), including with both defences reverted. + +The compile point deliberately stays outside the try. Moving it in would newly +expose the compiler's own bare invariants and the host-supplied relationship +resolver to this degradation path — widening leniency in the opposite direction +from the fix. diff --git a/packages/services/service-analytics/src/__tests__/dataset-degradation-envelope.test.ts b/packages/services/service-analytics/src/__tests__/dataset-degradation-envelope.test.ts new file mode 100644 index 0000000000..f50e4ab7ef --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/dataset-degradation-envelope.test.ts @@ -0,0 +1,335 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5717] The dataset degradation path judges an error by its ENVELOPE first, + * and only then by its words. + * + * ## What was wrong + * + * `queryDataset` wraps execution in a catch whose whole job is #5033's + * deliberate leniency: a widget whose backing object is not mounted in this + * kernel renders "no data" instead of a 500. The criterion for "not mounted" + * was `isMissingSourceError` — a substring match over the error MESSAGE. So the + * degradation was available to any error that happened to phrase itself like a + * driver, and #5352/#5367's finding on the REST face ("the HTTP status of an + * error family must not be a property of its wording") applied here one level + * worse: the outcome was not a wrong status code but a SILENT EMPTY RESULT — + * no exception, no 4xx, no 5xx, one `warn`, and a confident empty chart. + * + * One refusal already matched. `dataset-compiler.ts`'s + * + * `[dataset-compiler] dataset "X" includes relationship "R" which does not + * exist on object "O".` + * + * carries both `relation` (inside "relationship") and `does not exist`, which + * is exactly what the postgres limb tested for. It never went off for one + * reason: `queryDataset` compiles BEFORE the try, so that throw has never been + * inside the catch's reach. A mine, wired and unarmed — armed by any future + * edit that moves the compile in, or by any executor/strategy refusal that ever + * phrases itself the same way. + * + * ## The two defences, and which case pins which + * + * They are independent, and this file keeps them separable on purpose: + * + * - **B (the criterion)** — an error carrying an ADR-0112 envelope (numeric + * `status` + non-empty `code`) is re-thrown untouched, whatever it says. + * Its producer already answered the classification question. Pinned by the + * cases whose message STILL matches the sniffer after C: the real + * `CUBE_NOT_FOUND` / 404 gate error (which says "…is not a registered + * object…") and the declared-5xx case. + * - **C (the sniffer)** — the postgres limb is anchored to postgres's actual + * wording instead of "any sentence with both words", so the compiler + * refusal no longer matches it even stripped of its envelope. Pinned by the + * BARE compiler-wording case. + * + * The compiler refusal itself is therefore covered TWICE — by B because #5963 + * gave it `DATASET_INVALID` / 400, and by C because the anchor no longer reads + * "relationship" as "relation". Both are asserted, and the pair is the reason + * the mine is disarmed rather than merely stepped around. + * + * ## What deliberately did NOT change + * + * #5033's semantics, verbatim: a BARE driver error is still classified by its + * words and still degrades to `{rows: [], fields: [], totals: []}` + a `warn` + * (`no such table`, postgres's real `relation "x" does not exist`), and a bare + * error naming a JOINED table still fails loudly as a cross-datasource dataset. + * The compile point also stays OUTSIDE the try — see the PR for the + * measurement: moving it in would newly expose the compiler's own bare + * invariants and the host-supplied relationship resolver to the degradation + * path, which widens leniency in the opposite direction from this fix. + * + * ## Reverse verification — direction predicted BEFORE running + * + * Ordinary direction (red), but SPLIT, and the split is the point: reverting + * one defence leaves the other holding the compiler case, so "revert the fix + * and watch it all go red" would be a false expectation here. + * + * - revert **B** only (drop the `hasDeclaredErrorEnvelope` branch): + * `CUBE_NOT_FOUND` and declared-5xx go RED (both degrade to an empty + * result); both compiler cases stay GREEN — C alone still saves them. + * - revert **C** only (restore `includes('relation') && includes('does not + * exist')`): the BARE compiler case goes RED; the enveloped one stays + * GREEN — B alone still saves it. + * - revert BOTH: all four go RED. + * + * MEASURED, one revert at a time, on this file (11 cases): + * B only reverted → **2 red / 9 green** — `CUBE_NOT_FOUND`, declared-5xx. + * C only reverted → **1 red / 10 green** — the bare compiler wording. + * both reverted → **4 red / 7 green** — those three plus the ENVELOPED + * compiler wording, the mine itself, which is the one case that needs BOTH + * defences gone before it fires. (This line predicted "3" first: the union of + * the two single-revert sets is 3, and the mine is the fourth — it is exactly + * the case neither single revert can reach. The prediction is left corrected + * rather than quietly fixed, because that fourth row IS the finding.) + * + * The #5033 cases are green in every one of those four states, which is what + * "the deliberate leniency is untouched" means as evidence rather than as a + * claim. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { DatasetSchema } from '@objectstack/spec/ui'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import { AnalyticsService } from '../analytics-service.js'; +import { compileDataset } from '../dataset-compiler.js'; + +/** The ADR-0112 fields the REST boundary classifies on. */ +interface Refusal extends Error { + code?: unknown; + status?: unknown; +} + +const EMPTY = { rows: [], fields: [], totals: [] }; + +const dataset = DatasetSchema.parse({ + name: 'sales', + label: 'Sales', + object: 'opportunity', + include: ['account'], + dimensions: [{ name: 'region', field: 'account.region', type: 'string' }], + measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }], +}); + +const SELECTION = { dimensions: ['region'], measures: ['revenue'] }; +const CTX = { tenantId: 'org_A' } as ExecutionContext; + +function logger() { + return { info: vi.fn(), debug: vi.fn(), warn: vi.fn(), error: vi.fn(), child: vi.fn() } as any; +} + +/** + * A service whose execution throws `thrown` — the only way into the catch under + * test. `isRegisteredObject` answers true so the #5033 cross-datasource triage + * behaves as it does in a real kernel. + */ +function serviceThatThrows(thrown: unknown, log = logger()) { + return new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }), + executeRawSql: async () => { throw thrown; }, + isRegisteredObject: () => true, + logger: log, + }); +} + +async function refusalFrom(thunk: () => unknown | Promise): Promise { + try { + await thunk(); + return undefined; + } catch (e) { + return e as Refusal; + } +} + +// ── the real producers, captured rather than re-typed ──────────────────────── + +/** + * `dataset-compiler.ts`'s resolveHop refusal — the exact error object the real + * producer builds, wording and `DATASET_INVALID` / 400 envelope included. Its + * throw site is outside `queryDataset`'s try today; re-throwing it from INSIDE + * the try is precisely the "what if the compile moved in" question the mine + * poses, asked without moving anything. + */ +async function compilerRelationshipRefusal(): Promise { + const err = await refusalFrom(() => + compileDataset( + DatasetSchema.parse({ + name: 'sales', + label: 'Sales', + object: 'opportunity', + include: ['bogus'], + dimensions: [{ name: 'stage', field: 'stage', type: 'string' }], + measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }], + }), + () => undefined, + ), + ); + if (!err) throw new Error('the compiler accepted an unresolvable relationship — fixture is stale'); + return err; +} + +/** + * `analytics-service.ts`'s own `assertInferableCube` gate (#3867) — + * `CUBE_NOT_FOUND` / 404, and its message ends "…and it is not a registered + * object either…", which matches the sniffer's framework limb VERBATIM. Same + * shape of luck as the compiler refusal: on the dataset path `queryDataset` + * registers the compiled cube before executing, so this gate is not reachable + * there today. It is the case that isolates B, because no narrowing of the + * postgres limb can save it. + */ +async function cubeNotFoundRefusal(): Promise { + const svc = new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }), + executeRawSql: async () => [], + isRegisteredObject: () => false, + logger: logger(), + }); + const err = await refusalFrom(() => svc.query({ cube: 'ghost', measures: ['count'] })); + if (!err) throw new Error('the cube-existence gate accepted an unregistered name — fixture is stale'); + return err; +} + +// ───────────────────────────────────────────────────────────────────────────── + +describe('[#5717] an error that declares an ADR-0112 envelope is never degraded to an empty result', () => { + it('the fixtures are the real producers, and both still match the sniffer’s framework limb', async () => { + // Guards the two captures above against silently becoming vacuous — the + // #5046 lesson: a case that passes because nothing is produced. + const compilerErr = await compilerRelationshipRefusal(); + expect(compilerErr.message).toMatch(/includes relationship "bogus" which does not exist on object "opportunity"/); + expect(compilerErr.code).toBe('DATASET_INVALID'); + expect(compilerErr.status).toBe(400); + + const cubeErr = await cubeNotFoundRefusal(); + expect(cubeErr.message).toMatch(/is not a registered object/); + expect(cubeErr.code).toBe('CUBE_NOT_FOUND'); + expect(cubeErr.status).toBe(404); + }); + + it('CUBE_NOT_FOUND / 404 propagates — the case that no message narrowing can save', async () => { + // Isolates defence B: "…is not a registered object either…" is a verbatim + // sniffer hit before AND after the postgres limb was anchored, so only the + // envelope check keeps a 404 from rendering as "no data". + const log = logger(); + const refusal = await cubeNotFoundRefusal(); + const err = await refusalFrom(() => + serviceThatThrows(refusal, log).queryDataset(dataset, SELECTION, CTX), + ); + expect(err, 'a 404 was swallowed into an empty grid').toBeInstanceOf(Error); + expect(err?.code).toBe('CUBE_NOT_FOUND'); + expect(err?.status).toBe(404); + // Not degraded ⇒ not warned about as an absent backing object either. + expect(log.warn).not.toHaveBeenCalledWith(expect.stringContaining('returning an empty result')); + }); + + it('a DECLARED 5xx propagates too — the envelope, not its range, is the criterion', async () => { + // A declared server fault (`read-scope-sql.ts`'s family — an RLS lowering + // that failed closed) is if anything worse to swallow than a 400: the + // widget would render a confident empty chart for a fault nobody is told + // about. The WORDING here is synthesised, deliberately: the rule under test + // is "the producer classified it", so the case must carry a message the + // sniffer would otherwise take (`unknown object`), which today's ten real + // read-scope messages do not. + const declared = Object.assign( + new Error('[read-scope-sql] read scope references unknown object "crm_account" (fail-closed).'), + { code: 'READ_SCOPE_COMPILE_FAILED', status: 500 }, + ); + const err = await refusalFrom(() => serviceThatThrows(declared).queryDataset(dataset, SELECTION, CTX)); + expect(err?.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(err?.status).toBe(500); + }); + + it('the compiler’s relationship refusal is not swallowed — the mine, disarmed', async () => { + // The direct assertion #5717 asks for: the exact refusal, thrown from + // INSIDE the try (which is what moving the compile point would do), reaches + // the caller with its 400 envelope intact instead of becoming an empty grid. + const refusal = await compilerRelationshipRefusal(); + const log = logger(); + const err = await refusalFrom(() => + serviceThatThrows(refusal, log).queryDataset(dataset, SELECTION, CTX), + ); + expect(err, 'the mine went off — a 400 refusal became an empty result').toBeInstanceOf(Error); + expect(err?.code).toBe('DATASET_INVALID'); + expect(err?.status).toBe(400); + expect(String(err?.message)).toMatch(/includes relationship "bogus" which does not exist/); + expect(log.warn).not.toHaveBeenCalledWith(expect.stringContaining('returning an empty result')); + }); + + it('…and stays unswallowed with the envelope STRIPPED — defence C, standing alone', async () => { + // The same wording as a bare `Error`: what this refusal was before #5963, + // and what any future re-worded refusal looks like. Nothing here declares + // anything, so only the anchored postgres limb can tell it apart from a + // driver reporting a missing table — and it does: "relationship" is not + // `relation "x"`, and the missing thing is a RELATIONSHIP, not a table. + const refusal = await compilerRelationshipRefusal(); + const err = await refusalFrom(() => + serviceThatThrows(new Error(refusal.message)).queryDataset(dataset, SELECTION, CTX), + ); + expect(err, 'a bare refusal with the compiler wording was still swallowed').toBeInstanceOf(Error); + expect(String(err?.message)).toMatch(/includes relationship "bogus" which does not exist/); + // Bare ⇒ nothing to classify it with; it must arrive unclassified, not + // wearing an envelope this layer invented for it. + expect(err?.code).toBeUndefined(); + expect(err?.status).toBeUndefined(); + }); + + it('an enveloped 400 whose message says nothing driver-ish propagates as it always did', async () => { + // The neighbour a character away: this one never matched the sniffer, so it + // propagated before this change too. It is here so the block is not read as + // "only sniffer-matching refusals are protected" — the criterion is the + // envelope, and the guarantee is now independent of what the message says. + const { datasetInvalidError } = await import('../dataset-refusal.js'); + const err = await refusalFrom(() => + serviceThatThrows(datasetInvalidError('[dataset-executor] compareTo needs a dated window to shift.')) + .queryDataset(dataset, SELECTION, CTX), + ); + expect(err?.code).toBe('DATASET_INVALID'); + expect(err?.status).toBe(400); + }); +}); + +describe('[#5717] #5033’s deliberate leniency for BARE driver errors is untouched', () => { + it('sqlite/libsql "no such table" still degrades to an empty result, with the warn', async () => { + const log = logger(); + const result = await serviceThatThrows( + new Error('SELECT COUNT(*) FROM "opportunity" - no such table: opportunity'), + log, + ).queryDataset(dataset, SELECTION, CTX); + expect(result).toEqual(EMPTY); + expect(log.warn).toHaveBeenCalledWith( + expect.stringContaining('backing object "opportunity" is unavailable'), + ); + }); + + it('postgres’s REAL wording still degrades — the anchor keeps the limb it was written for', async () => { + const result = await serviceThatThrows( + new Error('select "region" from "opportunity" - relation "opportunity" does not exist'), + ).queryDataset(dataset, SELECTION, CTX); + expect(result).toEqual(EMPTY); + }); + + it('mysql’s "doesn’t exist" still degrades', async () => { + const result = await serviceThatThrows( + new Error("Table 'app.opportunity' doesn't exist"), + ).queryDataset(dataset, SELECTION, CTX); + expect(result).toEqual(EMPTY); + }); + + it('a bare missing JOINED table still fails loudly as a cross-datasource dataset (#5033)', async () => { + // The louder half of #5033: the base table resolved, a JOINED one did not, + // and the joined object IS registered — a topology error, never "no data". + // Regression guard that the new envelope branch did not short-circuit past it. + const err = await refusalFrom(() => + serviceThatThrows(new Error('no such table: account')).queryDataset(dataset, SELECTION, CTX), + ); + expect(String(err?.message)).toMatch(/cannot be executed as one statement/); + expect(String(err?.message)).toMatch(/A dataset JOIN cannot cross datasources/); + }); + + it('a bare non-missing-source error still surfaces (real query bugs are not hidden)', async () => { + const err = await refusalFrom(() => + serviceThatThrows(new Error('syntax error near "FROM"')).queryDataset(dataset, SELECTION, CTX), + ); + expect(String(err?.message)).toMatch(/syntax error/); + }); +}); diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index d39a779d59..1a063a9fa4 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -80,6 +80,31 @@ type AnalyticsResultWithDrill = AnalyticsResult & { drillRanges?: Array>; }; +/** + * [#5717] Does this error carry an ADR-0112 envelope — i.e. did its PRODUCER + * already classify it? + * + * The structural fact, read exactly as `rest-server.ts`'s + * `/analytics/dataset/query` catch reads it (`envelopeStatus`/`envelopeCode`): + * a numeric `status` plus a non-empty string `code`. Deliberately the SAME + * predicate rather than a second dialect of "looks enveloped" — a producer that + * ships half an envelope has a bug of its own and must be found, not guessed at + * from either end of the wire. + * + * Status RANGE is deliberately not part of it. The 4xx case is the loud one + * (#5717's own: a `DATASET_INVALID` / 400 refusal must reach the caller as a + * 400, never as an empty grid), but a DECLARED 5xx — `read-scope-sql.ts`'s + * `READ_SCOPE_COMPILE_FAILED` / 500 fail-closed refusals — is if anything worse + * to swallow: an RLS lowering that failed closed, rendered as a confident empty + * chart, is a server fault nobody is told about. Either way the producer has + * ANSWERED the classification question, and {@link isMissingSourceError} — a + * heuristic over DRIVER phrasing — has no business re-opening it. + */ +function hasDeclaredErrorEnvelope(err: unknown): boolean { + const e = err as { code?: unknown; status?: unknown } | null | undefined; + return typeof e?.status === 'number' && typeof e?.code === 'string' && e.code.length > 0; +} + /** * Detect the "backing object/table isn't present in this kernel" class of * error so a dataset query can degrade to an empty result instead of failing @@ -88,12 +113,43 @@ type AnalyticsResultWithDrill = AnalyticsResult & { * framework's own unknown-object signal. Deliberately scoped to MISSING SOURCE * (table/object/relation) — not column/syntax errors, which stay hard failures * so real query bugs still surface. + * + * ⚠️ It is a heuristic over driver PHRASING, so it is the SECOND question the + * degradation path asks, never the first: {@link hasDeclaredErrorEnvelope} runs + * ahead of it (#5717), and only an error whose producer declared nothing is + * classified by its words here. + * + * [#5717] The postgres limb is ANCHORED to postgres's actual wording + * (`relation "x" does not exist`, relation name quoted or bare) instead of the + * `includes('relation') && includes('does not exist')` conjunction it used to + * be. That conjunction matched any sentence carrying both words — including + * `dataset-compiler.ts`'s `… includes relationship "R" which does not exist on + * object "O"`, where the "relation" is inside "relationship" and the missing + * thing is a RELATIONSHIP, not a table. The anchor is the same pattern the + * sibling {@link missingSourceRelation} already uses for postgres (and the same + * shape as `metadata/src/utils/schema-sync-errors.ts`), so "is something + * missing" and "what is missing" can no longer disagree on this limb. + * + * MEASURED over the wordings this repo actually carries — 13 strings: the three + * driver families' phrasings (including sql-prefixed and schema-qualified + * forms), the framework's not-registered signals, and this package's own + * refusals — exactly ONE verdict moves, the compiler refusal above. No driver + * wording changes, which is what makes this a narrowing rather than a + * behaviour change for #5033's leniency. + * + * Known residue, filed as #6035 rather than widened into here: postgres spells + * a missing COLUMN on the write path as `column "c" of relation "t" does not + * exist`, which carries a whole missing-relation phrase inside it and so is a + * hit both before and after this anchor. Dormant on a read-only face (a SELECT + * says `column "c" does not exist`, no `relation`), but it is the one case + * where this predicate is still wider than the paragraph above it. */ function isMissingSourceError(err: unknown): boolean { - const msg = String((err as { message?: unknown })?.message ?? err ?? '').toLowerCase(); + const raw = String((err as { message?: unknown })?.message ?? err ?? ''); + const msg = raw.toLowerCase(); return ( msg.includes('no such table') || // sqlite / libsql - (msg.includes('relation') && msg.includes('does not exist')) || // postgres + /relation\s+[`"']?[A-Za-z0-9_$.]+[`"']?\s+does not exist/i.test(raw) || // postgres msg.includes("doesn't exist") || // mysql ("table ... doesn't exist") msg.includes('not registered') || // framework: object not in registry msg.includes('unknown object') || @@ -789,10 +845,22 @@ export class AnalyticsService implements IAnalyticsService { // wearing a new cause: the base table is right there, and the widget would keep // rendering the confident `0` this issue is about. So triage by WHICH relation // the driver named, and let a cross-datasource dataset fail loudly. + // + // #5717 — and the leniency is scoped to errors NOBODY classified. The + // triage below reads message text, so before it runs, an error that carries + // an ADR-0112 envelope is re-thrown untouched: its producer already said + // what it is, and a `DATASET_INVALID` / 400 turned into `{rows: []}` is the + // #5033 symptom wearing the opposite disguise — the caller's own mistake + // reported as "no data", with no exception, no 4xx, no 5xx, just a warn and + // a confident empty chart. See {@link hasDeclaredErrorEnvelope}. let result: AnalyticsResult; try { result = await new DatasetExecutor(this, orderLabels).execute(compiled, selection, context); } catch (err) { + // The producer answered the classification question — the route's + // envelope reader serves it (4xx as itself, declared 5xx through the + // `ANALYTICS_QUERY_FAILED` path). Nothing here may re-judge it by wording. + if (hasDeclaredErrorEnvelope(err)) throw err; if (isMissingSourceError(err)) { const missing = missingSourceRelation(err); const detail = String((err as Error)?.message ?? err);