-
Notifications
You must be signed in to change notification settings - Fork 0
fix(checkout): never confirm a pay-upfront order whose payment never started #224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,6 +31,47 @@ export default function CheckoutPage({ params }: { params: { slug: string } }) { | |
| const [isSubmitting, setIsSubmitting] = useState(false); | ||
| const [error, setError] = useState<string | null>(null); | ||
| const [orderResult, setOrderResult] = useState<OrderResult | null>(null); | ||
| // Set when a pay-upfront order was created but its checkout could not be | ||
| // minted. Drives the retry below, which re-mints against the EXISTING order | ||
| // rather than creating a second one. | ||
| const [pendingPaymentOrderId, setPendingPaymentOrderId] = useState< | ||
| string | null | ||
| >(null); | ||
| const [isRetryingPayment, setIsRetryingPayment] = useState(false); | ||
|
|
||
| const retryPayment = async () => { | ||
| if (!pendingPaymentOrderId) return; | ||
| setIsRetryingPayment(true); | ||
| setError(null); | ||
| try { | ||
| const res = await fetch( | ||
| `/api/store/${params.slug}/orders/${pendingPaymentOrderId}/pay`, | ||
| { method: "POST" }, | ||
| ); | ||
| const data = await res.json(); | ||
| if (!res.ok) throw new Error(data.error || "Could not start payment"); | ||
| if (data.payUrl) { | ||
| window.location.href = data.payUrl; | ||
| return; | ||
| } | ||
| if (data.paid) { | ||
| // Already settled (e.g. the first mint actually succeeded upstream). | ||
| setPendingPaymentOrderId(null); | ||
| setError(null); | ||
| clearCart(); | ||
| return; | ||
| } | ||
| throw new Error("Could not start payment"); | ||
| } catch (e) { | ||
| setError( | ||
| e instanceof Error | ||
| ? `${e.message}. Nothing has been charged.` | ||
| : "Could not start payment. Nothing has been charged.", | ||
| ); | ||
| } finally { | ||
| setIsRetryingPayment(false); | ||
| } | ||
| }; | ||
|
Comment on lines
+42
to
+74
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
fd -p 'orders/\[id\]|orders/\[orderId\]' nextjs_space/app/api/store
rg -n -A 20 'export (async )?function POST' nextjs_space/app/api/store/'[slug]'/orders/'[id]'/pay/route.ts 2>/dev/null
fd -e ts pay --full-path nextjs_space/app/api/storeRepository: AutomatosAI/budstack-saas Length of output: 802 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== checkout file outline =="
ast-grep outline nextjs_space/app/store/[slug]/checkout/page.tsx --view expanded || true
echo "== checkout lines 1-120 =="
sed -n '1,120p' nextjs_space/app/store/[slug]/checkout/page.tsx
echo "== checkout lines 200-320 =="
sed -n '200,320p' nextjs_space/app/store/[slug]/checkout/page.tsx
echo "== pay route =="
sed -n '1,260p' nextjs_space/app/api/store/[slug]/orders/[orderId]/pay/route.ts
echo "== payment status route (for contract context maybe) =="
sed -n '1,220p' nextjs_space/app/api/store/[slug]/orders/[orderId]/payment-status/route.ts
echo "== all references to data.paid / payUrl / pendingPaymentOrderId =="
rg -n "data\.paid|payUrl|pendingPaymentOrderId|setOrderResult|OrderResult|items\.length" nextjs_space/app nextjs_space/app/api -SRepository: AutomatosAI/budstack-saas Length of output: 19458 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== submit post/payment status relevant sections =="
sed -n '220,300p' nextjs_space/app/api/store/[slug]/orders/submit/route.ts
echo "== return page relevant state/contracts =="
sed -n '1,160p' nextjs_space/app/store/[slug]/payment/return/[orderId]/page.tsxRepository: AutomatosAI/budstack-saas Length of output: 9751 Show the success state when retry reports a settled order.
🤖 Prompt for AI Agents |
||
|
|
||
| // Saved address state | ||
| const [savedAddress, setSavedAddress] = useState<ShippingAddress | null>(null); | ||
|
|
@@ -144,6 +185,19 @@ export default function CheckoutPage({ params }: { params: { slug: string } }) { | |
| return; | ||
| } | ||
|
|
||
| // Pay-upfront store where the checkout could not be minted. The order | ||
| // exists but NOTHING has been paid, so the confirmation screen below | ||
| // would tell the customer they had ordered — and clearCart() would take | ||
| // their basket with it. That is exactly what happened on LekkerWeed on | ||
| // 2026-07-29. Keep the cart, say what happened, offer the retry. | ||
| if (data.order?.paymentStartFailed) { | ||
| setPendingPaymentOrderId(data.order.orderId ?? null); | ||
| setError( | ||
| "We couldn't start the payment for your order. Nothing has been charged — please try again.", | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| setOrderResult(data.order); | ||
| clearCart(); | ||
| } catch (err) { | ||
|
|
@@ -640,9 +694,26 @@ export default function CheckoutPage({ params }: { params: { slug: string } }) { | |
| )} | ||
|
|
||
| {error && ( | ||
| <div className="flex items-start gap-2 bg-red-50 border border-red-200 text-red-700 px-4 py-3 rounded-lg text-sm"> | ||
| <AlertCircle className="w-4 h-4 flex-shrink-0 mt-0.5" /> | ||
| <span>{error}</span> | ||
| <div className="flex flex-col gap-3 bg-red-50 border border-red-200 text-red-700 px-4 py-3 rounded-lg text-sm"> | ||
| <div className="flex items-start gap-2"> | ||
| <AlertCircle className="w-4 h-4 flex-shrink-0 mt-0.5" /> | ||
| <span>{error}</span> | ||
| </div> | ||
| {pendingPaymentOrderId && ( | ||
| <Button | ||
| type="button" | ||
| onClick={retryPayment} | ||
| disabled={isRetryingPayment} | ||
| className="w-full h-11 text-sm font-semibold text-white" | ||
| style={{ | ||
| backgroundColor: "hsl(var(--tenant-color-primary))", | ||
| }} | ||
| > | ||
| {isRetryingPayment | ||
| ? "Starting payment…" | ||
| : "Retry payment"} | ||
| </Button> | ||
| )} | ||
| </div> | ||
| )} | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| import { describe, expect, it, vi } from "vitest"; | ||
|
|
||
| /** | ||
| * A pay-upfront (DIRECT) order whose checkout could not be minted is a FAILED | ||
| * checkout, not a placed order. | ||
| * | ||
| * 2026-07-29, LekkerWeed: a production backend task exited mid-request, the ALB | ||
| * returned a 502 in 87ms, and the mint threw. The catch swallowed it, execution | ||
| * continued to log('SUCCESS'), and the storefront — seeing no payUrl — showed | ||
| * the order-confirmation screen AND called clearCart(). The customer was told | ||
| * her order was placed, lost her basket, and no payment was ever taken. Dr Green | ||
| * order ccdeea34 sat PENDING with stock committed. | ||
| * | ||
| * These tests pin the two halves of the contract: | ||
| * - transient mint failures are retried rather than stranding the order | ||
| * - an unminted DIRECT order reports paymentStartFailed so the storefront can | ||
| * tell the truth. On email-link stores the same shape is normal and the | ||
| * flag must stay false. | ||
| */ | ||
|
|
||
| // Mirrors the retry/report logic in | ||
| // app/api/store/[slug]/orders/submit/route.ts so the behaviour is pinned | ||
| // without standing up the full Next route (DB, Clerk, tenant resolution). | ||
| async function mintWithRetry( | ||
| mint: () => Promise<{ payUrl: string }>, | ||
| directPayEnabled: boolean, | ||
| sleep: (ms: number) => Promise<void> = async () => {}, | ||
| ): Promise<{ payUrl?: string; paymentStartFailed: boolean; attempts: number }> { | ||
| let payUrl: string | undefined; | ||
| let paymentStartFailed = false; | ||
| let attempts = 0; | ||
| if (directPayEnabled) { | ||
| const MINT_ATTEMPTS = 3; | ||
| for (let attempt = 1; attempt <= MINT_ATTEMPTS; attempt++) { | ||
| attempts = attempt; | ||
| try { | ||
| payUrl = (await mint()).payUrl; | ||
| break; | ||
| } catch { | ||
| if (attempt !== MINT_ATTEMPTS) { | ||
| await sleep(400 * attempt); | ||
| continue; | ||
| } | ||
| paymentStartFailed = true; | ||
| } | ||
| } | ||
| } | ||
| return { payUrl, paymentStartFailed, attempts }; | ||
| } | ||
|
|
||
| describe("direct-pay checkout — never report a placed order without payment", () => { | ||
| it("retries a transient mint failure and succeeds (the 502 case)", async () => { | ||
| const mint = vi | ||
| .fn() | ||
| .mockRejectedValueOnce(new Error("502 Bad Gateway")) | ||
| .mockResolvedValue({ payUrl: "https://pay/abc" }); | ||
|
|
||
| const r = await mintWithRetry(mint, true); | ||
|
|
||
| expect(r.payUrl).toBe("https://pay/abc"); | ||
| expect(r.paymentStartFailed).toBe(false); | ||
| expect(r.attempts).toBe(2); | ||
| }); | ||
|
|
||
| it("gives up after 3 attempts and flags paymentStartFailed", async () => { | ||
| const mint = vi.fn().mockRejectedValue(new Error("502 Bad Gateway")); | ||
|
|
||
| const r = await mintWithRetry(mint, true); | ||
|
|
||
| expect(r.payUrl).toBeUndefined(); | ||
| expect(r.paymentStartFailed).toBe(true); | ||
| expect(mint).toHaveBeenCalledTimes(3); | ||
| }); | ||
|
|
||
| it("does NOT flag email-link stores — an unminted order is correct there", async () => { | ||
| const mint = vi.fn(); | ||
|
|
||
| const r = await mintWithRetry(mint, false); | ||
|
|
||
| expect(mint).not.toHaveBeenCalled(); | ||
| expect(r.payUrl).toBeUndefined(); | ||
| expect(r.paymentStartFailed).toBe(false); | ||
| }); | ||
|
|
||
| it("a successful first attempt does not retry", async () => { | ||
| const mint = vi.fn().mockResolvedValue({ payUrl: "https://pay/xyz" }); | ||
|
|
||
| const r = await mintWithRetry(mint, true); | ||
|
|
||
| expect(r.attempts).toBe(1); | ||
| expect(r.paymentStartFailed).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe("storefront response handling", () => { | ||
| // Mirrors app/store/[slug]/checkout/page.tsx. | ||
| const decide = (order: { payUrl?: string; paymentStartFailed?: boolean }) => { | ||
| if (order.payUrl) return "redirect-to-pay"; | ||
| if (order.paymentStartFailed) return "show-error-keep-cart"; | ||
| return "show-confirmation-clear-cart"; | ||
| }; | ||
|
|
||
| it("redirects to payment when minted", () => { | ||
| expect(decide({ payUrl: "https://pay/abc" })).toBe("redirect-to-pay"); | ||
| }); | ||
|
|
||
| it("keeps the cart and shows an error when payment could not start", () => { | ||
| // Previously: 'show-confirmation-clear-cart' — the LekkerWeed bug. | ||
| expect(decide({ paymentStartFailed: true })).toBe("show-error-keep-cart"); | ||
| }); | ||
|
|
||
| it("still confirms an email-link order, which has no payUrl by design", () => { | ||
| expect(decide({ paymentStartFailed: false })).toBe( | ||
| "show-confirmation-clear-cart", | ||
| ); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate
payUrlbefore treating a mint attempt as successful.createDirectCheckout's own implementation notes Dr Green responses are "sometimes wrapped in{ data }, sometimes raw" — i.e. parsing ambiguity is a known risk. If a malformed/edge-case response resolves with an empty or missingpayUrl, this loop still treats it as success (break), leavingpaymentStartFailed = false. The storefront then falls through tosetOrderResult(...); clearCart()(checkout/page.tsxdata.order?.payUrlcheck fails,paymentStartFailedis falsy) — reproducing the exact incident this PR fixes: a placed-order confirmation and cleared cart with no way to pay.🐛 Proposed fix
const checkout = await createDirectCheckout({ ... }); - payUrl = checkout.payUrl; + if (!checkout.payUrl) { + throw new Error("createDirectCheckout returned no payUrl"); + } + payUrl = checkout.payUrl; log("DIRECT_CHECKOUT", { hasPayUrl: !!payUrl, expiresAt: checkout.expiresAt, attempt, }); break;Consider adding a corresponding test case in
direct-pay-no-phantom-order.test.ts(mint resolves with an emptypayUrl) alongside the existing retry/failure cases.📝 Committable suggestion
🤖 Prompt for AI Agents