feat(commerce-server): standardize route error handling

.changeset/clear-order-status-errors.md

@@ -0,0 +1,5 @@
+---
+'@godaddy/gd-commerce-server': minor
+---
+
+Return 400 from the order-status route for invalid order IDs, including IDs the Orders API rejects as malformed, and 404 when the Orders API reports the order missing or it belongs to another store or channel, instead of 500. A 404 without the Orders API's `NOT_FOUND` code, such as from a misconfigured base URL, stays a logged 500. The 500 response no longer includes the internal error message; it is logged server-side instead. Export `InvalidOrderIdError`, `OrderNotFoundError`, and `ORDER_STATUS_UNKNOWN` for in-process `getOrderStatus()` callers; check `error.name` rather than `instanceof` so the check holds when a host has more than one copy of this package.

.changeset/quiet-route-failures.md

@@ -0,0 +1,16 @@
+---
+'@godaddy/gd-commerce-server': minor
+---
+
+Standardize route error handling. Every failure now responds with `{ error, code, requestId }` and no longer includes internal error text in a `message` field.
+
+Status codes now reflect the cause:
+
+- 502 for Commerce failures, including rejected OAuth credentials (`upstream_unauthorized`); previously 500. This includes order-status upstream failures, which were 500. An OAuth 400 is `upstream_unauthorized` only for `invalid_client`, `invalid_grant`, `unauthorized_client`, or `invalid_scope`.
+- 503 for missing or unreadable configuration on every route; previously 500 everywhere except `/config`.
+- 404 for writes to an existing cart (`/cart/:id/...`) that is missing, expired, or completed; previously 500. Creating a cart and checkout are not classified this way and return 502.
+- 500 only for unexpected errors.
+
+Hosts can pass `logger` and `getRequestId` to the router factories to receive failure detail and reuse their own request ids; a throwing `getRequestId` falls back to a generated id. In-process helpers, including `getOrderStatus()` and `createCheckoutSession()`, throw exported `CommerceError` subclasses, also when a host configuration throws while being read. Check `error.code` rather than `instanceof` or `error.name`; it holds when a host has more than one copy of this package. `validateCommerceCartScope` was internal and is replaced by a throwing `assertCommerceCartScope`.
+
+Hosts that read `message` or check for status 500 should switch to `code` and server-side logs.

packages/commerce-server/README.md

@@ -56,4 +56,31 @@ Configure `checkoutReturnUrls` on the router using trusted deployment settings.
 
 Without this policy, HTTP checkout returns 503 before creating a session. Invalid request destinations return 400. This applies to both router presets that expose checkout. Trusted in-process callers of `createCheckoutSession()` own their return URLs and must construct or validate them server-side.
 
-A return from hosted checkout is not proof of payment. The order-status route uses the authorized Orders REST API, which supports completed orders, and returns its payment status (for example `PAID` or `PENDING`; `unknown` if absent). The server OAuth client must be granted `commerce.order:read`. The helper verifies the returned order ID, store, and channel and returns a limited summary without customer contact data. Hosts must authenticate callers and authorize access to each requested order before exposing this route.
+A return from hosted checkout is not proof of payment. The order-status route uses the authorized Orders REST API, which supports completed orders, and returns its payment status (for example `PAID` or `PENDING`; the exported `ORDER_STATUS_UNKNOWN`, `'unknown'`, if absent). Show it only as display enrichment and never present an unconfirmed payment as paid. The server OAuth client must be granted `commerce.order:read`. The helper verifies the returned order ID, store, and channel and returns a limited summary without customer contact data. The route returns 400 for a missing, blank, padded, `.`, or `..` order ID (padded IDs are rejected, not trimmed) or one the Orders API rejects as malformed (422 `VALIDATION_FAILED`), and 404 when the Orders API reports the order missing (404 `NOT_FOUND`) or the order belongs to another store or channel. A 404 without that code, such as from a misconfigured API base URL, is an upstream failure. Token and other Commerce failures, including a response for a different order ID, return 502 and unreadable configuration returns 503 (see [Errors](#errors)). Hosts must authenticate callers and authorize access to each requested order before exposing this route.
+
+## Errors
+
+Every route reports failures the same way. The body is `{ "error": "<customer-facing message>", "code": "<code>", "requestId": "<id>" }` (order-status also includes `success: false`). The same id is passed to the logger. Bodies never contain upstream messages, configuration details, or credentials.
+
+| Status | `code` | Meaning |
+| --- | --- | --- |
+| 400 | `invalid_request` | Missing or invalid input. |
+| 404 | `not_found` | Missing order or product, or a write to an existing cart (`/cart/:id/...`) that is missing, expired, or completed. Cart reads keep returning `200 { "cart": null }`. Checkout with a stale or completed `draftOrderId` is not yet distinguished and returns 502. |
+| 409 | `scope_mismatch` | `X-Commerce-Scope` no longer matches the configured store binding. |
+| 502 | `upstream_unauthorized` | Commerce rejected this server's OAuth client or requested scope (HTTP 401/403, or an OAuth 400 with `invalid_client`, `invalid_grant`, `unauthorized_client`, or `invalid_scope`). It concerns server credentials, not the caller, so it is never reported as 401/403. |
+| 502 | `upstream_error` | Commerce failed, returned an unexpected response, or could not be reached. |
+| 503 | `not_configured` | Configuration is missing or unreadable, or checkout return URLs are not configured. |
+| 500 | `internal_error` | An unexpected error in this package. |
+
+Upstream errors without a specific mapping stay 502; business errors such as invalid discount codes are not yet distinguished. 5xx detail, including upstream status and GraphQL error codes, goes to the logger:
+
+```ts
+createCommerceRouter({
+  configuration,
+  logger: { error: (message, context) => log.error(context, message) },
+  // Use the host's request id when its edge sets one; unsafe or missing ids fall back to a UUID.
+  getRequestId: (req) => req.get('x-request-id'),
+});
+```
+
+`createCommerceCatalogRouter` and `createGoDaddyPaymentsRouter` accept the same `{ logger, getRequestId }` as their last argument. The logger defaults to `console.error`. In-process helpers throw the exported `CommerceError` subclasses (`InvalidRequestError`, `NotFoundError`, `CommerceNotConfiguredError`, `UpstreamError`), including when a host configuration throws while being read. Check `error.code` (for example `'not_found'`) rather than `instanceof`, which fails when a host has more than one copy of this package.

packages/commerce-server/src/configuration-integration.test.ts

@@ -7,6 +7,7 @@ import { createCommerceRouter } from './router';
 const clientFetch = globalThis.fetch;
 afterEach((): void => {
   vi.unstubAllGlobals();
+  vi.restoreAllMocks();
 });
 
 it('serves variant product details without querying SKUGroup.status', async (): Promise<void> => {
@@ -95,7 +96,11 @@ it('serves variant product details without querying SKUGroup.status', async ():
     ]);
     const archived = await clientFetch(`${url.replace('/shirt', '/archived-shirt')}?attributeValues=blue`);
     expect(archived.status).toBe(404);
-    expect(await archived.json()).toEqual({ error: 'Product not found' });
+    expect(await archived.json()).toEqual({
+      error: 'Product not found',
+      code: 'not_found',
+      requestId: expect.any(String),
+    });
     expect(upstream).toHaveBeenCalledTimes(3);
   } finally {
     await new Promise<void>((resolve, reject) =>
@@ -237,15 +242,12 @@ it.each([undefined, 'https://api.example.com', 'https://api.example.com:8443'])(
 
 it.each([
   ['Order not found', 200, { cart: null }],
-  [
-    'Authentication token expired',
-    500,
-    { error: 'Failed to load cart', message: 'Authentication token expired' },
-  ],
-  ['Database unavailable', 500, { error: 'Failed to load cart', message: 'Database unavailable' }],
+  ['Authentication token expired', 502, { error: 'Failed to load cart', code: 'upstream_error' }],
+  ['Database unavailable', 502, { error: 'Failed to load cart', code: 'upstream_error' }],
 ] as const)(
   'handles the actual Apollo error envelope for %s',
   async (message, status, body): Promise<void> => {
+    const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {});
     vi.stubGlobal(
       'fetch',
       vi.fn(
@@ -278,7 +280,67 @@ it.each([
       if (!address || typeof address === 'string') throw new Error('Expected a listening TCP server');
       const response = await clientFetch(`http://127.0.0.1:${address.port}/api/commerce/cart/completed-cart`);
       expect(response.status).toBe(status);
-      expect(await response.json()).toEqual(body);
+      const json = (await response.json()) as { requestId?: string };
+      const requestId = json.requestId;
+      expect(json).toEqual(status === 200 ? body : { ...body, requestId: expect.any(String) });
+      if (status === 502) {
+        expect(consoleError).toHaveBeenCalledWith(
+          'commerce-server: Failed to load cart',
+          expect.objectContaining({
+            requestId,
+            error: expect.objectContaining({ message: expect.stringContaining(message) }),
+          }),
+        );
+      }
+    } finally {
+      await new Promise<void>((resolve, reject) =>
+        server.close((error) => (error ? reject(error) : resolve())),
+      );
+    }
+  },
+);
+
+it.each([
+  ['extensions.http.status', { code: 'UNAUTHENTICATED', http: { status: 401 } }],
+  ['extensions.status', { code: 'FORBIDDEN', status: 403 }],
+])(
+  'reports an auth failure in an HTTP 200 GraphQL body (%s) as upstream_unauthorized',
+  async (_case, extensions): Promise<void> => {
+    vi.spyOn(console, 'error').mockImplementation(() => {});
+    vi.stubGlobal(
+      'fetch',
+      vi.fn(
+        async (): Promise<Response> =>
+          Response.json({ data: { orderById: null }, errors: [{ message: 'Unauthorized', extensions }] }),
+      ),
+    );
+    const app = express();
+    app.use(
+      '/api/commerce',
+      createCommerceRouter({
+        configuration: createRuntimeCommerceConfiguration({
+          environment: {
+            GODADDY_OAUTH_CLIENT_ID: 'client-1',
+            GODADDY_OAUTH_CLIENT_SECRET: 'secret-1',
+            GODADDY_STORE_ID: 'store-1',
+            GODADDY_CHANNEL_ID: 'channel-1',
+            GODADDY_CURRENCY_CODE: 'USD',
+          },
+        }),
+      }),
+    );
+    const server = app.listen(0, '127.0.0.1');
+    await once(server, 'listening');
+    try {
+      const address = server.address();
+      if (!address || typeof address === 'string') throw new Error('Expected a listening TCP server');
+      const response = await clientFetch(`http://127.0.0.1:${address.port}/api/commerce/cart/cart-1`);
+      expect(response.status).toBe(502);
+      expect(await response.json()).toEqual({
+        error: 'Failed to load cart',
+        code: 'upstream_unauthorized',
+        requestId: expect.any(String),
+      });
     } finally {
       await new Promise<void>((resolve, reject) =>
         server.close((error) => (error ? reject(error) : resolve())),

packages/commerce-server/src/create-checkout-session.test.ts

@@ -255,13 +255,13 @@ describe('createCheckoutSession', () => {
     ]);
   });
 
-  it('returns host configuration errors immediately without OAuth or checkout requests', async (): Promise<void> => {
+  it('reports host configuration errors as not configured without OAuth or checkout requests', async (): Promise<void> => {
+    const cause = new Error('Host configuration unavailable');
     vi.mocked(configuration.read).mockImplementation(() => {
-      throw new Error('Host configuration unavailable');
+      throw cause;
     });
-    await expect(createCheckoutSession(cart, configuration)).rejects.toThrow(
-      'Host configuration unavailable',
-    );
+    const error = await createCheckoutSession(cart, configuration).catch((thrown: unknown) => thrown);
+    expect(error).toMatchObject({ code: 'not_configured', httpStatus: 503, cause });
     expect(configuration.read).toHaveBeenCalledTimes(1);
     expect(mockGetOAuthAccessToken).not.toHaveBeenCalled();
     expect(mockGqlRequest).not.toHaveBeenCalled();