From 897500eb980c83c799025df5a45581a7f7591936 Mon Sep 17 00:00:00 2001 From: Roberto Jimeno Date: Mon, 27 Jul 2026 19:34:00 -0400 Subject: [PATCH] feat(console): [SAS-172] surface structured backend errors instead of a generic fallback. --- console/src/api/cloudGlobalApi.ts | 4 +- .../src/api/mocks/cloudGlobalApiHandlers.ts | 16 ++++++ console/src/api/openApiUtils.ts | 53 +++++++++++++++++-- console/src/api/schemas/global-api.ts | 39 +++++++++++++- .../billing/AccountSpendBreakdown.tsx | 20 ++++++- .../src/platform/billing/UsagePage.test.tsx | 49 +++++++++++++++++ 6 files changed, 171 insertions(+), 10 deletions(-) diff --git a/console/src/api/cloudGlobalApi.ts b/console/src/api/cloudGlobalApi.ts index cfa5f2162b6b0..b9de390ef3f41 100644 --- a/console/src/api/cloudGlobalApi.ts +++ b/console/src/api/cloudGlobalApi.ts @@ -154,7 +154,7 @@ export async function getCostsBreakdownDaily( requestOptions: OpenApiRequestOptions = {}, ) { const { headers, ...options } = requestOptions; - const { data, response } = await getClient().GET( + const { data, error, response } = await getClient().GET( "/api/costs/breakdown/daily", { params: { @@ -173,7 +173,7 @@ export async function getCostsBreakdownDaily( ...options, }, ); - return handleOpenApiResponseWithBody(data, response); + return handleOpenApiResponseWithBody(data, response, error); } export async function createStripeSetupIntent( diff --git a/console/src/api/mocks/cloudGlobalApiHandlers.ts b/console/src/api/mocks/cloudGlobalApiHandlers.ts index c7af9ad27d275..22ed98459fc00 100644 --- a/console/src/api/mocks/cloudGlobalApiHandlers.ts +++ b/console/src/api/mocks/cloudGlobalApiHandlers.ts @@ -17,6 +17,7 @@ import { Region, Regions, } from "~/api/cloudGlobalApi"; +import { ApiError } from "~/api/openApiUtils"; export const buildCloudRegionsReponse = ( options: { @@ -52,6 +53,21 @@ export const buildDailyCostBreakdownResponse = ( return HttpResponse.json(payload, { status: options.status ?? 200 }); }); +/** A structured `ApiError` error response (SAS-172), e.g. for asserting the + * console renders the real message/request id instead of a generic fallback. */ +export const buildDailyCostBreakdownErrorResponse = ( + options: { apiError?: Partial; status?: number } = {}, +) => + http.get("*/api/costs/breakdown/daily", () => { + const payload: ApiError = { + reason: "internal_error", + message: "There was an error servicing this request.", + requestId: "11111111-1111-1111-1111-111111111111", + ...options.apiError, + }; + return HttpResponse.json(payload, { status: options.status ?? 500 }); + }); + export const buildInvoicesResponse = ( options: { invoices?: Invoice[]; status?: number } = {}, ) => diff --git a/console/src/api/openApiUtils.ts b/console/src/api/openApiUtils.ts index 967d344479956..e667f7858d99c 100644 --- a/console/src/api/openApiUtils.ts +++ b/console/src/api/openApiUtils.ts @@ -8,28 +8,71 @@ // by the Apache License, Version 2.0. import { OpenApiFetchError } from "./OpenApiFetchError"; +import { components } from "./schemas/global-api"; -export async function handleOpenApiResponse( +export type ApiError = components["schemas"]["ApiError"]; + +/** + * Extracts the structured `ApiError` body from a caught error, when the + * failing endpoint has been migrated to return one (SAS-172). Endpoints + * that haven't yet return a plain-text or otherwise-shaped body, so callers + * must still have a generic fallback for when this returns `null`. + */ +export function getApiError(error: unknown): ApiError | null { + if ( + error instanceof OpenApiFetchError && + typeof error.body === "object" && + error.body !== null && + "reason" in error.body && + "message" in error.body && + "requestId" in error.body + ) { + return error.body as ApiError; + } + return null; +} + +// `openapi-fetch` splits a response into `data` (success schema) or `error` +// (error-status schema), never both: `data` is always `undefined` on a +// non-2xx response, `error` is always `undefined` on a 2xx one. Callers that +// only ever passed `data` here could never surface a JSON error body, no +// matter what the backend sent, which is the deeper cause behind SAS-149's +// "Empty response" fallback masking real backend messages. `error` is +// optional so existing call sites that don't pass it keep working (they fall +// back to the generic "Empty response" string exactly as before); pass it +// through for endpoints migrated to a structured error body (SAS-172). +export async function handleOpenApiResponse( data: T | undefined, response: Response, + error?: E, ) { if (!response.ok) { - throw new OpenApiFetchError(response.status, data ?? "Empty response"); + throw new OpenApiFetchError( + response.status, + error ?? data ?? "Empty response", + ); } return { ...response, data, }; } -export async function handleOpenApiResponseWithBody( +export async function handleOpenApiResponseWithBody( data: T | undefined, response: Response, + error?: E, ) { if (!response.ok) { - throw new OpenApiFetchError(response.status, data ?? "Empty response"); + throw new OpenApiFetchError( + response.status, + error ?? data ?? "Empty response", + ); } if (!data) { - throw new OpenApiFetchError(response.status, data ?? "Empty response"); + throw new OpenApiFetchError( + response.status, + error ?? data ?? "Empty response", + ); } return { ...response, diff --git a/console/src/api/schemas/global-api.ts b/console/src/api/schemas/global-api.ts index 02d491a9ed89f..764cf4aa5ea64 100644 --- a/console/src/api/schemas/global-api.ts +++ b/console/src/api/schemas/global-api.ts @@ -275,6 +275,29 @@ export interface components { /** @description Storage resource costs. */ storage: components["schemas"]["Cost_StoragePrice"]; }; + /** + * @description A structured JSON error response. + * + * `reason` is a closed, safe-by-construction category (see [`ErrorReason`]). + * `message` is curated text safe to show a customer: for `Validation`, it + * may describe the problem specifically, since it only concerns the + * caller's own request; for every other reason it is a fixed, generic + * string, chosen by the constructor, never raw upstream or internal error + * text. `request_id` correlates this response to the full, unredacted + * error in server-side logs. + * + * Construct via the `ApiError::validation`/`forbidden`/`not_found`/ + * `upstream_error`/`upstream_limit_exceeded`/`internal` functions, not by + * hand, so every call site logs the real cause exactly once (except + * `validation`, which has no separate "real cause": the message *is* the + * cause) and gets a request id for free. + */ + ApiError: { + reason: components["schemas"]["ErrorReason"]; + message: string; + /** Format: uuid */ + requestId: string; + }; Card: { /** @description The last 4 digits of the card number. */ last4: string; @@ -494,6 +517,14 @@ export interface components { DetachPaymentMethodRequest: { paymentMethodId: string; }; + /** + * @description A small, closed set of safe-to-expose error categories. Never add a + * variant, or a call site, that could put upstream (Orb, Stripe, ...) error + * text, internal service/host names, or infra topology into a response — + * see `ApiError`. + * @enum {string} + */ + ErrorReason: "validation" | "forbidden" | "not_found" | "upstream_error" | "upstream_limit_exceeded" | "internal_error"; GetSelfManagedSubscriptionResponse: { /** Format: date-time */ endDate: string; @@ -823,14 +854,18 @@ export interface operations { headers: { [name: string]: unknown; }; - content?: never; + content: { + "application/json": components["schemas"]["ApiError"]; + }; }; /** @description Insufficient permissions */ 403: { headers: { [name: string]: unknown; }; - content?: never; + content: { + "application/json": components["schemas"]["ApiError"]; + }; }; }; }; diff --git a/console/src/platform/billing/AccountSpendBreakdown.tsx b/console/src/platform/billing/AccountSpendBreakdown.tsx index a9a6cbc3b2b5a..34ea0aaf7b26d 100644 --- a/console/src/platform/billing/AccountSpendBreakdown.tsx +++ b/console/src/platform/billing/AccountSpendBreakdown.tsx @@ -27,6 +27,7 @@ import { CostBreakdownCluster, CostBreakdownDay, } from "~/api/cloudGlobalApi"; +import { getApiError } from "~/api/openApiUtils"; import ErrorBox from "~/components/ErrorBox"; import { GraphEventOverlay, GraphTooltip } from "~/components/graphComponents"; import { LoadingContainer } from "~/components/LoadingContainer"; @@ -625,6 +626,7 @@ const AccountSpendBreakdown = ({ days, isLoading, isError, + error, regionFilter, setRegionFilter, timeRange, @@ -634,6 +636,7 @@ const AccountSpendBreakdown = ({ days, regionFilter, ); + const apiError = getApiError(error); return ( @@ -650,7 +653,22 @@ const AccountSpendBreakdown = ({ // data lands. ) : isError ? ( - + + {/* A validation error describes the caller's own request (e.g. an + out-of-range date), so there's nothing for support to look up; + every other reason gets a reference for support correlation. */} + {apiError && apiError.reason !== "validation" && ( + + Reference: {apiError.requestId} + + )} + ) : !days || accountIds.length === 0 ? ( ) : ( diff --git a/console/src/platform/billing/UsagePage.test.tsx b/console/src/platform/billing/UsagePage.test.tsx index 642825f009ed8..0069cb541ed99 100644 --- a/console/src/platform/billing/UsagePage.test.tsx +++ b/console/src/platform/billing/UsagePage.test.tsx @@ -17,6 +17,7 @@ import { buildCloudOrganizationsResponse, buildCloudRegionsReponse, buildCreditsResponse, + buildDailyCostBreakdownErrorResponse, buildDailyCostBreakdownResponse, buildInvoicesResponse, } from "~/api/mocks/cloudGlobalApiHandlers"; @@ -250,6 +251,54 @@ describe("UsagePage", () => { ).toBeVisible(); }); + it("shows the backend's real message and a request id for a structured API error (SAS-172)", async () => { + server.use( + buildDailyCostBreakdownErrorResponse({ + apiError: { + reason: "upstream_limit_exceeded", + message: + "This account has more billing configuration than we can currently summarize. Contact support with the reference below.", + requestId: "22222222-2222-2222-2222-222222222222", + }, + }), + ); + renderComponent(); + expect( + await screen.findByText( + "This account has more billing configuration than we can currently summarize. Contact support with the reference below.", + ), + ).toBeVisible(); + expect(await screen.findByTestId("api-error-request-id")).toHaveTextContent( + "22222222-2222-2222-2222-222222222222", + ); + // The generic fallback must not also render alongside the real message. + expect( + screen.queryByText("An error occurred loading your usage"), + ).not.toBeInTheDocument(); + }); + + it("does not show a request id for a validation error (the caller's own request, not something support needs to look up)", async () => { + server.use( + buildDailyCostBreakdownErrorResponse({ + status: 400, + apiError: { + reason: "validation", + message: "startDate cannot be more than 100 days in the past", + requestId: "33333333-3333-3333-3333-333333333333", + }, + }), + ); + renderComponent(); + expect( + await screen.findByText( + "startDate cannot be more than 100 days in the past", + ), + ).toBeVisible(); + expect( + screen.queryByTestId("api-error-request-id"), + ).not.toBeInTheDocument(); + }); + it("shows an empty state when the window has no usage", async () => { // beforeEach's default handler already returns { days: [] }. renderComponent();