diff --git a/.changeset/analytics-coded-400-rejected.md b/.changeset/analytics-coded-400-rejected.md new file mode 100644 index 0000000000..1b0272bfff --- /dev/null +++ b/.changeset/analytics-coded-400-rejected.md @@ -0,0 +1,26 @@ +--- +'@object-ui/data-objectstack': minor +--- + +`classifyAnalyticsFailure` now reads a 400 as a refusal of the query body we +sent regardless of which ADR-0112 `code` it carries, so `aggregate()` no +longer answers a rejected filter with client-side numbers from a different +door (objectui#7755). + +Before this fix, only 400 `VALIDATION_FAILED` (and a code-less 400) threw +`AnalyticsQueryRejectedError`. Any OTHER coded 400 — `service-analytics` ships +its own 400 `INVALID_FILTER` on a filter shape it refuses — matched none of +`classifyAnalyticsFailure`'s branches and fell through to `unknown`, which +`aggregate()`'s catch has no arm for, so it silently degraded to +`aggregateViaFind`: a re-read through `find()`'s `$filter` query-string +contract, which accepts array shapes the analytics request body does not. A +filter the analytics route refused could still be answered — with a +plausible, wrong number, and no sign the request had a defect. + +The fix is a floor UNDER the existing code branches, not a replacement for +them: `NOT_IMPLEMENTED` / `ROUTE_NOT_FOUND` still win `not-installed`, +`VALIDATION_FAILED` / `UNAUTHENTICATED` / `CUBE_NOT_FOUND` still win their own +outcomes first (objectui#5721). Only a 400 that none of those four already +claimed now falls to the new floor instead of past it. An unmatched NON-400 +coded error (e.g. a coded 5xx) is unaffected and keeps degrading exactly as +before — this fix is scoped to the 400 case only. diff --git a/packages/data-objectstack/src/aggregate-capability.test.ts b/packages/data-objectstack/src/aggregate-capability.test.ts index 2fbf9fb527..93386dbbaf 100644 --- a/packages/data-objectstack/src/aggregate-capability.test.ts +++ b/packages/data-objectstack/src/aggregate-capability.test.ts @@ -37,6 +37,7 @@ * | 404 `ROUTE_NOT_FOUND` | degrade LOUDLY | * | 404/501, no `code` at all | degrade LOUDLY | * | 400 `VALIDATION_FAILED` | THROW | + * | 400, any OTHER code (or none) | THROW | * | 401 `UNAUTHENTICATED` | THROW | * | 404 `CUBE_NOT_FOUND` | THROW | * | 5xx, network, unknown code | degrade SILENTLY | @@ -45,6 +46,14 @@ * The rows that matter most are the two 404s with DIFFERENT outcomes * (objectui#5721): they are the same transport status, so only the ADR-0112 * `code` can tell them apart, and a status-first classifier fails them both. + * + * The "400, any OTHER code" row (objectui#7755) is the opposite lesson on the + * SAME classifier: a `VALIDATION_FAILED`-only check let a coded 400 the + * classifier does not enumerate — `service-analytics` ships its own 400 + * `INVALID_FILTER` on a filter shape it refuses — fall through every branch to + * `unknown`, which `aggregate()`'s catch has no arm for, so it silently + * answered the refusal with `aggregateViaFind`'s client-side numbers. Here the + * transport status alone is decisive once it is 400, regardless of code. */ import { describe, it, expect, beforeEach, vi } from 'vitest'; @@ -174,6 +183,39 @@ describe('aggregate() when the server REJECTS our query body', () => { // No silent second answer from a different code path. expect(calls.some((c) => c.includes('/api/v1/data'))).toBe(false); }); + + /* + * objectui#7755 — the pin for the coded-400 gap. `INVALID_FILTER` is the + * real code `service-analytics` ships on a 400 for a filter shape it + * refuses (spec/src/api/errors.zod.ts standard catalog), chosen deliberately + * over a made-up code so this pin cannot pass by accident. Pre-fix, + * `classifyAnalyticsFailure` only recognised `VALIDATION_FAILED` on a 400; + * any OTHER code fell through every branch to `unknown`, which + * `aggregate()`'s catch has no arm for, so it silently re-answered the + * refusal through `aggregateViaFind` — a different door with a different + * filter contract ($filter query-string parsing accepts array shapes the + * analytics body does not) — producing a plausible number for a query the + * server had just refused. + */ + it('400 INVALID_FILTER (a coded 400 the classifier does not name) also throws — it must NOT be answered by the fallback', async () => { + const { fetchImpl, calls } = makeFetch(400, { + success: false, + error: { + code: 'INVALID_FILTER', + httpStatus: 400, + message: 'Invalid filter: unsupported operator "regex" for field "stage"', + }, + }); + + const err = await makeAdapter(fetchImpl).aggregate('opportunity', SUM_BY_STAGE).catch((e) => e); + + expect(err).toBeInstanceOf(Error); + expect((err as { code?: string }).code).toBe('ANALYTICS_QUERY_REJECTED'); + expect((err as { serverCode?: string }).serverCode).toBe('INVALID_FILTER'); + expect(String(err.message)).toContain('unsupported operator'); + // The point of this card: no silent second answer from a different door. + expect(calls.some((c) => c.includes('/api/v1/data'))).toBe(false); + }); }); describe('aggregate() on a healthy analytics endpoint', () => { diff --git a/packages/data-objectstack/src/index.ts b/packages/data-objectstack/src/index.ts index 78acecd040..99f408b1c5 100644 --- a/packages/data-objectstack/src/index.ts +++ b/packages/data-objectstack/src/index.ts @@ -1126,11 +1126,18 @@ export class AnalyticsQueryRejectedError extends Error { * `ROUTE_NOT_FOUND` (framework#4019 stops mounting the routes at all). * A client-side aggregate over a scoped `find()` answers the chart * correctly, and the operator is told once that the semantic layer is off. - * - **`rejected`** — *THROW*. The server refused OUR body (400 - * `VALIDATION_FAILED`; framework#4010 validates `/analytics/query` at the - * entry). Degrading would answer our own contract violation with plausible - * numbers from a different code path and bury it — the misdirection - * framework#3878 documented. + * - **`rejected`** — *THROW*. The server refused OUR body: 400 + * `VALIDATION_FAILED` (framework#4010 validates `/analytics/query` at the + * entry) OR any other coded 400 (objectui#7755 — e.g. `service-analytics` + * ships its own 400 `INVALID_FILTER` on a filter shape it refuses). The + * status alone is decisive once it is 400: a producer that refuses the + * body and ships a code this consumer does not enumerate is refusing it + * exactly as hard as one that ships `VALIDATION_FAILED` — the refusal is + * in the transport fact, not in which code spells it. Degrading would + * answer our own contract violation with plausible numbers from a + * different code path (`find()`'s `$filter` accepts array shapes the + * analytics body does not) and bury it — the misdirection framework#3878 + * documented. * - **`unauthenticated`** — *THROW*. 401 `UNAUTHENTICATED`: the request was * refused before it ran, so it is evidence about the SESSION and none at * all about the capability. Degrading is not merely misleading here, it is @@ -1182,16 +1189,30 @@ export function classifyAnalyticsFailure( // ④ Analytics answered; the cube this query named is the thing that is missing. if (errorCodeIs({ code }, 'CUBE_NOT_FOUND')) return { kind: 'cube-not-found', code, message }; - // ⑤ Residual — the answer declared NO ADR-0112 code, so no ObjectStack route + // ⑤ A FLOOR beneath the four code branches above, not a fifth alongside them: + // any 400 is a refusal of the body WE sent, whether or not its code is one + // of the ones this function happens to enumerate (objectui#7755). Reading + // the status only in the code-less residual below let a coded-but- + // unrecognized 400 — `service-analytics` ships 400 `INVALID_FILTER` on a + // filter shape it refuses — fall through this whole ladder to `unknown`, + // and `aggregate()`'s catch has no `unknown` arm, so it silently answered + // the refusal with `aggregateViaFind`'s client-side numbers instead of + // throwing `AnalyticsQueryRejectedError`. This branch never fires for a + // 400 the branches above already claimed more specifically — none of ①-④ + // test the status, so none of them can be shadowed by moving this earlier. + if (status === 400) return { kind: 'rejected', code, message }; + + // ⑥ Residual — the answer declared NO ADR-0112 code, so no ObjectStack route // wrote it (a proxy, a gateway, a host with no API mounted). Only here is // the bare status the best signal available, and only because every code // branch has already declined: this face's own 404s all ship a `code`, so a - // code-less 404 cannot be the unknown-cube case. + // code-less 404 cannot be the unknown-cube case. (The 400 case that used to + // live here moved up to ⑤ so it applies whether or not a code is present; + // this residual would never have reached it anyway once ⑤ runs first.) if (code === undefined) { if (status !== undefined && ANALYTICS_ABSENT_STATUSES.includes(status)) { return { kind: 'not-installed', code, message }; } - if (status === 400) return { kind: 'rejected', code, message }; if (status === 401) return { kind: 'unauthenticated', code, message }; }