Cancel orders that are still unpaid after 30 minutes

Checkout takes stock, so abandoned payments held it for good. A cron trigger now cancels the PaymentIntent and then the order every five minutes, which returns the stock. An intent that already succeeded is left for the webhook.

Justin Torre committed 7 days ago

5c7d1551058ad244594cc92c1925acb2e1130c58

Parent c8458cc Browse files History

Changed files

13 files changed143 additions13 deletionsagainst c8458cc

Changed.rebasely/architecture/system.yaml+1−1
Changed.rebasely/kb/concepts/checkout.md+11−1
Changed.rebasely/kb/concepts/payment-webhook.md+2−2
Added.rebasely/kb/decisions/0009-cancel-unpaid-orders-after-30-minutes.md+22−0
Changed.rebasely/kb/guides/testing.md+2−2
Changed.rebasely/kb/index.md+1−0
Addedsrc/expiry.ts+30−0
Changedsrc/index.ts+8−0
Changedsrc/payments.ts+5−3
Addedtest/expiry.test.ts+52−0
Changedtest/helpers/payments.ts+6−2
Changedtest/stripe-payments.test.ts+2−2
Changedwrangler.jsonc+1−0

Changed.rebasely/architecture/system.yaml+1−1

View file

Changed.rebasely/kb/concepts/checkout.md+11−1

View file

Changed.rebasely/kb/concepts/payment-webhook.md+2−2

View file

Added.rebasely/kb/decisions/0009-cancel-unpaid-orders-after-30-minutes.md+22−0

View file

Changed.rebasely/kb/guides/testing.md+2−2

View file

Changed.rebasely/kb/index.md+1−0

View file

Addedsrc/expiry.ts+30−0

View file

Changedsrc/index.ts+8−0

View file

Changedsrc/payments.ts+5−3

View file

Addedtest/expiry.test.ts+52−0

View file

Changedtest/helpers/payments.ts+6−2

View file

Changedtest/stripe-payments.test.ts+2−2

View file

Changedwrangler.jsonc+1−0

View file

Checks

No checks reported for this commit.

Runs

No runs for this commit.

		@@ -5,7 +5,7 @@
		The public HTTP API (wrangler.jsonc, Worker storefront-api). Shoppers browse the catalog, fill
		carts and check out; the merchant edits products and uploads their images with the admin
		token. Its OrdersService entrypoint is how the other Workers change orders: it is the only
		- Worker with the database binding.
		+ Worker with the database binding. A cron trigger expires unpaid orders every five minutes.
		owns: [src/**]
		talks_to:
		- { to: db, via: d1 }

		@@ -7,8 +7,9 @@
		- src/checkout.ts
		- src/orders.ts
		- src/payments.ts
		+ - src/expiry.ts
		- migrations/0003_orders.sql
		-reviewed: 2026-10-08
		+reviewed: 2026-10-09
		---
		# Checkout

		@@ -29,6 +30,15 @@
		## Retries

		Checking out the same cart again while its order is waiting for payment returns that order and the same client secret: the intent is created with the idempotency key `order-<id>`, so Stripe returns the existing one. Once the order is paid or cancelled, the cart answers `409 cart_closed`.
		+
		+## Unpaid orders
		+
		+An order holds its stock until it is paid or cancelled, so unpaid ones expire ([[0009-cancel-unpaid-orders-after-30-minutes]]). Every five minutes the cron trigger runs `expireOrders` (`src/expiry.ts`) over up to 50 orders that have been `pending_payment` for more than 30 minutes:
		+
		+1. Cancel the PaymentIntent at Stripe. If Stripe refuses because the intent succeeded or a payment is processing, the payment won the race: leave the order to the webhook.
		+2. Otherwise cancel the order, which puts the stock back and sends `order.cancelled`.
		+
		+Cancelling the intent first means a shopper cannot pay for an order after it has been cancelled.

		## Order states

		@@ -5,7 +5,7 @@
		component: stripe-webhook
		describes:
		- workers/stripe-webhook/src/**
		-reviewed: 2026-10-05
		+reviewed: 2026-10-09
		---
		# The payment webhook

		@@ -31,7 +31,7 @@

		## A payment for a cancelled order

		-If an order was cancelled and its payment still succeeds, `markPaid` leaves the order cancelled and the Worker logs `payment for a cancelled order, refund it` as an error. That should not happen once unpaid orders cancel their intent first, but the log line is there to search for.
		+If an order was cancelled and its payment still succeeds, `markPaid` leaves the order cancelled and the Worker logs `payment for a cancelled order, refund it` as an error. Expiry cancels the intent before the order ([[checkout]]), so this needs an order cancelled some other way; the log line is there to search for if it happens.

		## Local testing

		@@ -0,0 +1,22 @@
		+---
		+id: 0009-cancel-unpaid-orders-after-30-minutes
		+type: decision
		+owner: justin
		+status: accepted
		+reviewed: 2026-10-09
		+---
		+# 0009 Cancel unpaid orders after 30 minutes
		+
		+## Context
		+
		+Checkout takes stock ([[0004-take-stock-at-checkout]]), and an order that is never paid keeps it. Shoppers abandon the payment step often: a card is declined, a bank's confirmation page times out, a tab is closed. Stripe leaves those PaymentIntents open indefinitely.
		+
		+## Decision
		+
		+Orders still `pending_payment` 30 minutes after checkout are cancelled by a cron trigger on storefront-api that runs every five minutes. The PaymentIntent is cancelled at Stripe first, and an order whose intent can no longer be cancelled is left for the webhook ([[payment-webhook]]).
		+
		+## Consequences
		+
		+- Stock held by an abandoned checkout comes back within 35 minutes.
		+- A shopper who comes back after the window has to check out again; the old client secret no longer works because the intent is cancelled.
		+- Bank redirects and 3D Secure fit comfortably in 30 minutes. Delayed payment methods (bank debits, which take days) would not. They are off in the Stripe dashboard, and turning one on needs a different rule.

		@@ -4,7 +4,7 @@
		owner: justin
		describes:
		- test/helpers/**
		-reviewed: 2026-10-08
		+reviewed: 2026-10-09
		---
		# Running and writing tests

		@@ -22,7 +22,7 @@
		- `DB` is `TestD1` (`test/helpers/d1.ts`), a small D1 stand-in over `node:sqlite` with every file in `migrations/` applied in order and foreign keys on. It covers `prepare`, `bind`, `first`, `all`, `run` and `batch`, and a batch is a transaction, as in D1.
		- `IMAGES` is `TestR2` (`test/helpers/r2.ts`), an in-memory bucket whose `objects` map tests can inspect, and `IMAGES_URL` is `https://images.shop.test`.
		- `ORDER_EVENTS` is `TestQueue` (`test/helpers/queue.ts`), which keeps every message in `sent`.
		-- Payments are `FakePayments` (`test/helpers/payments.ts`), which records every charge and fails the next one when `failNext` is set. `test/stripe-payments.test.ts` checks the real Stripe adapter against a fake `fetch`, so no test calls Stripe.
		+- Payments are `FakePayments` (`test/helpers/payments.ts`), which records every charge, fails the next one when `failNext` is set, and refuses to cancel the intents in `uncancellable`. `test/stripe-payments.test.ts` checks the real Stripe adapter against a fake `fetch`, so no test calls Stripe.
		- The clock is fixed and ids count up (`id-1`, `id-2`), so assertions can name them.

		Tests talk HTTP to the handler through `call(method, path, body, headers)`, which returns the status, headers and parsed JSON. `addProduct` creates a product with the admin token, `payments` is the fake the app was given and `events` is its queue.

		@@ -22,6 +22,7 @@
		- [[0006-one-writer-for-the-database]]: other Workers change orders through OrdersService.
		- [[0007-at-least-once-with-idempotency-keys]]: repeats are expected and harmless.
		- [[0008-images-from-the-bucket-domain]]: browsers load images from R2's custom domain.
		+- [[0009-cancel-unpaid-orders-after-30-minutes]]: unpaid orders give their stock back.

		## Guides

		@@ -0,0 +1,30 @@
		+// Unpaid orders hold stock (checkout takes it), so they cannot wait forever. Every five minutes
		+// the cron trigger cancels orders still pending_payment after PAYMENT_WINDOW_MS, cancelling the
		+// PaymentIntent first so a late payment cannot land on a cancelled order.
		+
		+import type { OrderEvent } from "./events.ts";
		+import { cancelOrder } from "./orders.ts";
		+import type { Payments } from "./payments.ts";
		+
		+export const PAYMENT_WINDOW_MS = 30 * 60 * 1000;
		+const BATCH = 50;
		+
		+export type ExpiryReport = { cancelled: string[]; left_to_webhook: string[] };
		+
		+export async function expireOrders(db: D1Database, events: Queue<OrderEvent>, payments: Payments, now: number): Promise<ExpiryReport> {
		+ const { results } = await db
		+ .prepare("SELECT id, payment_intent_id FROM orders WHERE status = 'pending_payment' AND created_at < ? ORDER BY created_at LIMIT ?")
		+ .bind(now - PAYMENT_WINDOW_MS, BATCH)
		+ .all<{ id: string; payment_intent_id: string \| null }>();
		+ const report: ExpiryReport = { cancelled: [], left_to_webhook: [] };
		+ for (const o of results) {
		+ // The intent succeeded or is processing: the payment won the race, and the webhook will mark
		+ // the order paid. Leave it alone.
		+ if (o.payment_intent_id && !(await payments.cancelIntent(o.payment_intent_id))) {
		+ report.left_to_webhook.push(o.id);
		+ continue;
		+ }
		+ if ((await cancelOrder(db, events, o.id, now)).changed) report.cancelled.push(o.id);
		+ }
		+ return report;
		+}

		@@ -1,7 +1,9 @@
		import { WorkerEntrypoint } from "cloudflare:workers";
		import { createApp } from "./app.ts";
		import type { Env } from "./env.ts";
		+import { expireOrders } from "./expiry.ts";
		import { cancelOrder, getOrder, markPaid, recordShipment, type Order, type Settled } from "./orders.ts";
		+import { stripePayments } from "./payments.ts";

		// The calls the shop's other Workers make through a service binding (entrypoint OrdersService).
		// storefront-api stays the only Worker that writes to the database; see
		@@ -27,5 +29,11 @@
		export default {
		fetch(req, env) {
		return createApp(env).fetch(req);
		+ },
		+
		+ // triggers.crons in wrangler.jsonc: every five minutes.
		+ async scheduled(controller, env) {
		+ const report = await expireOrders(env.DB, env.ORDER_EVENTS, stripePayments(env.STRIPE_SECRET_KEY), controller.scheduledTime);
		+ if (report.cancelled.length \|\| report.left_to_webhook.length) console.log(JSON.stringify({ msg: "expired unpaid orders", ...report }));
		},
		} satisfies ExportedHandler<Env>;

		@@ -10,7 +10,9 @@
		// Creates the PaymentIntent the shopper pays in the browser. Calling it again for the same
		// order returns the same intent.
		createIntent(charge: Charge): Promise<Intent>;
		- cancelIntent(intentId: string): Promise<void>;
		+ // True when the intent is cancelled, false when it can no longer be (it succeeded, or a
		+ // payment is processing). In that case the webhook settles the order.
		+ cancelIntent(intentId: string): Promise<boolean>;
		}

		export function stripePayments(secretKey: string, fetchFn?: typeof fetch): Payments {
		@@ -37,9 +39,9 @@
		async cancelIntent(intentId) {
		try {
		await stripe.paymentIntents.cancel(intentId);
		+ return true;
		} catch (e) {
		- // Already canceled, or paid in the meantime: the webhook settles the order either way.
		- if (e instanceof Stripe.errors.StripeInvalidRequestError && e.code === "payment_intent_unexpected_state") return;
		+ if (e instanceof Stripe.errors.StripeInvalidRequestError && e.code === "payment_intent_unexpected_state") return false;
		throw e;
		}
		},

		@@ -0,0 +1,52 @@
		+import assert from "node:assert/strict";
		+import { test } from "node:test";
		+import { expireOrders, PAYMENT_WINDOW_MS } from "../src/expiry.ts";
		+import { markPaid } from "../src/orders.ts";
		+import { NOW, setup } from "./helpers/app.ts";
		+
		+const CHECKOUT = { email: "ada@park.test", ship_to: { name: "Ada Park", line1: "12 Alder St", city: "Portland", postal_code: "97205", country: "US" } };
		+
		+async function pending() {
		+ const s = setup();
		+ await s.addProduct("wool-socks", { stock: 3 });
		+ const cartId = (await s.call("POST", "/carts")).body.cart.id;
		+ await s.call("PUT", `/carts/${cartId}/items/wool-socks`, { quantity: 2 });
		+ const order = (await s.call("POST", `/carts/${cartId}/checkout`, CHECKOUT)).body.order;
		+ const expire = (at: number) => expireOrders(s.env.DB, s.events, s.payments, at);
		+ const status = async () => (await s.call("GET", `/orders/${order.id}`)).body.order.status;
		+ const stock = async () => (await s.call("GET", "/products/wool-socks")).body.product.stock;
		+ return { ...s, order, expire, status, stock };
		+}
		+
		+test("an order unpaid after 30 minutes is cancelled with its intent, and the stock comes back", async () => {
		+ const s = await pending();
		+ const report = await s.expire(NOW + PAYMENT_WINDOW_MS + 1);
		+ assert.deepEqual(report, { cancelled: [s.order.id], left_to_webhook: [] });
		+ assert.deepEqual(s.payments.cancelled, [`pi_${s.order.id}`]);
		+ assert.equal(await s.status(), "cancelled");
		+ assert.equal(await s.stock(), 3);
		+ assert.deepEqual(s.events.sent.map((e) => e.type), ["order.cancelled"]);
		+});
		+
		+test("orders inside the payment window are left alone", async () => {
		+ const s = await pending();
		+ assert.deepEqual(await s.expire(NOW + PAYMENT_WINDOW_MS - 1), { cancelled: [], left_to_webhook: [] });
		+ assert.equal(await s.status(), "pending_payment");
		+});
		+
		+test("an intent that succeeded at the last moment is left for the webhook", async () => {
		+ const s = await pending();
		+ s.payments.uncancellable.add(`pi_${s.order.id}`);
		+ const report = await s.expire(NOW + PAYMENT_WINDOW_MS + 1);
		+ assert.deepEqual(report, { cancelled: [], left_to_webhook: [s.order.id] });
		+ assert.equal(await s.status(), "pending_payment");
		+ await markPaid(s.env.DB, s.events, s.order.id, `pi_${s.order.id}`, NOW);
		+ assert.equal(await s.status(), "paid");
		+});
		+
		+test("paid orders are never expired", async () => {
		+ const s = await pending();
		+ await markPaid(s.env.DB, s.events, s.order.id, `pi_${s.order.id}`, NOW);
		+ assert.deepEqual(await s.expire(NOW + 2 * PAYMENT_WINDOW_MS), { cancelled: [], left_to_webhook: [] });
		+ assert.deepEqual(s.payments.cancelled, []);
		+});

		@@ -1,11 +1,13 @@
		// A Payments stand-in that records what checkout asked for. Set `failNext` to make the next
		-// createIntent throw, as when the provider times out.
		+// createIntent throw, as when the provider times out, and add an intent to `uncancellable` to
		+// play one that was paid at the last moment.

		import type { Charge, Intent, Payments } from "../../src/payments.ts";

		export class FakePayments implements Payments {
		readonly charges: Charge[] = [];
		readonly cancelled: string[] = [];
		+ readonly uncancellable = new Set<string>(); // intents that already succeeded at Stripe
		failNext = false;

		async createIntent(c: Charge): Promise<Intent> {
		@@ -17,7 +19,9 @@
		return { id: `pi_${c.order_id}`, client_secret: `pi_${c.order_id}_secret` };
		}

		- async cancelIntent(intentId: string): Promise<void> {
		+ async cancelIntent(intentId: string): Promise<boolean> {
		+ if (this.uncancellable.has(intentId)) return false;
		this.cancelled.push(intentId);
		+ return true;
		}
		}

		@@ -30,11 +30,11 @@
		assert.equal(req!.body.get("automatic_payment_methods[enabled]"), "true");
		});

		-test("cancelling an intent that is no longer cancellable is not an error", async () => {
		+test("cancelling an intent that already succeeded answers false instead of throwing", async () => {
		const stripe = fakeStripe(() => ({
		status: 400,
		body: { error: { type: "invalid_request_error", code: "payment_intent_unexpected_state", message: "This PaymentIntent has already succeeded." } },
		}));
		- await stripePayments("sk_test_123", stripe.fetchFn).cancelIntent("pi_123");
		+ assert.equal(await stripePayments("sk_test_123", stripe.fetchFn).cancelIntent("pi_123"), false);
		assert.equal(new URL(stripe.seen[0]!.url).pathname, "/v1/payment_intents/pi_123/cancel");
		});

		@@ -17,6 +17,7 @@
		"r2_buckets": [{ "binding": "IMAGES", "bucket_name": "storefront-images" }],
		// storefront-images is public on this custom domain (R2 > Settings > Custom Domains).
		"vars": { "IMAGES_URL": "https://images.larkspur.shop" },
		+ "triggers": { "crons": ["/5 * * *"] },
		"queues": {
		"producers": [{ "binding": "ORDER_EVENTS", "queue": "order-events" }]
		}