Skip to content

fix(rest/nodejs): fail completion when a line item's product no longer exists - #274

Open
arjun2075 wants to merge 1 commit into
Universal-Commerce-Protocol:mainfrom
arjun2075:fix/nodejs-deleted-product-completion
Open

arjun2075 wants to merge 1 commit into
Universal-Commerce-Protocol:mainfrom
arjun2075:fix/nodejs-deleted-product-completion

Conversation

@arjun2075

Copy link
Copy Markdown
Contributor

Problem

Completing a checkout whose line item's product had been deleted from the catalog returned 200 completed and created an order. No stock was reserved for the missing product.

Root cause

The reservation loop in completeCheckout treated a missing product as nothing to reserve (if (product && !reserveStock(...))), so the order was still created.

Fix

Reject the completion with InvalidRequestError (Product X not found, HTTP 400 INVALID_REQUEST), inside the existing completion transaction. This is the same behavior as the Python reference in #248 and the same message the Node create and update paths already use. Because the throw is inside runInTransaction, earlier reservations roll back and no order is written.

Regression coverage

Two tests in lifecycle.test.ts, next to the existing rollback tests:

  • a single deleted product: 400 with the UCP error envelope, stock unchanged, no order persisted, the checkout stays ready_for_complete with no order reference, and it completes once the product exists again
  • a deleted product on a later line: the first line's reservation rolls back, no order is persisted, and the checkout stays ready_for_complete

Both fail on main (200 instead of 400). Valid completion and the 409 out-of-stock behavior are unchanged and covered by the existing tests.

Validation

  • rest/nodejs: npm test, 177 passed
  • npm run build (tsc) clean

Follows the Python fix in #248.

@google-cla

google-cla Bot commented Nov 4, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@damaz91 damaz91 added the status:needs-triage Signal that the PR is ready for human triage label Nov 4, 2026

@damaz91 damaz91 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @arjun2075! Throwing InvalidRequestError inside runInTransaction cleanly rolls back earlier line-item stock reservations without persisting an order or poisoning idempotency_keys, bringing Node.js into full parity with #248. Both new lifecycle tests look great.

Once the commit author email / CLA check is resolved (see details on #273), this is ready to approve and merge!

Completion skipped the stock reservation for a line item whose product no
longer exists (`if (product && !reserveStock(...))`) and still created the
order, so a checkout could complete for a product that was removed from
the catalog, with nothing reserved.

Throw InvalidRequestError ("Product X not found", 400) inside the
completion transaction instead, matching the Python reference (Universal-Commerce-Protocol#248) and
the create/update paths. Earlier reservations roll back and no order is
written. Add lifecycle tests for a single deleted product and for a
deleted product on a later line.
@arjun2075
arjun2075 force-pushed the fix/nodejs-deleted-product-completion branch from 224bbd7 to 68316bf Compare November 11, 2026 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:needs-triage Signal that the PR is ready for human triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants