|
| 1 | +--- |
| 2 | +"@objectstack/service-analytics": patch |
| 3 | +--- |
| 4 | + |
| 5 | +fix(service-analytics): thirteen caller-shaped analytics refusals answer 4xx from their own envelope instead of `500` (#5716) |
| 6 | + |
| 7 | +**Observable behaviour change — read this if you alert, retry, or assert on status.** |
| 8 | +Thirteen refusal conditions in `service-analytics` (twelve `throw` sites — the |
| 9 | +cross-object measure and filter share one) used to reach the caller as |
| 10 | +`500 {"code":"ANALYTICS_QUERY_FAILED"}` on `POST /analytics/dataset/query`, and as |
| 11 | +`500 {"code":"INTERNAL_ERROR"}` on `POST /analytics/query`. They now answer **400** — |
| 12 | +`DATASET_INVALID` for the seven that are a verdict about the dataset or the whole |
| 13 | +selection, `INVALID_FIELD` for the six that name one member of the request: |
| 14 | + |
| 15 | +| refusal | now | |
| 16 | +|---|---| |
| 17 | +| dataset JOIN crosses datasources (#5115) | `DATASET_INVALID` / 400 | |
| 18 | +| `include` names a relationship the object does not have | `DATASET_INVALID` / 400 | |
| 19 | +| `include` path past the 3-hop limit | `DATASET_INVALID` / 400 | |
| 20 | +| a `dateRange` bound that is not a date | `DATASET_INVALID` / 400 | |
| 21 | +| `compareTo` names a timeDimension with no `dateRange` | `DATASET_INVALID` / 400 | |
| 22 | +| `compareTo` with no dated window to shift | `DATASET_INVALID` / 400 | |
| 23 | +| `compareTo` ambiguous between two dated windows | `DATASET_INVALID` / 400 | |
| 24 | +| cube declares no such measure (#4157) | `INVALID_FIELD` / 400 | |
| 25 | +| ObjectQL: cross-object time-dimension bucket | `INVALID_FIELD` / 400 | |
| 26 | +| ObjectQL: cross-object measure | `INVALID_FIELD` / 400 | |
| 27 | +| ObjectQL: cross-object filter | `INVALID_FIELD` / 400 | |
| 28 | +| ObjectQL: multi-hop cross-object dimension | `INVALID_FIELD` / 400 | |
| 29 | +| ObjectQL: non-recombinable measure over a cross-object dimension | `INVALID_FIELD` / 400 | |
| 30 | + |
| 31 | +Monitoring that counted these as server errors will see a 5xx disappear and a 4xx |
| 32 | +appear, and a client retrying on 5xx will stop retrying a request that cannot |
| 33 | +succeed until the request or the dataset changes. **No refusal condition moved and |
| 34 | +no message was reworded** — the same inputs are refused, in the same words; only |
| 35 | +the envelope is new. (The messages are load-bearing beyond readability: #5923's |
| 36 | +tests assert the `planCrossObject` wording, and #5717 tracks one compiler message |
| 37 | +for colliding with a downstream sniffer.) |
| 38 | + |
| 39 | +## What was wrong |
| 40 | + |
| 41 | +#5352 gave the dataset route a list of message SUBSTRINGS so six refusal families |
| 42 | +could answer 400, and #5367 retired five of those entries by giving their |
| 43 | +producers an ADR-0112 envelope. Both rounds worked from that list — and the list |
| 44 | +was only ever the refusals someone had already hit. Reading every `throw` in the |
| 45 | +package afterwards found thirteen more of exactly the same kind, which had never |
| 46 | +been on it: a typo in `compareTo`, a `dateRange` the dashboard sent, a dataset |
| 47 | +whose `include` names a relationship that does not exist. Each answered "the |
| 48 | +platform is broken" for a mistake the caller or the author could fix, on both |
| 49 | +analytics faces. |
| 50 | + |
| 51 | +**Both faces move, measured.** `/analytics/dataset/query` reads the envelope in |
| 52 | +its catch (#5352); `/analytics/query` exits through |
| 53 | +`dispatcher-plugin.errorResponseBase`, which already adopts a thrown `status` and |
| 54 | +carries the `code` (#3867/#3842) — so the cross-object refusals go from |
| 55 | +`500 INTERNAL_ERROR` to `400 INVALID_FIELD` there as well, without touching that |
| 56 | +route. The open question #5811 tracks on that face is about *withholding the |
| 57 | +message of a declared 5xx*, which none of these are. |
| 58 | + |
| 59 | +## Why two codes |
| 60 | + |
| 61 | +`dataset-refusal.ts` gains a second constructor, `invalidMemberError` |
| 62 | +(`INVALID_FIELD` / 400 + `member`/`param`/`cube`), beside `datasetInvalidError`. |
| 63 | +The split is by what the refusal is a verdict ABOUT: the dataset/selection as a |
| 64 | +whole, or one member the request named. The member family is `INVALID_FIELD` |
| 65 | +because the three shipped analytics gates already answer exactly that for the |
| 66 | +NEIGHBOURING member-level mistakes on the same request keys — `measures` (#4437), |
| 67 | +`dimensions`/`timeDimensions` (#5520), `where` (#5669) — so one class of mistake |
| 68 | +keeps one wire shape; and because these six fire on `/analytics/query` too, where |
| 69 | +there is no dataset for `DATASET_INVALID` to be about. No new code is registered: |
| 70 | +both are already in the ADR-0112 vocabulary. |
| 71 | + |
| 72 | +## What deliberately did NOT change |
| 73 | + |
| 74 | +`native-sql-strategy`'s "measure … has unrecognised type" stays a bare `Error` |
| 75 | +(an undeclared 500) although #5716 listed it as author-shaped. Measured: |
| 76 | +`Metric.type` is the closed `AggregationMetricType` enum, `metric-type-coverage.test.ts` |
| 77 | +pins that the strategy handles every member of it, the dataset compiler writes |
| 78 | +only `SUPPORTED_AGGREGATES` into a cube, and `inferMeasure` mints six known types |
| 79 | +— so no spec-valid cube can reach it. An arrival is our own drift or a host |
| 80 | +registering an unparsed cube, and blaming the caller would hide a platform fault |
| 81 | +from 5xx alerting. The two "Cube not found" guards and the two operator-drift |
| 82 | +throws stay bare for the same reason. |
| 83 | + |
| 84 | +Coverage: `unlisted-refusal-envelope.test.ts` (service-analytics) drives all |
| 85 | +thirteen refusals through the real producers — one block pinning that the refusal |
| 86 | +SET and its wording are unchanged, one pinning the envelope, one pinning the |
| 87 | +verdicts that stay 500; `analytics-dataset-unlisted-refusal-envelope.test.ts` |
| 88 | +(rest) drives eleven of them end-to-end through the route with a real |
| 89 | +`AnalyticsService`, plus three positive controls and the two sites that route |
| 90 | +cannot reach (with the measurement that explains why). |
0 commit comments