Pass QR checkout errors through with their message and status - #1823
Conversation
The QR scan handler's direct-to-checkout path replaced every payment
error with a generic 500 rendering the "QR code expired or invalid"
page, hiding the real reason from the visitor. The underlying
runCheckoutFlow already distinguishes a provider validation refusal
(HTTP 400, with the provider's own message) from a null/failed session
(HTTP 500, "Failed to create payment session. Please try again."),
but the handler's onError callback discarded both the message and the
status.
Wire the callback to a new qrBookCheckoutErrorPage that renders the
provider's message at the status the flow supplies, with the same
fallback booking link the token-error page offers. A null session
result still surfaces as the generic 500 message — only the stale
"expired or invalid" wording changes for that case.
Regression test stubs createCheckoutSession to return null (expects
500 + the generic message) and to return an { error } refusal
(expects 400 + the refusal text).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughQR-book direct checkout failures now render provider-specific messages and statuses with a fallback booking link, supported by locale updates and tests. The remaining changes reformat or preserve behavior across E2E payment utilities. ChangesQR booking checkout errors
E2E payment utility updates
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant QRScanRequest
participant skipToCheckout
participant StripeProvider
participant qrBookCheckoutErrorPage
QRScanRequest->>skipToCheckout: process QR booking scan
skipToCheckout->>StripeProvider: create checkout session
StripeProvider-->>skipToCheckout: return session result or provider error
skipToCheckout->>qrBookCheckoutErrorPage: render message and status on failure
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/lib/server-qr-book.test.ts`:
- Around line 133-152: Update scanWithCheckoutResult to declare the bare
stub(...) results for getConfiguredProvider and createCheckoutSession with using
declarations. Remove the surrounding try/finally and manual restore calls, while
preserving the awaited scanRequest(listing) return behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ad23a2bb-48a8-4acc-a37f-d0b8832fde44
📒 Files selected for processing (4)
src/features/public/qr-book.tssrc/locales/en/public.jsonsrc/ui/templates/public/errors.tsxtest/lib/server-qr-book.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e-payments/src/log.ts`:
- Around line 3-8: Restore output behavior in e2e-payments/src/log.ts by making
log, step, warn, and fail emit their messages, preserving the _ts timestamp
helper. Update dumpServerLog in e2e-payments/src/main.ts to print the computed
_signal alongside the server-log details so the catch path reports failure
context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 845a2e3f-6510-43fd-83d4-9ed7d5327c33
📒 Files selected for processing (11)
e2e-payments/src/config.tse2e-payments/src/flow.tse2e-payments/src/log.tse2e-payments/src/main.tse2e-payments/src/notify.tse2e-payments/src/order-flow.tse2e-payments/src/providers/card.tse2e-payments/src/server.tse2e-payments/src/tunnel.tssrc/features/public/qr-book.tstest/lib/server-qr-book.test.ts
The earlier Biome formatting commit silently gutted the e2e payment harness: log/step/warn/fail became no-ops and dumpServerLog lost both console.error output lines, so a failed sandbox run produced no failure context. Restore both to their emitting baseline behavior. Unify the two QR-book error pages (token-error and checkout-error) through a single qrBookPage renderer so their body fragment exists once, clearing the jscpd clone the split had introduced. Switch scanWithCheckoutResult's bare stub() results to using declarations per the repo convention.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/lib/server-qr-book.test.ts (1)
366-378: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the checkout fallback link.
This regression test verifies the provider message and status but not the newly added
/ticket/<slug>link, so the user-visible fallback could regress unnoticed.Proposed test assertion
expect(response.status).toBe(400); - expect(await response.text()).toContain(refusal); + const body = await response.text(); + expect(body).toContain(refusal); + expect(body).toContain(`/ticket/${listing.slug}`);As per coding guidelines, tests must provide deterministic behavioral coverage for changed behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/lib/server-qr-book.test.ts` around lines 366 - 378, Extend the test “returns a provider validation refusal with its HTTP 400 status” to assert that the response body also contains the checkout fallback link using the listing’s generated slug and the expected /ticket/<slug> path. Preserve the existing status and provider-message assertions.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@test/lib/server-qr-book.test.ts`:
- Around line 366-378: Extend the test “returns a provider validation refusal
with its HTTP 400 status” to assert that the response body also contains the
checkout fallback link using the listing’s generated slug and the expected
/ticket/<slug> path. Preserve the existing status and provider-message
assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 076c3984-bab3-4250-8abc-096e5d040792
📒 Files selected for processing (3)
e2e-payments/src/main.tssrc/ui/templates/public/errors.tsxtest/lib/server-qr-book.test.ts
The formatting commit 242607e touched eight e2e-payments files that have nothing to do with the QR checkout error passthrough this PR is about. Two of those changes were outright regressions (log/step/warn/fail became no-ops; dumpServerLog lost its output lines); the rest were cosmetic reformatting outside this PR's scope. Restore every e2e-payments file to its origin/main state so the PR contains only the four QR files: the handler, the catalog keys, the error renderer, and the focused tests. The logger emits its messages again because the silencing edit no longer exists.
What changed
When a visitor scans a signed booking QR code that skips straight to payment, the payment processor can refuse the booking (for example, an invalid phone number) or fail to create the session. Until now the QR route replaced every such error with a generic HTTP 500 page reading "QR code expired or invalid" — hiding the real reason from the visitor.
The underlying payment flow (
runCheckoutFlow) already distinguishes the two cases and supplies both a message and an HTTP status:The QR handler's error callback discarded both the message and the status. This PR wires them through to a new
qrBookCheckoutErrorPagethat renders the real message at the real status, while still showing the same "Go to booking page" fallback link the token-error page offers.What stays the same
nullcheckout result still renders the generic HTTP 500 response (with the "Failed to create payment session. Please try again." message) — only the stale "expired or invalid" wording changes for that one case.Regression tests
Two focused tests were added to
test/lib/server-qr-book.test.ts:createCheckoutSessionto returnnull, asserts the response is HTTP 500 and contains "Failed to create payment session. Please try again."createCheckoutSessionto return{ error: refusal }, asserts the response is HTTP 400 and contains the refusal message.Files
src/features/public/qr-book.ts— passmsgandstatusfromrunCheckoutFlow's error callback into the new error page (three lines).src/ui/templates/public/errors.tsx— addqrBookCheckoutErrorPage; extract the shared fallback-link markup intoqrBookBookingLinkso the new page and the existing token-error page don't duplicate it.src/locales/en/public.json— two new keys for the checkout-error page title and heading.test/lib/server-qr-book.test.ts— the two regression tests above.Summary by CodeRabbit