fix(rest): relay a producer-declared 5xx on /analytics/dataset/query instead of collapsing it to 500 ANALYTICS_QUERY_FAILED

.changeset/analytics-dataset-query-declared-5xx-relay.md

@@ -0,0 +1,60 @@
+---
+"@objectstack/rest": patch
+---
+
+fix(rest): `POST /analytics/dataset/query` relays a producer-declared 5xx instead of collapsing it to `500 ANALYTICS_QUERY_FAILED` (#11718)
+
+**Response-contract change on a shipped public route.** The door is
+`POST /api/v1/analytics/dataset/query` (and its environment-scoped twin). An
+error whose producer declared a 5xx `status` now reaches the client with **that
+status and that ADR-0112 `code`**, where the route previously answered a
+hand-built `500` `ANALYTICS_QUERY_FAILED` for every one of them.
+
+Measured door-to-door before the fix, one error object, both handlers driven in
+process:
+
+| face | answer |
+|---|---|
+| `POST /api/v1/data/:object` | `503` — `{"error":"Internal server error","code":"SERVICE_UNAVAILABLE"}` |
+| `POST /api/v1/analytics/dataset/query` | `500` — `{"code":"ANALYTICS_QUERY_FAILED","error":"Internal server error"}` |
+
+`/data` is the reference and does not move. Its relay is #5582's ruling — `502`
+and `503` are `isExpectedDataStatus` lifecycle outcomes that proxies and retry
+policies read differently from a `500`, so collapsing them destroys the
+declaration — and that ruling never reached this route because the analytics
+catch built its 5xx envelope by hand. The **sibling** analytics face
+`/analytics/query` already relayed both halves through
+`dispatcher-plugin.errorResponseBase`, so this door was the only one of three
+overwriting a producer's declaration.
+
+The repair imports `/data`'s own arm rather than restating it: the branch is
+lifted into `declaredServerFaultAnswer` in `error-response.ts` and read by both
+doors, the same way the 4xx arm already imports `classifiedRefusalAnswer`. A
+third local opinion at this boundary is how the two faces came to disagree.
+
+**Not a re-opening of #5352/#5367/#5811 — the prose is still withheld.** A
+declared server fault's message is still replaced by the generic sentence, from
+the same shared arm, and the full original text still reaches the operator: the
+`logError` line runs *before* the relay branch and is unconditional, so a
+producer cannot buy its way past the operator's log with a declared status. What
+moves is the classification the producer declared and this route was
+overwriting.
+
+**What callers see change:**
+
+- A declared `{ status: 503, code: 'SERVICE_UNAVAILABLE' }` → `503`
+ `SERVICE_UNAVAILABLE` (was `500` `ANALYTICS_QUERY_FAILED`).
+- An unregistered declared code demotes exactly as `/data` demotes it — `503`
+ `{"code":"SERVICE_UNAVAILABLE","declaredCode":"WAREHOUSE_UNAVAILABLE"}` (#9232).
+- `read-scope-sql`'s ten fail-closed RLS refusals answer `500`
+ `READ_SCOPE_COMPILE_FAILED` instead of `500` `ANALYTICS_QUERY_FAILED`. Their
+ 2026-08-06 ruling is untouched in substance — still a SERVER fault, still
+ `500`, still with the RLS policy content withheld from the body and intact in
+ the log — and the code they now carry is the one they declare and the one the
+ sibling `/analytics/query` face has always shipped to clients.
+
+**Unchanged:** an *undeclared* fault. No declared status means nothing to relay,
+so it keeps `500` `ANALYTICS_QUERY_FAILED` and #5667's tiering, which leaves a
+self-authored fault readable. A declared **4xx** is untouched — that band is
+arms ① and ①b, and the half-envelope rule (a 4xx status with no code invents no
+code) still stands.

packages/rest/src/analytics-dataset-dimension-gate.test.ts

@@ -325,7 +325,9 @@ describe('[#5520] the 500 body no longer ships driver internals', () => {
  // messages name RLS policy fields — the 5xx branch withholds the text. So
  // what this half now guards is that #5520's `looksLikeInternalErrorLeak`
  // withhold and #5367's declared-server-fault withhold COMPOSE rather than
- // fight: same 500 code, message withheld, log intact.
+ // fight: same 500, message withheld, log intact. [#11718] The CODE on the
+ // wire is now the producer's own `READ_SCOPE_COMPILE_FAILED`; the status
+ // and the withhold — this half's actual subject — do not move.
  const c = await post(
  buildRoute(async () =>
  throwingAnalytics(
@@ -338,7 +340,12 @@ describe('[#5520] the 500 body no longer ships driver internals', () => {
  { dataset, selection: { measures: ['account_count'], dimensions: ['industry'] } },
  );
  expect(c.statusCode).toBe(500);
- expect(c.body.code).toBe('ANALYTICS_QUERY_FAILED');
+ // [#11718] The declared code is RELAYED rather than overwritten with
+ // `ANALYTICS_QUERY_FAILED`. What this half guards is unchanged and is
+ // asserted on the next two lines: #5520's `looksLikeInternalErrorLeak`
+ // withhold and #5367's declared-server-fault withhold still COMPOSE rather
+ // than fight — same 500, message withheld, no policy field in the body.
+ expect(c.body.code).toBe('READ_SCOPE_COMPILE_FAILED');
  expect(c.body.error).toBe(INTERNAL_ERROR_MESSAGE);
  expect(String(c.body.error)).not.toMatch(/owner_email/);
  });

packages/rest/src/analytics-filter-refusal-envelope.test.ts

@@ -284,13 +284,18 @@ describe('[#5352 → #5367] the message-sniffing fallback is GONE', () => {
  // So the same input is asserted the other way round — and the bare form, which
  // is what the list used to rescue, is asserted too. Between them they pin that
  // no message test survives anywhere in this catch.
- it('read-scope-sql: the DECLARED 500 → 500 ANALYTICS_QUERY_FAILED, policy content withheld', async () => {
+ it('read-scope-sql: the DECLARED 500 → 500 READ_SCOPE_COMPILE_FAILED, policy content withheld', async () => {
  const message = '[read-scope-sql] unsupported operator "$regex" on "owner_email" (fail-closed).';
  const err = Object.assign(new Error(message), { code: 'READ_SCOPE_COMPILE_FAILED', status: 500 });
  const route = buildRoute(async () => ({ queryDataset: vi.fn().mockRejectedValue(err) }));
  const res = await post(route, { dataset, selection });
  expect(res.statusCode).toBe(500);
- expect(res.body.code).toBe('ANALYTICS_QUERY_FAILED');
+ // [#11718] The declared code is RELAYED now, not overwritten — see this
+ // file's sibling `analytics-read-scope-refusal-envelope.test.ts` header.
+ // The status this test is really about is untouched: still 500, still not
+ // the sniffed 400 the deleted message list used to produce.
+ expect(res.body.code).toBe('READ_SCOPE_COMPILE_FAILED');
+ expect(res.body.code).not.toBe('DATASET_INVALID');
  // The disclosure half: an RLS policy's field name must not come back.
  expect(String(res.body.error)).not.toMatch(/owner_email/);
  expect(String(res.body.error)).not.toMatch(/read-scope-sql/);
@@ -380,19 +385,56 @@ describe('[#5352] reading the envelope did not turn every failure into a 400', (
  expect(res.body.code).toBe('ANALYTICS_QUERY_FAILED');
  });
 
- it('a 5xx-status error is NOT passed through — an internal fault keeps the 500 envelope, message withheld', async () => {
- // Deliberate asymmetry: the passthrough is 4xx-only, so a producer cannot
- // re-label a server fault with a code of its own and slip past the
- // `logError` line that makes it visible to operators.
+ it('[#11718 — INVERTED] a declared 5xx IS relayed, and is still logged and still withheld', async () => {
+ // ── This pin asserted the collapse. It is inverted, not deleted ─────────
+ //
+ // What it asserted, verbatim: "a 5xx-status error is NOT passed through —
+ // an internal fault keeps the 500 envelope, message withheld", reasoned as
+ // "the passthrough is 4xx-only, so a producer cannot re-label a server
+ // fault with a code of its own and slip past the `logError` line that makes
+ // it visible to operators."
+ //
+ // That REASON is answered rather than overruled, and answering it is what
+ // made the repair safe: `logError` runs BEFORE the relay branch and is
+ // unconditional, so every declared 5xx is still on the operator's line with
+ // its full original text. Asserted here, not assumed — the argument for the
+ // old behaviour is only retired if its concern is actually covered.
+ //
+ // What was NOT answerable was the collapse itself. `/data` relays a declared
+ // 5xx's status and code (#5582: `502`/`503` are `isExpectedDataStatus`
+ // lifecycle outcomes proxies and retry policies read differently from a
+ // `500`), and so does the sibling `/analytics/query`. Measured door-to-door
+ // in `rest-hook-refusal-message-parity.test.ts` §8f.
  const err = Object.assign(new Error('upstream analytics warehouse is unavailable'), {
  code: 'WAREHOUSE_UNAVAILABLE',
  status: 503,
  });
  const route = buildRoute(async () => ({ queryDataset: vi.fn().mockRejectedValue(err) }));
- const res = await post(route, { dataset, selection });
-
- expect(res.statusCode).toBe(500);
- expect(res.body.code).toBe('ANALYTICS_QUERY_FAILED');
+ const logSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+ let res: any;
+ let logged: string;
+ try {
+ res = await post(route, { dataset, selection });
+ // Read the calls BEFORE restoring: `mockRestore` resets the recorded
+ // calls as well as the implementation, so reading after it reports an
+ // empty log for a route that logged perfectly well.
+ logged = logSpy.mock.calls.map((args) => args.map(String).join(' ')).join('\n');
+ } finally {
+ logSpy.mockRestore();
+ }
+
+ expect(res.statusCode).toBe(503);
+ expect(res.statusCode).not.toBe(500);
+ // [#9232] `WAREHOUSE_UNAVAILABLE` is not an ADR-0112 member, so it is
+ // DEMOTED to `declaredCode` beside the code the status derives — the same
+ // answer `/data` gives it, which is the whole point of importing that arm
+ // instead of hand-building a second envelope here.
+ expect(res.body.code).toBe('SERVICE_UNAVAILABLE');
+ expect(res.body.declaredCode).toBe('WAREHOUSE_UNAVAILABLE');
+ expect(res.body.code).not.toBe('ANALYTICS_QUERY_FAILED');
+ // The operator still has the whole thing — the concern the old pin named.
+ expect(logged).toContain('Analytics dataset query error');
+ expect(logged).toContain('upstream analytics warehouse is unavailable');
  // [#5367] Second half of the asymmetry, added with the read-scope ruling: a
  // producer that DECLARES a server fault has declared that the detail is the
  // operator's, so the message is withheld here and kept in `logError`. This

packages/rest/src/analytics-read-scope-refusal-envelope.test.ts

@@ -2,8 +2,24 @@
 
 /**
  * [#5367, maintainer ruling 2026-08-06] A read-scope lowering failure reaches the
- * caller as `500 ANALYTICS_QUERY_FAILED` **with the RLS policy withheld**, and the
- * full text reaches the operator's log.
+ * caller as a **500 with the RLS policy withheld**, and the full text reaches the
+ * operator's log.
+ *
+ * [#11718] The CODE in that sentence was `ANALYTICS_QUERY_FAILED` and is now
+ * `READ_SCOPE_COMPILE_FAILED` — the one these refusals declare. Nothing in the
+ * 2026-08-06 ruling moved: the family is still a SERVER fault, still `500`, still
+ * has its policy content withheld, and the `400 DATASET_INVALID` it was rescued
+ * from is still asserted against below. What changed is that
+ * `/analytics/dataset/query` stopped OVERWRITING the producer's declaration with
+ * a code of its own. That overwrite was never ruled — it was what ③'s hand-built
+ * envelope happened to emit — and it made this face the only one of three that
+ * did it: `/data` relays a declared 5xx's status and code (#5582), and the
+ * SIBLING analytics face `/analytics/query` has shipped
+ * `READ_SCOPE_COMPILE_FAILED` to clients all along
+ * (`analytics-query-read-scope-withhold.test.ts`, measured against a real
+ * `AnalyticsService`). So this file's answer now equals its sibling's for one
+ * refusal, where before the same fault was named two different things depending
+ * on which analytics door the caller used.
  *
  * ## What this closes
  *
@@ -178,13 +194,19 @@ describe('[#5367] POST /analytics/dataset/query — a read-scope failure is a 50
  ];
 
  for (const c of CASES) {
- it(`${c.name} → 500 ANALYTICS_QUERY_FAILED, body carries no policy detail`, async () => {
+ it(`${c.name} → 500 READ_SCOPE_COMPILE_FAILED, body carries no policy detail`, async () => {
  const route = buildRoute(async () => analyticsWithScope(c.scope));
  const res = await post(route, { dataset, selection });
 
  // Classification: a server fault, not the caller's mistake.
  expect(res.statusCode).toBe(500);
- expect(res.body.code).toBe('ANALYTICS_QUERY_FAILED');
+ // [#11718] …named by its PRODUCER. `read-scope-sql` declares
+ // `READ_SCOPE_COMPILE_FAILED` / 500 and the route relays it instead of
+ // overwriting it with `ANALYTICS_QUERY_FAILED`. The status is untouched
+ // by that repair — this family declares 500 — so the 2026-08-06 ruling's
+ // own subject is asserted unchanged, immediately above.
+ expect(res.body.code).toBe('READ_SCOPE_COMPILE_FAILED');
+ expect(res.body.code).not.toBe('ANALYTICS_QUERY_FAILED');
  // …and specifically NOT the pre-ruling answer.
  expect(res.statusCode).not.toBe(400);
  expect(res.body.code).not.toBe('DATASET_INVALID');

packages/rest/src/error-response.ts

@@ -418,6 +418,58 @@ function thrownCodeFields(error: any, status: number): { code?: string; declared
  return { code: thrown.code, ...(demoted !== undefined ? { declaredCode: demoted } : {}) };
 }
 
+/**
+ * [#11718] The DECLARED-SERVER-FAULT relay, as one definition instead of a
+ * shape each door re-derives: a producer that declared a 5xx keeps its
+ * `status` and its ADR-0112 `code`, and loses only its prose.
+ *
+ * Answers `undefined` for everything else — no declared status, a declared
+ * 4xx, a status outside 400–599 — so a caller can ask it first and keep its
+ * own arm for the rest.
+ *
+ * ## Why this is a function and not a fourth copy
+ *
+ * #11718 measured one producer-declared `{ status: 503, code:
+ * 'SERVICE_UNAVAILABLE' }` through two doors and got two answers:
+ *
+ * ```
+ * POST /api/v1/data/:object → 503 {"error":"Internal server error","code":"SERVICE_UNAVAILABLE"}
+ * POST /api/v1/analytics/dataset/query → 500 {"code":"ANALYTICS_QUERY_FAILED","error":"Internal server error"}
+ * ```
+ *
+ * The analytics dataset face built its 5xx body by hand, so #5582's relay —
+ * argued on the ground that `502`/`503` are `isExpectedDataStatus` lifecycle
+ * outcomes that proxies and retry policies read differently from a `500` —
+ * simply never reached it. It is the same failure mode {@link
+ * classifiedRefusalAnswer} was extracted for one arm earlier: a third local
+ * opinion at this boundary is precisely how two faces come to disagree. So the
+ * arm is imported, not restated.
+ *
+ * ⚠️ Its gate is `declaredHttpStatus(...) >= 500`, NOT `declaresServerFault`
+ * alone. The two differ on a 5xx that declared no `code`, and the difference is
+ * load-bearing: a producer declaring `{ status: 503 }` and nothing else still
+ * gets its status relayed, carrying no code at all. Nothing is invented for the
+ * half that was not declared — see the paragraph in {@link mapDataError}'s arm
+ * this body was lifted from. Gating on `declaresServerFault` instead would
+ * silently keep collapsing that shape onto `500`, which is the very defect,
+ * one case narrower.
+ */
+export function declaredServerFaultAnswer(
+ error: any,
+): { status: number; body: Record<string, unknown> } | undefined {
+ const declaredStatus = declaredHttpStatus(error);
+ if (declaredStatus === undefined || declaredStatus < 500) return undefined;
+ return {
+ status: declaredStatus,
+ body: {
+ error: INTERNAL_ERROR_MESSAGE,
+ ...(declaresServerFault({ status: declaredStatus, code: error?.code })
+ ? thrownCodeFields(error, declaredStatus)
+ : {}),
+ },
+ };
+}
+
 /**
  * [#8264] Postgres' missing-relation template, anchored on the QUOTED
  * identifier the driver always emits — never on the bare "does not exist"
@@ -896,16 +948,16 @@ function classifyDataError(error: any, object?: string): { status: number; body:
  // still decides WHETHER a code rides — its non-empty-string half is the
  // same question `thrownCodeFields` asks internally, so the two agree by
  // construction and this arm's body-shape decision is unchanged.
-if (declaredStatus >= 500) {
- return {
- status: declaredStatus,
- body: {
- error: INTERNAL_ERROR_MESSAGE,
- ...(declaresServerFault({ status: declaredStatus, code: error?.code })
- ? thrownCodeFields(error, declaredStatus)
- : {}),
- },
-};
+//
+// [#11718] The arm's BODY now lives in {@link declaredServerFaultAnswer},
+// unchanged — every paragraph above still describes it, and this call is
+// the only reader of it that existed before. It was lifted out so the
+// `/analytics/dataset/query` face could answer a declared 5xx with the
+// same bytes rather than a second hand-built envelope; `/data`'s answer
+// is the reference and does not move.
+const declaredServerFault = declaredServerFaultAnswer(error);
+if (declaredServerFault !== undefined) {
+return declaredServerFault;
  }
  // [#5423] The 4xx arm is UNCHANGED by #5582: a 4xx message is addressed
  // TO the caller and is the remedy, so it keeps its wording, its