A Bare Await Lost Receipts Forever on Webhook Retry

The short version

Awaiting a best-effort side effect bare, right after winning an idempotency claim, turns its throw into a 500 for the entire webhook handler, while Stripe’s redelivery skips the work because the core processing is already claimed — so the side effect alone never gets resent, permanently. The payment itself was recorded correctly on the first try, so nobody notices.

Symptom

The webhook claimed payment_status with a conditional UPDATE in markContractPaid, then called receipt issuance.

// persist.ts:34-45 (before the fix, at b8ca5e9)
  const { data: claimed } = await admin
    .from("contracts")
    .update(patch)
    .eq("id", contractId)
    .in("payment_status", ["未請求", "請求済"])
    .select("id")
    .maybeSingle();

  if (!claimed) return { claimed: false }; // already paid = idempotent no-op (don't resend the receipt)

  // System-issued = issuerId null. Won't break the payment record if it fails (best-effort).
  await issueReceiptAndEmail(admin, contractId, null);

The comment says “won’t break the payment record if it fails,” but the implementation is still a bare await. When issueReceiptAndEmail throws, the error propagates straight out of the function.

The caller — the webhook handler — is built to catch that exception in an outer try/catch and return a 500.

// route.ts:66-81
  try {
    const { contractId } = await handleStripeEvent(admin, stripe, event);
    await admin
      .from("stripe_events")
      .update({ processed_at: new Date().toISOString(), contract_id: contractId })
      .eq("id", event.id);
    return Response.json({ received: true });
  } catch (e) {
    // Delete the ledger row and return 500 so Stripe's redelivery reprocesses it (at-least-once).
    await admin.from("stripe_events").delete().eq("id", event.id);
    const msg = e instanceof Error ? e.message : "error";
    await notifySlack(
      `⚠️ Exception in Stripe webhook handling (${escapeSlack(event.type)}: ${escapeSlack(msg)}) → awaiting redelivery/manual check ${slackLink("/admin/contracts", "Contract")}`
    );
    return new Response(`error: ${msg}`, { status: 500 });
  }

The idempotency ledger row in stripe_events is deleted here. From Stripe’s point of view this event.id “hasn’t been processed yet,” and per at-least-once it gets redelivered.

But by the time the redelivery arrives, contracts.payment_status has already flipped to paid from the first call. markContractPaid’s UPDATE is conditioned on .in("payment_status", ["未請求", "請求済"]), so the redelivery no longer matches that condition, claimed comes back null, and if (!claimed) return { claimed: false }; fires the early return. The line that would call receipt issuance is never reached again.

At the ledger level (stripe_events), it looks “unclaimed.” At the business-data level (contracts.payment_status), it’s already “claimed.” The mismatch between these two claim states is the real mechanism — the redelivery trusts the former and never discovers the latter.

Why

The underlying intent — “don’t let the core processing break on failure” — was a correct design call. The core update (recording the payment) and the side effect (issuing a receipt) shouldn’t share the same failure severity. The problem was that this intent was never enforced in syntax. A bare await propagates the exception outward regardless of intent.

The outer catch is written on the premise that “the whole webhook handler failed,” so it deletes the ledger row and returns a 500 — the behavior appropriate when the core processing itself fails. But what actually failed here was only the side effect; the core processing (the payment_status update) had already committed in the preceding UPDATE statement. The outer try/catch has no way to distinguish “core processing succeeded, only the side effect failed” from an actual core failure.

Fixing it

We wrapped the call to issueReceiptAndEmail in try/catch, keeping the paid state intact and only logging on failure.

// persist.ts:42-53 (after the fix, at 56f097fb)
  if (!claimed) return { claimed: false }; // already paid = idempotent no-op (don't resend the receipt)

  // System-issued = issuerId null. Won't break the payment record if it fails (best-effort —
  // reviewer nit: a bare await means a throw 500s the webhook → redelivery loses the claim →
  // the receipt is never sent, permanently).
  try {
    await issueReceiptAndEmail(admin, contractId, null);
  } catch (e) {
    console.error("[stripe] receipt issue failed (paid state kept):", e);
  }

Now a receipt-issuance failure no longer propagates to the outer catch, and the webhook returns 200 and finishes normally. The core processing’s success and the side effect’s failure are separated at the syntax level.

The side-effect failure is logged via console.error and no longer gets automatically recovered via retry — which is the correct design here. Retry only helps when the work is “not yet claimed.” A side effect whose core processing has already completed, as in this case, is an asymmetric failure that retry can never fix in the first place.

Why wasn’t this caught

This same commit also fixed a separate, more severe misclassification: an INSERT failure into the idempotency ledger being swallowed as a “duplicate delivery” regardless of reason, which lost the core processing itself. Whether a “won’t break the payment record if it fails” comment is actually enforced in syntax (try/catch) has since become a fixed checkpoint for reviewing best-effort code like markContractPaid. Wherever a comment says “best-effort” or “won’t break on failure” but the implementation underneath is still a bare await remains something to check each time similar code is added or changed.