feat(commerce-server): standardize route error handling

[anam-godaddy]

Important

Depends on #1488 — merge that first. This branch contains #1488's commits, so the diff against main includes them until #1488 merges. To review only this PR's changes, look at its non-merge commits (8aecdf2 onward); merge commits 6d4846e and 897cf6d bring in #1488. After #1488 merges, this branch will be updated from main.

What changed

Error handling in @godaddy/gd-commerce-server is now one standard process instead of a try/catch per route.

1. Typed errors (lib/commerce/errors.ts): CommerceError(httpStatus, code, message, { publicMessage, cause, details }), with subclasses InvalidRequestError, ScopeMismatchError, NotFoundError, CommerceNotConfiguredError, and UpstreamError. message is internal; only publicMessage (or the route's label) reaches the browser.
2. Translation where Commerce is called: gqlRequest, the OAuth token request, and the Orders REST lookup turn network failures, non-2xx responses, non-JSON bodies, and GraphQL errors into UpstreamError, keeping the upstream status and GraphQL codes in details for the log. GraphQLErrorWithCodes now extends UpstreamError and reports upstream_unauthorized when the HTTP status or any error's embedded status is 401/403. config.ts throws CommerceNotConfiguredError, and host-supplied configurations that throw plain errors are also reported as not configured, both through the routes and when getOrderStatus() / createCheckoutSession() are called in-process.
3. Mapping table (lib/commerce/upstream-errors.ts): rules that turn a specific upstream signal into a more precise error. It holds one rule, the existing cart-not-found classifier, moved out of cart/[id]/GET.ts. Routes opt in through commerceRoute's classifyUpstreamError option; only the /cart/:id routes use the cart rule, so creating a cart and the catalog routes never report "Cart not found". New rules need evidence and a test.
4. One responder (commerceRoute(label, handler) / sendCommerceError): every route's catch-all is replaced. It picks the status, writes { error, code, requestId } (order-status keeps success: false), and logs 5xx detail with the same id. 4xx responses are not logged.
5. Observability options: logger (defaults to console.error) and getRequestId(req) on createCommerceRouter, and as the last argument of createCommerceCatalogRouter / createGoDaddyPaymentsRouter. The id is resolved only when a failure body needs it. A host id is used only if it matches [A-Za-z0-9._:-]{1,128}; otherwise, or if getRequestId throws, a UUID is generated. No response header is set yet; its name should be chosen once it is known which request-id header the GoDaddy edge and Commerce APIs use.

Status codes

Status	code	When	Before
400	invalid_request	Invalid input	400 (same)
404	not_found	Missing order (Orders API NOT_FOUND) or product; write to an existing cart (/cart/:id/...) that is missing, expired, or completed	500 for cart writes
409	scope_mismatch	Stale X-Commerce-Scope	409 (same)
502	upstream_unauthorized	Commerce rejected the server's OAuth client or scope (401/403, or an OAuth 400 invalid_client / invalid_grant / unauthorized_client / invalid_scope)	500
502	upstream_error	Commerce failed, returned an unexpected response, or was unreachable	500
503	not_configured	Missing or unreadable configuration, or checkout return URLs not configured	503 on /config and for checkout's missing return URLs; 500 elsewhere
500	internal_error	Unexpected error in this package	500

GET /cart/:id keeps returning 200 { cart: null } for a missing cart, as the storefront contract documents.

Why token failures are 502, not 401/403

The token is the server's own client_credentials grant (getOAuthAccessToken); browsers send no token. A 401/403 would blame the caller and can trigger host logic for an expired session (sign-out, login redirect). code: upstream_unauthorized, together with the logged upstream status and requested scope, identifies the problem without that side effect. OAuth 400 responses are classified the same way only when their RFC 6749 §5.2 error is invalid_client, invalid_grant, unauthorized_client, or invalid_scope; other 400s (e.g. invalid_request) are upstream_error, so a malformed request doesn't send operators rotating valid credentials.

Limits

• Upstream errors without a specific mapping stay 502. Shopper errors such as an invalid SKU or discount code are not yet distinguished, because the codes Commerce sends for them are not documented here. The logged upstreamCodes and upstreamStatus are the evidence for future rules.
• The checkout subgraph is not run through the cart-not-found rule, which is evidenced only for the order storefront. Checkout with a stale or completed draftOrderId therefore returns 502, not 404.
• Request ids are not yet sent to or read from Commerce. Which headers or body fields the Commerce APIs return or honour is unverified; a follow-up should capture real error responses before adding that.

Relationship to #1488

This branch merges #1488's branch so it can cover order-status; the diff includes #1488's commits until #1488 merges. All of #1488's review fixes are merged (897cf6d), expressed with this branch's error types:

• Only an Orders API 404 with code: NOT_FOUND is OrderNotFoundError (404); a 422 VALIDATION_FAILED is InvalidOrderIdError (400).
• Any other 404 (unrouted path, wrong base URL), incomplete store/channel bindings, a non-string or mismatched order ID, and other upstream failures throw UpstreamError (502). fix(commerce-server): return 400/404 from order-status instead of 500 #1488 reports these as 500.
• fix(commerce-server): return 400/404 from order-status instead of 500 #1488's route-level error.name check is not carried over: the route goes through commerceRoute, which shares a module with the errors it maps. In-process callers should check error.code, which holds across duplicate package copies.

#1488's InvalidOrderIdError and OrderNotFoundError become subclasses of InvalidRequestError and NotFoundError; their bodies gain code and requestId.

Compatibility

• message removed from failure bodies. The bundled storefront reads only error (commerce-storefront/src/api.ts).
• Status codes changed: some 500s become 502, 503, or 404. Hosts checking for 500 should check code.
• New fields code and requestId in failure bodies.
• Changeset is minor because response codes and bodies change. App Builder pins ^0.1.1, so it won't pick this up until its range is bumped. fix(commerce-server): return 400/404 from order-status instead of 500 #1488's changeset is also minor, so neither reaches App Builder until its range is bumped.

Testing

• pnpm --filter @godaddy/gd-commerce-server typecheck, lint, test — 233 passed
• Per route: unexpected errors → 500 internal_error; upstream errors → 502 with no upstream text in the body and upstream codes in the log; cart writes on a missing cart → 404; an upstream GraphQL 401 → upstream_unauthorized, including a 401/403 embedded in an HTTP 200 body (extensions.status / extensions.http.status)
• Router: a host logger receives failures with the same id as the body; a throwing host logger still yields the standard body, with a console.error fallback; safe host ids are used and unsafe ones replaced; unreadable configuration → 503
• Order-status: Orders API NOT_FOUND → 404, VALIDATION_FAILED → 400, any other 404 → 502; token denied → 502 upstream_unauthorized; OAuth 400 invalid_request → 502 upstream_error; not configured → 503, including in-process
• Cart rule scope: "Order not found" during cart create or a catalog route → 502, not 404
• Request id: a throwing getRequestId still yields the standard body with a UUID; the resolver isn't called on success

🤖 Generated with Claude Code

[changeset-bot]

🦋 Changeset detected

Latest commit: 34f0279

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@godaddy/gd-commerce-server	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[sbolinger-godaddy]

No P0/P1 blockers found. I found two P2 items worth fixing:

1. GraphQL authentication errors can get the wrong code. In gql.ts:58, unauthorized checks only the HTTP status. I reproduced an HTTP 200 GraphQL response containing extensions.http.status: 401; it returns upstream_error instead of upstream_unauthorized. Include embedded 401/403 statuses when choosing the code.

2. A logger failure prevents the standard error response. In commerce-route.ts:75, the host logger runs before the JSON response is sent. I reproduced a throwing logger: the handler rejects without sending { error, code, requestId }. Guard logging so a logger failure does not prevent the response.

Verification:

• Read the description, full diff, repository guidance, and affected call paths.
• Build, typecheck, lint, and built-package import checks passed.
• 145 existing tests passed; two temporary tests confirmed the findings above.
• Local socket restrictions blocked the remaining integration checks. No live Commerce or browser verification was performed.

Reviewed commit: 30ca479f23d17123712db8f5bc1c15feb5a3e312.

[anam-godaddy]

Thanks — both confirmed and fixed in eb36409.

1. Embedded 401/403. GraphQLErrorWithCodes now reports upstream_unauthorized when the HTTP status or any error's embedded status (extensions.status / extensions.http.status, as normalised by gqlRequest) is 401/403. New tests: an end-to-end HTTP 200 response with each extension shape returns 502 upstream_unauthorized, and a mixed [ORDER_NOT_FOUND, FORBIDDEN 403] is upstream_unauthorized and does not clear the cart. GraphQL codes such as UNAUTHENTICATED/FORBIDDEN alone are deliberately not used, since we haven't confirmed Commerce sends them; that follows the same evidence rule as the mapping table.
2. Throwing logger. sendCommerceError now catches a host logger failure, falls back to console.error with the original context plus the logger's error, and still sends { error, code, requestId }. A new router test uses a throwing logger.

All four new tests fail without the fixes. Full suite: typecheck, lint, and 208 tests pass, including the socket-based integration tests your environment couldn't run.

[pbennett1-godaddy]

Review notes inline. The first four comments (request ID resolver, bare 404 in order lookup, checkout stale-cart status, cart rule scope) affect the error contract and are worth addressing before merge. The rest are smaller.

[pbennett1-godaddy]

The host's getRequestId runs outside any try/catch. If it throws (e.g. req.get('x-request-id')!.trim() with the header missing), Express's default handler sends an HTML 500, with no { error, code, requestId } body and no logger call. Suggest catching and falling back to randomUUID().

A smaller related point: this resolves eagerly on every request, while requestIdFor in commerce-route.ts has its own lazy fallback that ignores the resolver. Storing the resolver in res.locals and resolving lazily in one place would remove the split and also cover routes mounted outside the router.

[anam-godaddy]

Done in 34f0279. The router now stores the host resolver in res.locals instead of resolving eagerly, and requestIdFor resolves lazily in one place, at most once per request and only when a failure body needs an id. If getRequestId throws, it falls back to randomUUID(), so the standard { error, code, requestId } body is still sent. Tests cover a throwing resolver and assert the resolver isn't called on a successful request.

[pbennett1-godaddy]

isCartNotFoundError deliberately excludes bare 404s because "a bare HTTP 404 can also mean the upstream endpoint itself is unavailable", but here every 404 becomes OrderNotFoundError. A wrong storeId or an apiBaseUrl that doesn't serve the Orders route would make every lookup return 404 not_found, and since only 5xx is logged, nothing would be recorded. Could this require evidence of an actual missing order (body/error code), and otherwise throw UpstreamError?

[anam-godaddy]

Addressed in #1488 (c43083d) and merged here in 897cf6d. After checking order-api's REST handler, only a 404 whose body is { code: 'NOT_FOUND' } maps to OrderNotFoundError. Any other 404 (e.g. Express's HTML "Cannot GET" for an unrouted path, or another base URL's 404) throws UpstreamError, so it's a logged 502. A wrong storeId gets a 403 from order-api's authorization check, which was already a logged 502. Details: #1488 (comment)

[pbennett1-godaddy]

I see the comment explaining why this isn't classified. The result is that checkout with a stale or completed draftOrderId (e.g. already paid in another tab) returns 502 upstream_error. The README's 404 row says "a cart write against a missing, expired, or completed cart" returns not_found, which checkout is arguably part of, and the client can't tell this case apart from an outage. Either classify it (if checkout-api's response for this is known) or narrow the README wording to say checkout is excluded.

[anam-godaddy]

Went with narrowing the docs in 34f0279. We don't have evidence of what checkout-api returns for a stale or completed draftOrderId, so classifying it would break the mapping table's evidence rule. The README's 404 row now covers writes to an existing cart (/cart/:id/...) only and says checkout with a stale draft returns 502; the changeset says the same. Happy to add a rule once we capture a real response.

[pbennett1-godaddy]

This rule applies to every route that goes through commerceRoute, including POST /cart (create) and the catalog routes. For example, if addDraftOrder succeeds and the follow-up addLineItemBySkuId returns INTERNAL_SERVER_ERROR / "Order not found" (eventual consistency), the create request returns 404 "Cart not found" and leaves an orphaned cart. Should this rule be scoped to routes with a :id cart path parameter?

[anam-godaddy]

Agreed, done in 34f0279. commerceRoute takes a per-route classifyUpstreamError option, and only the five /cart/:id routes pass classifyCartUpstreamError. POST /cart and the catalog routes now return 502 upstream_error for "Order not found". Tests cover create (failing on the first item add after addDraftOrder succeeds) and products.

[pbennett1-godaddy]

RFC 6749 uses 400 for invalid_request, unsupported_grant_type, etc., not just invalid_client/invalid_grant. Treating every 400 as upstream_unauthorized could send operators rotating valid credentials when the actual problem is the request shape. Consider parsing the OAuth error field and only flagging invalid_client/invalid_grant/unauthorized_client as unauthorized.

[anam-godaddy]

Done in 34f0279. For a 400, the token request reads the OAuth error field. Only invalid_client, invalid_grant, unauthorized_client, and invalid_scope are upstream_unauthorized. I kept invalid_scope because it means the client lacks the scope it requested (e.g. commerce.order:read), which is a credentials/grant problem for operators. invalid_request, other codes, and non-JSON 400s are upstream_error. The error value is also logged as details.oauthError.

[pbennett1-godaddy]

This wrapping only happens through commerceConfigurationForResponse. The in-process getOrderStatus() (get-order-status.ts:73) and createCheckoutSession() (create-checkout-session.ts:190) call configuration.read() directly, so a host config that throws a plain Error reaches in-process callers unwrapped. The README says "In-process helpers throw the exported CommerceError subclasses". Applying readOrNotConfigured in those helpers too, or at a shared configuration boundary, would make that true for both paths.

[anam-godaddy]

Done in 34f0279. The wrapper is now guardCommerceConfiguration, used by commerceConfigurationForResponse and by getOrderStatus and createCheckoutSession directly, so a host configuration that throws a plain error is CommerceNotConfiguredError (503 not_configured, original error as cause) for in-process callers too.

[pbennett1-godaddy]

nit: getOrderStatus now fully validates the ID (missing, blank, padded, ., ..) and throws InvalidOrderIdError. Only the non-string check is unique to the route. Narrowing it to typeof orderId !== 'string' gives a single source of truth.

[anam-godaddy]

Done. The typeof orderId !== 'string' narrowing landed in #1488 and is merged here (897cf6d); getOrderStatus is the single source of truth for string IDs.

[pbennett1-godaddy]

nit: this fetch to UpstreamError and JSON-parse to UpstreamError wrapping is the third copy, after getOAuthAccessToken and gqlRequest, and the copies differ in which details they include (endpoint/scope). A shared fetchUpstream/readUpstreamJson helper would keep classification consistent.

[anam-godaddy]

Agreed it's duplication, but I've left the shared fetchUpstream/readUpstreamJson helper for a follow-up to keep this PR's scope contained. For the inconsistency you pointed out, the order lookup now logs endpoint with every upstream failure (plus the Orders API error code on 404/422), matching what gqlRequest logs (34f0279).

[pbennett1-godaddy]

nit: the router always installs a validated logger (options.logger ?? consoleCommerceLogger), so duck-typing res.locals.commerceLogger on every 5xx seems unnecessary. Typing res.locals or passing the logger through commerceRoute would remove the runtime check.

[anam-godaddy]

Done in 34f0279. The router's res.locals entries are typed (CommerceObservabilityLocals) and the runtime shape check is gone. Routes mounted outside the router still fall back to console.error, and the existing try/catch around the logger call covers a host logger that throws.