From e7d6e7946c4d4b4f44f5f8ae6d2d4f466a92370e Mon Sep 17 00:00:00 2001 From: Aaron Nam <112416280+anam-godaddy@users.noreply.github.com> Date: Wed, 30 Sep 2026 18:37:03 -0700 Subject: [PATCH 1/5] fix(commerce-server): return 400/404 from order-status instead of 500 Invalid order IDs (blank, padded, `.`, `..`) and missing orders previously surfaced as 500s, indistinguishable from credential or upstream failures. - getOrderStatus throws InvalidOrderIdError for invalid IDs and OrderNotFoundError when the Orders API returns 404 or the order is bound to another order ID, store, or channel. Incomplete responses stay generic. - The order-status route maps these to 400 and 404; everything else is 500. - Padded IDs are rejected rather than trimmed so the lookup never targets a different ID than the caller supplied. Co-Authored-By: Claude Opus 5.5 --- .changeset/clear-order-status-errors.md | 5 + packages/commerce-server/README.md | 2 +- .../src/get-order-status.test.ts | 116 ++++++++++++++++-- packages/commerce-server/src/index.ts | 2 + .../src/lib/commerce/get-order-status.ts | 33 ++++- .../server/api/commerce/order-status/GET.ts | 22 +++- 6 files changed, 161 insertions(+), 19 deletions(-) create mode 100644 .changeset/clear-order-status-errors.md diff --git a/.changeset/clear-order-status-errors.md b/.changeset/clear-order-status-errors.md new file mode 100644 index 00000000..dda3fadf --- /dev/null +++ b/.changeset/clear-order-status-errors.md @@ -0,0 +1,5 @@ +--- +'@godaddy/gd-commerce-server': patch +--- + +Return 400 from the order-status route for invalid order IDs and 404 when the order is missing or belongs to another store or channel, instead of 500. Export `InvalidOrderIdError` and `OrderNotFoundError` for in-process `getOrderStatus()` callers. diff --git a/packages/commerce-server/README.md b/packages/commerce-server/README.md index eb867c1f..71c69439 100644 --- a/packages/commerce-server/README.md +++ b/packages/commerce-server/README.md @@ -56,4 +56,4 @@ 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`; `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. The route returns 400 for a missing, blank, padded, `.`, or `..` order ID and 404 when the Orders API has no such order or the order belongs to another store or channel; credential and other upstream failures return 500. In-process callers can check for the exported `InvalidOrderIdError` and `OrderNotFoundError`. Hosts must authenticate callers and authorize access to each requested order before exposing this route. diff --git a/packages/commerce-server/src/get-order-status.test.ts b/packages/commerce-server/src/get-order-status.test.ts index ca89fc3f..ae6a033b 100644 --- a/packages/commerce-server/src/get-order-status.test.ts +++ b/packages/commerce-server/src/get-order-status.test.ts @@ -2,7 +2,7 @@ import { once } from 'node:events'; import express from 'express'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { createRuntimeCommerceConfiguration } from './lib/commerce/config'; -import { getOrderStatus } from './lib/commerce/get-order-status'; +import { getOrderStatus, InvalidOrderIdError, OrderNotFoundError } from './lib/commerce/get-order-status'; import { createGoDaddyPaymentsRouter } from './router'; const clientFetch = globalThis.fetch; @@ -126,20 +126,42 @@ describe('authorized order lookup', () => { }, ); + it.each([undefined, { ...order, context: undefined }])( + 'rejects an incomplete order response', + async (result): Promise => { + upstream + .mockResolvedValueOnce(Response.json({ access_token: 'order-token' })) + .mockResolvedValueOnce(Response.json({ order: result })); + const lookup = getOrderStatus(order.id, configuration); + await expect(lookup).rejects.toThrow('Order lookup did not return the requested order'); + await expect(lookup).rejects.not.toBeInstanceOf(OrderNotFoundError); + }, + ); + it.each([ - undefined, { ...order, id: 'another-order' }, { ...order, context: { ...order.context, storeId: 'another-store' } }, { ...order, context: { ...order.context, channelId: 'another-channel' } }, - { ...order, context: undefined }, - ])('rejects missing or mismatched order bindings', async (result): Promise => { + ])( + 'reports an order bound to another order, store, or channel as not found', + async (result): Promise => { + upstream + .mockResolvedValueOnce(Response.json({ access_token: 'order-token' })) + .mockResolvedValueOnce(Response.json({ order: result })); + await expect(getOrderStatus(order.id, configuration)).rejects.toBeInstanceOf(OrderNotFoundError); + }, + ); + + it('reports an upstream 404 as not found without exposing its body', async (): Promise => { upstream .mockResolvedValueOnce(Response.json({ access_token: 'order-token' })) - .mockResolvedValueOnce(Response.json({ order: result })); - await expect(getOrderStatus(order.id, configuration)).rejects.toThrow('Order lookup'); + .mockResolvedValueOnce(new Response('Private upstream details', { status: 404 })); + const lookup = getOrderStatus(order.id, configuration); + await expect(lookup).rejects.toBeInstanceOf(OrderNotFoundError); + await expect(lookup).rejects.toThrow(/^Order not found$/); }); - it.each([401, 403, 404, 500])( + it.each([401, 403, 500])( 'preserves an upstream order lookup failure (%i) without exposing its body', async (status): Promise => { upstream @@ -157,11 +179,87 @@ describe('authorized order lookup', () => { expect(upstream).toHaveBeenCalledTimes(1); }); - it.each(['', ' ', '.', '..'])( + it.each(['', ' ', '.', '..', ' completed-order', 'completed-order\n'])( 'rejects invalid order ID %j before requesting credentials', async (id): Promise => { - await expect(getOrderStatus(id, configuration)).rejects.toThrow('a valid orderId is required'); + const lookup = getOrderStatus(id, configuration); + await expect(lookup).rejects.toBeInstanceOf(InvalidOrderIdError); + await expect(lookup).rejects.toThrow('a valid orderId is required'); expect(upstream).not.toHaveBeenCalled(); }, ); }); + +describe('order-status route', () => { + async function requestOrderStatus(query: string): Promise<{ status: number; body: unknown }> { + const app = express(); + app.use('/api/commerce', createGoDaddyPaymentsRouter(configuration)); + 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/order-status${query}`, + ); + return { status: response.status, body: await response.json() }; + } finally { + await new Promise((resolve, reject) => + server.close((error) => (error ? reject(error) : resolve())), + ); + } + } + + it.each([ + '', + '?orderId=', + '?orderId=%20', + '?orderId=.', + '?orderId=..', + '?orderId=%20cart-1', + '?orderId=a&orderId=b', + ])( + 'returns 400 for invalid order ID query %j without an upstream request', + async (query): Promise => { + await expect(requestOrderStatus(query)).resolves.toEqual({ + status: 400, + body: { success: false, error: 'missing or invalid orderId query parameter' }, + }); + expect(upstream).not.toHaveBeenCalled(); + }, + ); + + it.each([ + ['an upstream 404', new Response('Private upstream details', { status: 404 })], + [ + 'another store', + Response.json({ order: { ...order, context: { ...order.context, storeId: 'another-store' } } }), + ], + ])('returns 404 for %s', async (_case, orderResponse): Promise => { + upstream + .mockResolvedValueOnce(Response.json({ access_token: 'order-token' })) + .mockResolvedValueOnce(orderResponse); + await expect(requestOrderStatus(`?orderId=${order.id}`)).resolves.toEqual({ + status: 404, + body: { success: false, error: 'Order not found' }, + }); + }); + + it.each([ + ['a denied token', [new Response('Invalid scope', { status: 403 })]], + [ + 'an upstream 500', + [ + Response.json({ access_token: 'order-token' }), + new Response('Private upstream details', { status: 500 }), + ], + ], + ['an incomplete order', [Response.json({ access_token: 'order-token' }), Response.json({})]], + ])('returns 500 for %s', async (_case, responses): Promise => { + for (const response of responses) upstream.mockResolvedValueOnce(response); + const result = await requestOrderStatus(`?orderId=${order.id}`); + expect(result.status).toBe(500); + expect(result.body).toMatchObject({ success: false, error: 'Failed to get order status' }); + expect(JSON.stringify(result.body)).not.toContain('Private upstream details'); + }); +}); diff --git a/packages/commerce-server/src/index.ts b/packages/commerce-server/src/index.ts index fd69b46b..596de819 100644 --- a/packages/commerce-server/src/index.ts +++ b/packages/commerce-server/src/index.ts @@ -19,6 +19,8 @@ export { export { type CommerceOrderStatus, getOrderStatus, + InvalidOrderIdError, + OrderNotFoundError, } from './lib/commerce/get-order-status'; export { type CommerceRouterFeatures, diff --git a/packages/commerce-server/src/lib/commerce/get-order-status.ts b/packages/commerce-server/src/lib/commerce/get-order-status.ts index a01c66a3..83e290f6 100644 --- a/packages/commerce-server/src/lib/commerce/get-order-status.ts +++ b/packages/commerce-server/src/lib/commerce/get-order-status.ts @@ -23,6 +23,20 @@ export interface CommerceOrderStatus { lineItems?: unknown[]; } +export class InvalidOrderIdError extends Error { + constructor() { + super('getOrderStatus: a valid orderId is required'); + this.name = 'InvalidOrderIdError'; + } +} + +export class OrderNotFoundError extends Error { + constructor() { + super('Order not found'); + this.name = 'OrderNotFoundError'; + } +} + interface OrderResponse { order?: { id: string; @@ -39,8 +53,15 @@ export async function getOrderStatus( orderId: string, configuration: CommerceConfiguration = createRuntimeCommerceConfiguration(), ): Promise { - if (typeof orderId !== 'string' || !orderId.trim() || orderId === '.' || orderId === '..') { - throw new Error('getOrderStatus: a valid orderId is required'); + // `.` and `..` survive encodeURIComponent and would resolve the URL to the store resource. + if ( + typeof orderId !== 'string' || + !orderId || + orderId !== orderId.trim() || + orderId === '.' || + orderId === '..' + ) { + throw new InvalidOrderIdError(); } const { storeId, channelId, clientId, clientSecret, apiBaseUrl, currencyCode } = configuration.read(); @@ -61,13 +82,15 @@ export async function getOrderStatus( cache: 'no-store', }, ); + if (response.status === 404) throw new OrderNotFoundError(); if (!response.ok) throw new Error(`Failed to load order: upstream returned ${response.status}`); const data = (await response.json()) as OrderResponse; const order = data?.order; - if (!order?.id || order.id !== orderId) throw new Error('Order lookup did not return the requested order'); - if (order.context?.storeId !== storeId || order.context?.channelId !== channelId) { - throw new Error('Order lookup returned a different store or channel'); + if (!order?.id || !order.context) throw new Error('Order lookup did not return the requested order'); + // Report an order bound to another store or channel as missing so the route doesn't reveal it exists. + if (order.id !== orderId || order.context.storeId !== storeId || order.context.channelId !== channelId) { + throw new OrderNotFoundError(); } const total = order.totals?.total; diff --git a/packages/commerce-server/src/server/api/commerce/order-status/GET.ts b/packages/commerce-server/src/server/api/commerce/order-status/GET.ts index 797f5404..82f892a1 100644 --- a/packages/commerce-server/src/server/api/commerce/order-status/GET.ts +++ b/packages/commerce-server/src/server/api/commerce/order-status/GET.ts @@ -11,26 +11,40 @@ * Query: * orderId - GoDaddy order id (required) * - * Response: { success: true, order: CommerceOrderStatus } - * order.status is the payment status returned by the authorized Orders API. + * Responses: + * 200 { success: true, order: CommerceOrderStatus } + * order.status is the payment status returned by the authorized Orders API. + * 400 missing, blank, padded, `.` or `..` orderId + * 404 no order with that id in the configured store and channel + * 500 credential, upstream, or configuration failure * The host must authorize the caller's access to the requested order. */ import type { Request, Response } from 'express'; import { commerceConfigurationForResponse } from '@/lib/commerce/config'; -import { getOrderStatus } from '@/lib/commerce/get-order-status'; +import { getOrderStatus, InvalidOrderIdError, OrderNotFoundError } from '@/lib/commerce/get-order-status'; + +const invalidOrderIdBody = { success: false, error: 'missing or invalid orderId query parameter' }; export default async function handler(req: Request, res: Response): Promise { try { const { orderId } = req.query; if (!orderId || typeof orderId !== 'string') { - res.status(400).json({ success: false, error: 'missing or invalid orderId query parameter' }); + res.status(400).json(invalidOrderIdBody); return; } const order = await getOrderStatus(orderId, commerceConfigurationForResponse(res)); res.status(200).json({ success: true, order }); } catch (error) { + if (error instanceof InvalidOrderIdError) { + res.status(400).json(invalidOrderIdBody); + return; + } + if (error instanceof OrderNotFoundError) { + res.status(404).json({ success: false, error: 'Order not found' }); + return; + } res.status(500).json({ success: false, error: 'Failed to get order status', From 0482854e9d24dc397c2867d94390dd7ac0f382d6 Mon Sep 17 00:00:00 2001 From: Aaron Nam <112416280+anam-godaddy@users.noreply.github.com> Date: Mon, 5 Oct 2026 12:20:50 -0700 Subject: [PATCH 2/5] fix(commerce-server): keep order-status 500 detail server-side The 500 body echoed the internal error message, which can name upstream HTTP statuses, OAuth/token failures, or configuration problems. Return only { success: false, error } and log the detail with console.error. Export ORDER_STATUS_UNKNOWN so callers compare against a constant rather than the 'unknown' literal; status stays a string because the Orders API's full paymentStatus set is not documented here. Co-Authored-By: Claude Opus 5.5 --- .changeset/clear-order-status-errors.md | 2 +- packages/commerce-server/README.md | 2 +- .../src/get-order-status.test.ts | 18 +++++++++++------- packages/commerce-server/src/index.ts | 1 + .../src/lib/commerce/get-order-status.ts | 7 +++++-- .../server/api/commerce/order-status/GET.ts | 8 +++----- 6 files changed, 22 insertions(+), 16 deletions(-) diff --git a/.changeset/clear-order-status-errors.md b/.changeset/clear-order-status-errors.md index dda3fadf..6ae979f0 100644 --- a/.changeset/clear-order-status-errors.md +++ b/.changeset/clear-order-status-errors.md @@ -2,4 +2,4 @@ '@godaddy/gd-commerce-server': patch --- -Return 400 from the order-status route for invalid order IDs and 404 when the order is missing or belongs to another store or channel, instead of 500. Export `InvalidOrderIdError` and `OrderNotFoundError` for in-process `getOrderStatus()` callers. +Return 400 from the order-status route for invalid order IDs and 404 when the order is missing or belongs to another store or channel, instead of 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. diff --git a/packages/commerce-server/README.md b/packages/commerce-server/README.md index 71c69439..4a961c22 100644 --- a/packages/commerce-server/README.md +++ b/packages/commerce-server/README.md @@ -56,4 +56,4 @@ 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. The route returns 400 for a missing, blank, padded, `.`, or `..` order ID and 404 when the Orders API has no such order or the order belongs to another store or channel; credential and other upstream failures return 500. In-process callers can check for the exported `InvalidOrderIdError` and `OrderNotFoundError`. 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) and 404 when the Orders API has no such order or the order belongs to another store or channel. Credential and other upstream failures return 500 with a generic body; the detail is logged server-side with `console.error`. In-process callers can check for the exported `InvalidOrderIdError` and `OrderNotFoundError`. Hosts must authenticate callers and authorize access to each requested order before exposing this route. diff --git a/packages/commerce-server/src/get-order-status.test.ts b/packages/commerce-server/src/get-order-status.test.ts index ae6a033b..3b94ffd2 100644 --- a/packages/commerce-server/src/get-order-status.test.ts +++ b/packages/commerce-server/src/get-order-status.test.ts @@ -255,11 +255,15 @@ describe('order-status route', () => { ], ], ['an incomplete order', [Response.json({ access_token: 'order-token' }), Response.json({})]], - ])('returns 500 for %s', async (_case, responses): Promise => { - for (const response of responses) upstream.mockResolvedValueOnce(response); - const result = await requestOrderStatus(`?orderId=${order.id}`); - expect(result.status).toBe(500); - expect(result.body).toMatchObject({ success: false, error: 'Failed to get order status' }); - expect(JSON.stringify(result.body)).not.toContain('Private upstream details'); - }); + ])( + 'returns a generic 500 for %s and logs the detail server-side', + async (_case, responses): Promise => { + const log = vi.spyOn(console, 'error').mockImplementation((): void => {}); + for (const response of responses) upstream.mockResolvedValueOnce(response); + const result = await requestOrderStatus(`?orderId=${order.id}`); + expect(result).toEqual({ status: 500, body: { success: false, error: 'Failed to get order status' } }); + expect(log).toHaveBeenCalledWith('order-status: failed to get order status', expect.any(Error)); + log.mockRestore(); + }, + ); }); diff --git a/packages/commerce-server/src/index.ts b/packages/commerce-server/src/index.ts index 596de819..385a8993 100644 --- a/packages/commerce-server/src/index.ts +++ b/packages/commerce-server/src/index.ts @@ -20,6 +20,7 @@ export { type CommerceOrderStatus, getOrderStatus, InvalidOrderIdError, + ORDER_STATUS_UNKNOWN, OrderNotFoundError, } from './lib/commerce/get-order-status'; export { diff --git a/packages/commerce-server/src/lib/commerce/get-order-status.ts b/packages/commerce-server/src/lib/commerce/get-order-status.ts index 83e290f6..d460e462 100644 --- a/packages/commerce-server/src/lib/commerce/get-order-status.ts +++ b/packages/commerce-server/src/lib/commerce/get-order-status.ts @@ -6,10 +6,13 @@ import { authorizationHeaders, getOAuthAccessToken } from './checkout-subgraph'; import { type CommerceConfiguration, createRuntimeCommerceConfiguration } from './config'; import type { Money } from './gql'; +/** `CommerceOrderStatus.status` when the Orders API reports no payment status. */ +export const ORDER_STATUS_UNKNOWN = 'unknown'; + export interface CommerceOrderStatus { /** GoDaddy order id. */ id: string; - /** Payment status reported by Commerce, or 'unknown' when it is unavailable. */ + /** Payment status reported by Commerce (e.g. `PAID`, `PENDING`), or `ORDER_STATUS_UNKNOWN`. */ status: string; /** Total amount in the currency's smallest unit (cents for USD). */ amount: number; @@ -96,7 +99,7 @@ export async function getOrderStatus( return { id: order.id, - status: order.statuses?.paymentStatus ?? 'unknown', + status: order.statuses?.paymentStatus ?? ORDER_STATUS_UNKNOWN, amount: total?.value ?? 0, currency: total?.currencyCode ?? currencyCode, createdAt: order.createdAt, diff --git a/packages/commerce-server/src/server/api/commerce/order-status/GET.ts b/packages/commerce-server/src/server/api/commerce/order-status/GET.ts index 82f892a1..6de46885 100644 --- a/packages/commerce-server/src/server/api/commerce/order-status/GET.ts +++ b/packages/commerce-server/src/server/api/commerce/order-status/GET.ts @@ -45,10 +45,8 @@ export default async function handler(req: Request, res: Response): Promise Date: Mon, 5 Oct 2026 17:59:16 -0700 Subject: [PATCH 3/5] fix(commerce-server): treat incomplete order bindings as upstream failures An Orders API response whose context lacks storeId or channelId fell through to the store/channel mismatch check and was reported as 404. Require both binding fields before comparing them so malformed responses keep the generic 500 and server-side logging. Co-Authored-By: Claude Opus 5.5 --- .../src/get-order-status.test.ts | 33 ++++++++++++------- .../src/lib/commerce/get-order-status.ts | 12 +++++-- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/packages/commerce-server/src/get-order-status.test.ts b/packages/commerce-server/src/get-order-status.test.ts index 3b94ffd2..d348791a 100644 --- a/packages/commerce-server/src/get-order-status.test.ts +++ b/packages/commerce-server/src/get-order-status.test.ts @@ -126,17 +126,21 @@ describe('authorized order lookup', () => { }, ); - it.each([undefined, { ...order, context: undefined }])( - 'rejects an incomplete order response', - async (result): Promise => { - upstream - .mockResolvedValueOnce(Response.json({ access_token: 'order-token' })) - .mockResolvedValueOnce(Response.json({ order: result })); - const lookup = getOrderStatus(order.id, configuration); - await expect(lookup).rejects.toThrow('Order lookup did not return the requested order'); - await expect(lookup).rejects.not.toBeInstanceOf(OrderNotFoundError); - }, - ); + it.each([ + undefined, + { ...order, context: undefined }, + { ...order, context: {} }, + { ...order, context: { channelId: order.context.channelId } }, + { ...order, context: { storeId: order.context.storeId } }, + { ...order, context: { ...order.context, storeId: '' } }, + ])('rejects an incomplete order response', async (result): Promise => { + upstream + .mockResolvedValueOnce(Response.json({ access_token: 'order-token' })) + .mockResolvedValueOnce(Response.json({ order: result })); + const lookup = getOrderStatus(order.id, configuration); + await expect(lookup).rejects.toThrow('Order lookup did not return the requested order'); + await expect(lookup).rejects.not.toBeInstanceOf(OrderNotFoundError); + }); it.each([ { ...order, id: 'another-order' }, @@ -255,6 +259,13 @@ describe('order-status route', () => { ], ], ['an incomplete order', [Response.json({ access_token: 'order-token' }), Response.json({})]], + [ + 'an order missing its store binding', + [ + Response.json({ access_token: 'order-token' }), + Response.json({ order: { ...order, context: { channelId: order.context.channelId } } }), + ], + ], ])( 'returns a generic 500 for %s and logs the detail server-side', async (_case, responses): Promise => { diff --git a/packages/commerce-server/src/lib/commerce/get-order-status.ts b/packages/commerce-server/src/lib/commerce/get-order-status.ts index d460e462..9b97b7e4 100644 --- a/packages/commerce-server/src/lib/commerce/get-order-status.ts +++ b/packages/commerce-server/src/lib/commerce/get-order-status.ts @@ -52,6 +52,10 @@ interface OrderResponse { }; } +function isBindingId(value: unknown): value is string { + return typeof value === 'string' && value !== ''; +} + export async function getOrderStatus( orderId: string, configuration: CommerceConfiguration = createRuntimeCommerceConfiguration(), @@ -90,9 +94,13 @@ export async function getOrderStatus( const data = (await response.json()) as OrderResponse; const order = data?.order; - if (!order?.id || !order.context) throw new Error('Order lookup did not return the requested order'); + const context = order?.context; + // A binding missing either field is a malformed upstream response, not proof of another store or channel. + if (!order?.id || !isBindingId(context?.storeId) || !isBindingId(context?.channelId)) { + throw new Error('Order lookup did not return the requested order'); + } // Report an order bound to another store or channel as missing so the route doesn't reveal it exists. - if (order.id !== orderId || order.context.storeId !== storeId || order.context.channelId !== channelId) { + if (order.id !== orderId || context.storeId !== storeId || context.channelId !== channelId) { throw new OrderNotFoundError(); } const total = order.totals?.total; From 456728b5cc95993f56250fa5f5925cefc86f8842 Mon Sep 17 00:00:00 2001 From: Aaron Nam <112416280+anam-godaddy@users.noreply.github.com> Date: Tue, 6 Oct 2026 11:57:53 -0700 Subject: [PATCH 4/5] fix(commerce-server): tighten order-status error classification - Treat an upstream order ID mismatch or non-string ID as a malformed response (500 + log) instead of not found. - Cancel unread error response bodies and log upstream 404s. - Match route errors by name so they survive duplicate package copies. - Leave string ID validation to getOrderStatus. - Restore mocks in afterEach. - Bump the changeset to minor. Co-Authored-By: Claude Opus 5.5 --- .changeset/clear-order-status-errors.md | 4 +- packages/commerce-server/README.md | 2 +- .../src/get-order-status.test.ts | 63 +++++++++++++++---- .../src/lib/commerce/get-order-status.ts | 22 +++++-- .../server/api/commerce/order-status/GET.ts | 14 +++-- 5 files changed, 81 insertions(+), 24 deletions(-) diff --git a/.changeset/clear-order-status-errors.md b/.changeset/clear-order-status-errors.md index 6ae979f0..42aabe02 100644 --- a/.changeset/clear-order-status-errors.md +++ b/.changeset/clear-order-status-errors.md @@ -1,5 +1,5 @@ --- -'@godaddy/gd-commerce-server': patch +'@godaddy/gd-commerce-server': minor --- -Return 400 from the order-status route for invalid order IDs and 404 when the order is missing or belongs to another store or channel, instead of 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. +Return 400 from the order-status route for invalid order IDs and 404 when the order is missing or belongs to another store or channel, instead of 500. An upstream 404 is also logged with `console.warn`. 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. diff --git a/packages/commerce-server/README.md b/packages/commerce-server/README.md index 4a961c22..b53e433c 100644 --- a/packages/commerce-server/README.md +++ b/packages/commerce-server/README.md @@ -56,4 +56,4 @@ 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`; 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) and 404 when the Orders API has no such order or the order belongs to another store or channel. Credential and other upstream failures return 500 with a generic body; the detail is logged server-side with `console.error`. In-process callers can check for the exported `InvalidOrderIdError` and `OrderNotFoundError`. 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) and 404 when the Orders API has no such order or the order belongs to another store or channel. Because the Orders API also returns 404 for a misconfigured store ID or API base URL, upstream 404s are logged with `console.warn`. Credential and other upstream failures, including a response for a different order ID, return 500 with a generic body; the detail is logged server-side with `console.error`. In-process callers can identify the exported `InvalidOrderIdError` and `OrderNotFoundError` by `error.name`; prefer that to `instanceof`, which fails when a host has more than one copy of this package. Hosts must authenticate callers and authorize access to each requested order before exposing this route. diff --git a/packages/commerce-server/src/get-order-status.test.ts b/packages/commerce-server/src/get-order-status.test.ts index d348791a..4467f74d 100644 --- a/packages/commerce-server/src/get-order-status.test.ts +++ b/packages/commerce-server/src/get-order-status.test.ts @@ -52,6 +52,7 @@ beforeEach((): void => { }); afterEach((): void => { vi.unstubAllGlobals(); + vi.restoreAllMocks(); }); describe('authorized order lookup', () => { @@ -133,7 +134,10 @@ describe('authorized order lookup', () => { { ...order, context: { channelId: order.context.channelId } }, { ...order, context: { storeId: order.context.storeId } }, { ...order, context: { ...order.context, storeId: '' } }, - ])('rejects an incomplete order response', async (result): Promise => { + { ...order, id: undefined }, + { ...order, id: 42 }, + { ...order, id: 'another-order' }, + ])('rejects an incomplete or mismatched order response', async (result): Promise => { upstream .mockResolvedValueOnce(Response.json({ access_token: 'order-token' })) .mockResolvedValueOnce(Response.json({ order: result })); @@ -143,37 +147,39 @@ describe('authorized order lookup', () => { }); it.each([ - { ...order, id: 'another-order' }, { ...order, context: { ...order.context, storeId: 'another-store' } }, { ...order, context: { ...order.context, channelId: 'another-channel' } }, - ])( - 'reports an order bound to another order, store, or channel as not found', - async (result): Promise => { - upstream - .mockResolvedValueOnce(Response.json({ access_token: 'order-token' })) - .mockResolvedValueOnce(Response.json({ order: result })); - await expect(getOrderStatus(order.id, configuration)).rejects.toBeInstanceOf(OrderNotFoundError); - }, - ); + ])('reports an order bound to another store or channel as not found', async (result): Promise => { + upstream + .mockResolvedValueOnce(Response.json({ access_token: 'order-token' })) + .mockResolvedValueOnce(Response.json({ order: result })); + await expect(getOrderStatus(order.id, configuration)).rejects.toBeInstanceOf(OrderNotFoundError); + }); - it('reports an upstream 404 as not found without exposing its body', async (): Promise => { + it('reports an upstream 404 as not found, logs it, and releases the body', async (): Promise => { + const warn = vi.spyOn(console, 'warn').mockImplementation((): void => {}); + const cancel = vi.spyOn(ReadableStream.prototype, 'cancel'); upstream .mockResolvedValueOnce(Response.json({ access_token: 'order-token' })) .mockResolvedValueOnce(new Response('Private upstream details', { status: 404 })); const lookup = getOrderStatus(order.id, configuration); await expect(lookup).rejects.toBeInstanceOf(OrderNotFoundError); await expect(lookup).rejects.toThrow(/^Order not found$/); + expect(warn).toHaveBeenCalledWith('getOrderStatus: Orders API returned 404 for store store-1'); + expect(cancel).toHaveBeenCalledTimes(1); }); it.each([401, 403, 500])( 'preserves an upstream order lookup failure (%i) without exposing its body', async (status): Promise => { + const cancel = vi.spyOn(ReadableStream.prototype, 'cancel'); upstream .mockResolvedValueOnce(Response.json({ access_token: 'order-token' })) .mockResolvedValueOnce(new Response('Private upstream details', { status })); await expect(getOrderStatus(order.id, configuration)).rejects.toThrow( `Failed to load order: upstream returned ${status}`, ); + expect(cancel).toHaveBeenCalledTimes(1); }, ); @@ -240,6 +246,7 @@ describe('order-status route', () => { Response.json({ order: { ...order, context: { ...order.context, storeId: 'another-store' } } }), ], ])('returns 404 for %s', async (_case, orderResponse): Promise => { + vi.spyOn(console, 'warn').mockImplementation((): void => {}); upstream .mockResolvedValueOnce(Response.json({ access_token: 'order-token' })) .mockResolvedValueOnce(orderResponse); @@ -266,6 +273,13 @@ describe('order-status route', () => { Response.json({ order: { ...order, context: { channelId: order.context.channelId } } }), ], ], + [ + 'a different order ID', + [ + Response.json({ access_token: 'order-token' }), + Response.json({ order: { ...order, id: 'another-order' } }), + ], + ], ])( 'returns a generic 500 for %s and logs the detail server-side', async (_case, responses): Promise => { @@ -274,7 +288,30 @@ describe('order-status route', () => { const result = await requestOrderStatus(`?orderId=${order.id}`); expect(result).toEqual({ status: 500, body: { success: false, error: 'Failed to get order status' } }); expect(log).toHaveBeenCalledWith('order-status: failed to get order status', expect.any(Error)); - log.mockRestore(); }, ); }); + +describe('order-status route with another copy of the package', () => { + afterEach((): void => { + vi.doUnmock('@/lib/commerce/get-order-status'); + vi.resetModules(); + }); + + it.each([ + ['InvalidOrderIdError', 400, 'missing or invalid orderId query parameter'], + ['OrderNotFoundError', 404, 'Order not found'], + ])('matches a %s thrown by a different class by name', async (name, status, error): Promise => { + vi.resetModules(); + vi.doMock('@/lib/commerce/get-order-status', () => ({ + getOrderStatus: async (): Promise => { + throw Object.assign(new Error('from another copy'), { name }); + }, + })); + const { default: handler } = await import('./server/api/commerce/order-status/GET'); + const res = { locals: {}, status: vi.fn().mockReturnThis(), json: vi.fn().mockReturnThis() }; + await handler({ query: { orderId: order.id } } as never, res as never); + expect(res.status).toHaveBeenCalledWith(status); + expect(res.json).toHaveBeenCalledWith({ success: false, error }); + }); +}); diff --git a/packages/commerce-server/src/lib/commerce/get-order-status.ts b/packages/commerce-server/src/lib/commerce/get-order-status.ts index 9b97b7e4..85541815 100644 --- a/packages/commerce-server/src/lib/commerce/get-order-status.ts +++ b/packages/commerce-server/src/lib/commerce/get-order-status.ts @@ -89,18 +89,32 @@ export async function getOrderStatus( cache: 'no-store', }, ); - if (response.status === 404) throw new OrderNotFoundError(); - if (!response.ok) throw new Error(`Failed to load order: upstream returned ${response.status}`); + if (!response.ok) { + // Unread bodies can hold pooled connections while callers poll. + await response.body?.cancel(); + if (response.status === 404) { + // The Orders API doesn't distinguish a missing order from a wrong store or base URL, so log for diagnosis. + console.warn(`getOrderStatus: Orders API returned 404 for store ${storeId}`); + throw new OrderNotFoundError(); + } + throw new Error(`Failed to load order: upstream returned ${response.status}`); + } const data = (await response.json()) as OrderResponse; const order = data?.order; const context = order?.context; // A binding missing either field is a malformed upstream response, not proof of another store or channel. - if (!order?.id || !isBindingId(context?.storeId) || !isBindingId(context?.channelId)) { + // So is a different order id: the lookup was by id, so it means upstream returned the wrong record. + if ( + !isBindingId(order?.id) || + order.id !== orderId || + !isBindingId(context?.storeId) || + !isBindingId(context?.channelId) + ) { throw new Error('Order lookup did not return the requested order'); } // Report an order bound to another store or channel as missing so the route doesn't reveal it exists. - if (order.id !== orderId || context.storeId !== storeId || context.channelId !== channelId) { + if (context.storeId !== storeId || context.channelId !== channelId) { throw new OrderNotFoundError(); } const total = order.totals?.total; diff --git a/packages/commerce-server/src/server/api/commerce/order-status/GET.ts b/packages/commerce-server/src/server/api/commerce/order-status/GET.ts index 6de46885..5791de86 100644 --- a/packages/commerce-server/src/server/api/commerce/order-status/GET.ts +++ b/packages/commerce-server/src/server/api/commerce/order-status/GET.ts @@ -22,14 +22,20 @@ import type { Request, Response } from 'express'; import { commerceConfigurationForResponse } from '@/lib/commerce/config'; -import { getOrderStatus, InvalidOrderIdError, OrderNotFoundError } from '@/lib/commerce/get-order-status'; +import { getOrderStatus } from '@/lib/commerce/get-order-status'; const invalidOrderIdBody = { success: false, error: 'missing or invalid orderId query parameter' }; +// Matched by name, not instanceof, so errors still match when a host has several copies of this package. +function hasErrorName(error: unknown, name: string): boolean { + return error instanceof Error && error.name === name; +} + export default async function handler(req: Request, res: Response): Promise { try { const { orderId } = req.query; - if (!orderId || typeof orderId !== 'string') { + // getOrderStatus validates string IDs; only repeated or nested query values need rejecting here. + if (typeof orderId !== 'string') { res.status(400).json(invalidOrderIdBody); return; } @@ -37,11 +43,11 @@ export default async function handler(req: Request, res: Response): Promise Date: Tue, 6 Oct 2026 12:20:04 -0700 Subject: [PATCH 5/5] fix(commerce-server): classify order-status 404s by the Orders API error code The Orders API reports a missing order as 404 with code NOT_FOUND and an undecodable order ID as 422 with code VALIDATION_FAILED. Only those map to OrderNotFoundError and InvalidOrderIdError; any other 404, such as an unrouted path or misconfigured base URL, is an upstream failure (logged 500). This replaces the console.warn on every upstream 404. Co-Authored-By: Claude Opus 5.5 --- .changeset/clear-order-status-errors.md | 2 +- packages/commerce-server/README.md | 2 +- .../src/get-order-status.test.ts | 62 ++++++++++++++++--- .../src/lib/commerce/get-order-status.ts | 24 +++++-- .../server/api/commerce/order-status/GET.ts | 2 +- 5 files changed, 75 insertions(+), 17 deletions(-) diff --git a/.changeset/clear-order-status-errors.md b/.changeset/clear-order-status-errors.md index 42aabe02..bcd7f745 100644 --- a/.changeset/clear-order-status-errors.md +++ b/.changeset/clear-order-status-errors.md @@ -2,4 +2,4 @@ '@godaddy/gd-commerce-server': minor --- -Return 400 from the order-status route for invalid order IDs and 404 when the order is missing or belongs to another store or channel, instead of 500. An upstream 404 is also logged with `console.warn`. 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. +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. diff --git a/packages/commerce-server/README.md b/packages/commerce-server/README.md index b53e433c..91ed9574 100644 --- a/packages/commerce-server/README.md +++ b/packages/commerce-server/README.md @@ -56,4 +56,4 @@ 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`; 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) and 404 when the Orders API has no such order or the order belongs to another store or channel. Because the Orders API also returns 404 for a misconfigured store ID or API base URL, upstream 404s are logged with `console.warn`. Credential and other upstream failures, including a response for a different order ID, return 500 with a generic body; the detail is logged server-side with `console.error`. In-process callers can identify the exported `InvalidOrderIdError` and `OrderNotFoundError` by `error.name`; prefer that to `instanceof`, which fails when a host has more than one copy of this package. 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 treated as an upstream failure. Credential and other upstream failures, including a response for a different order ID, return 500 with a generic body; the detail is logged server-side with `console.error`. In-process callers can identify the exported `InvalidOrderIdError` and `OrderNotFoundError` by `error.name`; prefer that to `instanceof`, which fails when a host has more than one copy of this package. Hosts must authenticate callers and authorize access to each requested order before exposing this route. diff --git a/packages/commerce-server/src/get-order-status.test.ts b/packages/commerce-server/src/get-order-status.test.ts index 4467f74d..0cb85279 100644 --- a/packages/commerce-server/src/get-order-status.test.ts +++ b/packages/commerce-server/src/get-order-status.test.ts @@ -34,6 +34,17 @@ const summary = { updatedAt: order.updatedAt, lineItems: [{ id: 'line-1', name: 'Mug', quantity: 1 }], }; +// Bodies the Orders API's REST error handler sends for a missing order and an undecodable ID. +const ordersApiNotFound = (): Response => + Response.json( + { code: 'NOT_FOUND', message: 'Order not found. Possible reasons: invalid orderId, storeID mismatch.' }, + { status: 404 }, + ); +const ordersApiInvalidId = (): Response => + Response.json( + { code: 'VALIDATION_FAILED', message: 'Invalid global ID: completed-order' }, + { status: 422 }, + ); let upstream: ReturnType>; beforeEach((): void => { @@ -156,17 +167,36 @@ describe('authorized order lookup', () => { await expect(getOrderStatus(order.id, configuration)).rejects.toBeInstanceOf(OrderNotFoundError); }); - it('reports an upstream 404 as not found, logs it, and releases the body', async (): Promise => { - const warn = vi.spyOn(console, 'warn').mockImplementation((): void => {}); - const cancel = vi.spyOn(ReadableStream.prototype, 'cancel'); + it('reports an Orders API NOT_FOUND as not found without exposing its body', async (): Promise => { upstream .mockResolvedValueOnce(Response.json({ access_token: 'order-token' })) - .mockResolvedValueOnce(new Response('Private upstream details', { status: 404 })); + .mockResolvedValueOnce(ordersApiNotFound()); const lookup = getOrderStatus(order.id, configuration); await expect(lookup).rejects.toBeInstanceOf(OrderNotFoundError); await expect(lookup).rejects.toThrow(/^Order not found$/); - expect(warn).toHaveBeenCalledWith('getOrderStatus: Orders API returned 404 for store store-1'); - expect(cancel).toHaveBeenCalledTimes(1); + }); + + it('reports an Orders API VALIDATION_FAILED for the order ID as an invalid order ID', async (): Promise => { + upstream + .mockResolvedValueOnce(Response.json({ access_token: 'order-token' })) + .mockResolvedValueOnce(ordersApiInvalidId()); + await expect(getOrderStatus(order.id, configuration)).rejects.toBeInstanceOf(InvalidOrderIdError); + }); + + it.each([ + ['an HTML 404 from an unrouted path', new Response('
Cannot GET /v1/x
', { status: 404 })], + ['a JSON 404 without a code', Response.json({ message: 'Not Found' }, { status: 404 })], + ['a 404 with another code', Response.json({ code: 'ROUTE_NOT_FOUND' }, { status: 404 })], + ['a 422 with another code', Response.json({ code: 'CONFLICT' }, { status: 422 })], + ['a non-JSON 422', new Response('Unprocessable', { status: 422 })], + ])('treats %s as an upstream failure', async (_case, failure): Promise => { + upstream + .mockResolvedValueOnce(Response.json({ access_token: 'order-token' })) + .mockResolvedValueOnce(failure); + const lookup = getOrderStatus(order.id, configuration); + await expect(lookup).rejects.toThrow(`Failed to load order: upstream returned ${failure.status}`); + await expect(lookup).rejects.not.toBeInstanceOf(OrderNotFoundError); + await expect(lookup).rejects.not.toBeInstanceOf(InvalidOrderIdError); }); it.each([401, 403, 500])( @@ -239,14 +269,23 @@ describe('order-status route', () => { }, ); + it('returns 400 when the Orders API rejects the order ID format', async (): Promise => { + upstream + .mockResolvedValueOnce(Response.json({ access_token: 'order-token' })) + .mockResolvedValueOnce(ordersApiInvalidId()); + await expect(requestOrderStatus(`?orderId=${order.id}`)).resolves.toEqual({ + status: 400, + body: { success: false, error: 'missing or invalid orderId query parameter' }, + }); + }); + it.each([ - ['an upstream 404', new Response('Private upstream details', { status: 404 })], + ['an Orders API NOT_FOUND', ordersApiNotFound()], [ 'another store', Response.json({ order: { ...order, context: { ...order.context, storeId: 'another-store' } } }), ], ])('returns 404 for %s', async (_case, orderResponse): Promise => { - vi.spyOn(console, 'warn').mockImplementation((): void => {}); upstream .mockResolvedValueOnce(Response.json({ access_token: 'order-token' })) .mockResolvedValueOnce(orderResponse); @@ -273,6 +312,13 @@ describe('order-status route', () => { Response.json({ order: { ...order, context: { channelId: order.context.channelId } } }), ], ], + [ + 'a 404 from an unrouted path', + [ + Response.json({ access_token: 'order-token' }), + new Response('
Cannot GET /v1/x
', { status: 404 }), + ], + ], [ 'a different order ID', [ diff --git a/packages/commerce-server/src/lib/commerce/get-order-status.ts b/packages/commerce-server/src/lib/commerce/get-order-status.ts index 85541815..48d5e4f5 100644 --- a/packages/commerce-server/src/lib/commerce/get-order-status.ts +++ b/packages/commerce-server/src/lib/commerce/get-order-status.ts @@ -56,6 +56,17 @@ function isBindingId(value: unknown): value is string { return typeof value === 'string' && value !== ''; } +async function readErrorCode(response: Response): Promise { + try { + const body: unknown = await response.json(); + return body && typeof body === 'object' && 'code' in body && typeof body.code === 'string' + ? body.code + : null; + } catch { + return null; + } +} + export async function getOrderStatus( orderId: string, configuration: CommerceConfiguration = createRuntimeCommerceConfiguration(), @@ -90,13 +101,14 @@ export async function getOrderStatus( }, ); if (!response.ok) { + // The Orders API reports a missing order as 404 NOT_FOUND and an ID it can't decode as 422 + // VALIDATION_FAILED. A 404 without that code comes from an unrouted path or base URL. + const code = + response.status === 404 || response.status === 422 ? await readErrorCode(response) : undefined; // Unread bodies can hold pooled connections while callers poll. - await response.body?.cancel(); - if (response.status === 404) { - // The Orders API doesn't distinguish a missing order from a wrong store or base URL, so log for diagnosis. - console.warn(`getOrderStatus: Orders API returned 404 for store ${storeId}`); - throw new OrderNotFoundError(); - } + if (code === undefined) await response.body?.cancel(); + if (response.status === 404 && code === 'NOT_FOUND') throw new OrderNotFoundError(); + if (response.status === 422 && code === 'VALIDATION_FAILED') throw new InvalidOrderIdError(); throw new Error(`Failed to load order: upstream returned ${response.status}`); } diff --git a/packages/commerce-server/src/server/api/commerce/order-status/GET.ts b/packages/commerce-server/src/server/api/commerce/order-status/GET.ts index 5791de86..cf8e288e 100644 --- a/packages/commerce-server/src/server/api/commerce/order-status/GET.ts +++ b/packages/commerce-server/src/server/api/commerce/order-status/GET.ts @@ -14,7 +14,7 @@ * Responses: * 200 { success: true, order: CommerceOrderStatus } * order.status is the payment status returned by the authorized Orders API. - * 400 missing, blank, padded, `.` or `..` orderId + * 400 missing, blank, padded, `.` or `..` orderId, or one the Orders API rejects as malformed * 404 no order with that id in the configured store and channel * 500 credential, upstream, or configuration failure * The host must authorize the caller's access to the requested order.