|
| 1 | +--- |
| 2 | +"@objectstack/service-analytics": patch |
| 3 | +--- |
| 4 | + |
| 5 | +fix(analytics): a dataset refusal that declares an ADR-0112 envelope is never degraded to an empty result (#5717) |
| 6 | + |
| 7 | +`queryDataset` wraps execution in a catch that exists for one deliberate reason |
| 8 | +(#5033): a widget whose backing object is not mounted in this kernel renders |
| 9 | +"no data" instead of failing with a 500. The criterion for "not mounted" was |
| 10 | +`isMissingSourceError` — a substring match over the error MESSAGE. So the |
| 11 | +leniency was available to any error that happened to phrase itself like a |
| 12 | +driver, and #5352 / #5367's finding on the REST face — "the wire shape of an |
| 13 | +error family must not be a property of its wording" — applied here one level |
| 14 | +worse: the outcome was not a wrong status code but a **silent empty result**. |
| 15 | +No exception, no 4xx, no 5xx; one `warn` line and a confident empty chart, which |
| 16 | +is the "populated table, Total Spend: 0" symptom #5033 was filed about. |
| 17 | + |
| 18 | +One refusal already matched. `dataset-compiler.ts` refuses an `include` naming a |
| 19 | +relationship the object graph does not have with |
| 20 | + |
| 21 | +> `[dataset-compiler] dataset "X" includes relationship "R" which does not exist on object "O".` |
| 22 | +
|
| 23 | +which carries both `relation` (inside "relationship") and `does not exist` — and |
| 24 | +that conjunction was the postgres limb. It has never gone off for one reason: |
| 25 | +`queryDataset` compiles **before** the try, so that throw has never been inside |
| 26 | +the catch's reach. A mine, wired and unarmed. |
| 27 | + |
| 28 | +**Two independent defences, so the disarming does not depend on either one.** |
| 29 | + |
| 30 | +- **The criterion (main change).** An error carrying an ADR-0112 envelope — |
| 31 | + numeric `status` + non-empty `code`, the same structural fact |
| 32 | + `rest-server.ts`'s `/analytics/dataset/query` catch reads — is re-thrown |
| 33 | + untouched, ahead of any message inspection. Its producer already answered the |
| 34 | + classification question. The status RANGE is deliberately not part of the |
| 35 | + test: a `DATASET_INVALID` / 400 rendered as an empty grid is the loud case, |
| 36 | + but a declared 5xx (`READ_SCOPE_COMPILE_FAILED` — an RLS lowering that failed |
| 37 | + closed) is if anything worse to swallow, since nobody is told at all. |
| 38 | +- **The sniffer.** Its postgres limb is now anchored to postgres's actual |
| 39 | + wording (`relation "x" does not exist`) instead of "any sentence containing |
| 40 | + both words" — the same pattern the sibling `missingSourceRelation` already |
| 41 | + used, so "is something missing" and "what is missing" can no longer disagree. |
| 42 | + |
| 43 | +**Observable behaviour change — read this if you alert on empty widgets.** The |
| 44 | +guarantee is new, not the status of any shipped message: measured over the 13 |
| 45 | +real wordings this repo carries (three driver families including sql-prefixed |
| 46 | +and schema-qualified forms, the framework's not-registered signals, and this |
| 47 | +package's own refusals), exactly one verdict moves — the compiler refusal above, |
| 48 | +which reaches callers as `400 DATASET_INVALID` either way because its throw site |
| 49 | +sits outside the try. What changes is that a caller-shaped refusal raised |
| 50 | +**during execution** can no longer become `{rows: [], fields: [], totals: []}` |
| 51 | +by phrasing alone: it now propagates and the route answers its declared code |
| 52 | +(4xx as itself, declared 5xx through `ANALYTICS_QUERY_FAILED`). A dashboard that |
| 53 | +silently rendered an empty chart for such a refusal will now surface the error. |
| 54 | + |
| 55 | +**#5033's leniency is untouched, and that is asserted rather than claimed.** A |
| 56 | +bare driver error is still classified by its words and still degrades: `no such |
| 57 | +table` (sqlite/libsql), postgres's real `relation "x" does not exist`, mysql's |
| 58 | +`doesn't exist`, the framework's not-registered signals — and a bare error |
| 59 | +naming a JOINED table still fails loudly as a cross-datasource dataset. Those |
| 60 | +cases are green in all four states of the reverse verification |
| 61 | +(`dataset-degradation-envelope.test.ts`), including with both defences reverted. |
| 62 | + |
| 63 | +The compile point deliberately stays outside the try. Moving it in would newly |
| 64 | +expose the compiler's own bare invariants and the host-supplied relationship |
| 65 | +resolver to this degradation path — widening leniency in the opposite direction |
| 66 | +from the fix. |
0 commit comments