fix: user gesture fix for trustpay applepay#1604
Conversation
🚫 Missing Linked IssueHi 👋 This pull request does not appear to be linked to any open issue yet. Linking your PR to an issue helps keep the project tidy and ensures the issue is closed automatically. ✔️ How to fix this
Once linked, this check will pass automatically on your next push or when you re-run the workflow. Thanks for helping maintainers! 🙌 |
XyneSpaces
left a comment
There was a problem hiding this comment.
Automated Review Summary
This PR fixes the user gesture issue for TrustPay Apple Pay by deferring the /confirm call until onvalidatemerchant. The approach is sound but has some implementation concerns.
🔴 Critical Issues
1. Race Condition in Message Passing
- File:
src/hyper-loader/ApplePayInterceptor.res - Issue: Deferring
/confirmuntilonvalidatemerchantcreates a timing window:- Iframe sends
APPLE_PAY_BUTTON_CLICKEDto parent - Parent creates
ApplePaySessionsynchronously onvalidatemerchantfires → parent messages iframe → iframe calls/confirm- Risk: If network latency causes
/confirmto complete afteronpaymentauthorizedfires, the payment token may be lost
- Iframe sends
- Fix: Add a session state machine to track
validating→confirmed→authorizedtransitions and queue theonpaymentauthorizedevent until confirmation completes.
🟡 Medium Priority Issues
2. Memory Leaks from Event Listeners
- File:
src/hyper-loader/ApplePayInterceptor.res(316 lines) - Issue: Event listeners for
messageevents from parent iframe may not be removed on:- Session abortion
- Component unmount
- Payment cancellation
- Fix: Ensure cleanup function is exposed and called by
Elements.res:
let cleanup = () => {
Window.removeEventListener("message", messageHandler)
session->abort()
}3. CSP Compliance
- Files:
src/hyper-loader/ApplePayInterceptor.res,src/hyper-loader/Elements.res - Issue:
postMessagecommunication between iframe and parent must use specifictargetOrigin. - Fix: Verify:
postMessagecalls use explicit origin (not*)- The SDK verifies
event.originmatches expectedHYPERSWITCH_CLIENT_URL
- Risk: Wildcard origins violate strict CSP policies merchants may have.
🟢 Low Priority Issues
4. Type Safety in ReScript
- File:
src/hyper-loader/ApplePayInterceptor.res - Issue: Verify proper typing for
ApplePaySessionconstructor arguments and message payloads. - Fix: Define explicit variant types:
type applePayMessage =
| ApplePayButtonClicked
| ValidateMerchant({validationURL: string})
| ConfirmRequest({paymentData: JSON.t})5. Timeout Handling
- File:
src/Utilities/ApplePayHelpers.res - Issue: If
/confirmtakes > 30 seconds,ApplePaySessionwill timeout. - Fix: Ensure error handling calls
session.abort()and propagatesUSER_GESTURE_TIMEOUTerror.
6. Feature Flag Gating & State Reset
- File:
src/Utilities/PaymentHelpers.res - Issue: The
isTrustpayInterceptorConfirmflag should:- Only be
truewhen connector is TrustPay AND Apple Pay flow is active - Reset to
falseafter payment completion/failure to prevent state leakage
- Only be
- Risk: Flag not being reset could affect non-TrustPay Apple Pay flows.
✅ Positive Patterns to Verify
| Aspect | Expected Implementation |
|---|---|
| Cross-browser | typeof window.ApplePaySession !== "undefined" check |
| Cleanup | session.oncancel handler removes all listeners |
| Error propagation | postMessage errors bubble to hyper.confirmPayment() rejection |
Summary
| Severity | Count | Issues |
|---|---|---|
| 🔴 Critical | 1 | Race condition in confirm/authorized sequence |
| 🟡 Medium | 2 | Memory leaks, CSP wildcard origins |
| 🟢 Low | 3 | Type safety, timeout handling, flag gating |
Key Recommendation: The fix correctly addresses the core user gesture issue, but ensure the async /confirm call doesn't race with onpaymentauthorized by implementing a promise queue or state gate.
Type of Change
Description
Added user gesture handler flow change
How did you test it?
Opened TrustPay ApplePay till the payment sheet is getting opened and confirm call is being made
Checklist
npm run re:build