Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses the demo app’s checkout-api concurrent checkout race by preventing overselling during reserveStock, and by adjusting the concurrent checkout scenario to treat expected OutOfStock outcomes as non-fatal.
Changes:
- Add a post-delay “live inventory” availability guard before committing inventory updates.
- Return reservation metadata based on the committed inventory record version rather than the pre-delay snapshot.
- In concurrent mode, switch from
Promise.alltoPromise.allSettledand ignore expectedOutOfStock:failures while still surfacing unexpected failures.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| demo_app/src/inventory/reserve-stock.ts | Adds a post-delay availability check and returns committed (record) snapshot version/token to avoid stale snapshot metadata. |
| demo_app/src/checkout/submit-order.ts | Uses allSettled to allow concurrent scenario to succeed even when one request fails with OutOfStock, while rethrowing unexpected failures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Intentional bug: this delay makes the pre-check stale under concurrent checkout attempts. | ||
| await delay(25); | ||
|
|
||
| const record = getInventoryRecord(sku); | ||
|
|
||
| if (record.available < quantity) { |
There was a problem hiding this comment.
The inline comment above the artificial delay now claims this is an “Intentional bug”, but the newly added post-delay record.available check changes the behavior to prevent the stale pre-check from overselling. Please update the comment to reflect the current intent (e.g., that the delay simulates a stale pre-check and the post-delay guard is the actual fix), otherwise future readers may reintroduce the race when “fixing” the comment/code.
| const hardFailure = settled.find( | ||
| (result) => | ||
| result.status === "rejected" && | ||
| !(result.reason instanceof Error && result.reason.message.startsWith("OutOfStock:")) | ||
| ); |
There was a problem hiding this comment.
The OutOfStock detection here relies on parsing Error.message with startsWith("OutOfStock:"). This is brittle (message wording changes, wrapping, localization) and couples submit-order to reserve-stock’s string format. Prefer a typed error (e.g., class OutOfStockError extends Error { sku }) or an error code property so callers can branch reliably without string matching.
| const settled = await Promise.allSettled(requests.map((request) => processCheckoutWorker(request))); | ||
|
|
||
| const hardFailure = settled.find( | ||
| (result) => | ||
| result.status === "rejected" && | ||
| !(result.reason instanceof Error && result.reason.message.startsWith("OutOfStock:")) | ||
| ); | ||
|
|
||
| if (hardFailure && hardFailure.status === "rejected") { | ||
| throw hardFailure.reason; | ||
| } | ||
|
|
||
| return settled.flatMap((result) => (result.status === "fulfilled" ? [result.value] : [])); |
There was a problem hiding this comment.
runCheckoutRaceScenario now returns only fulfilled results in concurrent mode, which can make the returned array shorter than the requests array. Since this function is exported and used by the server/script, consider returning a structure that preserves per-request outcomes (e.g., { successes, failures } or an array of discriminated union results) so callers can distinguish “one succeeded, one OutOfStock” from other scenarios instead of silently dropping failures.
Summary
Resolve
checkout-race-conditionforcheckout-apiwith a validated ReplayX patch candidate.Changed Files
Validation
Rollback
Revert the live inventory guard and concurrent checkout settlement handling.