From 8487c3bd364df0284f8bf0e082f7a0372618c06b Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Mon, 19 Oct 2026 20:53:34 +0200 Subject: [PATCH 1/2] fix: guard concurrent startPayment calls on Android and in the hook - Android: reject a second startPayment while one is in flight with "A payment is already in progress" instead of overwriting the stored promise. The pending slot is atomic because startPayment runs on the JS thread and onActivityResult on the main thread. - Hook: add a synchronous in-flight ref guard so two calls in the same frame start one native payment; reset() no longer clears isProcessing. - Share one PaymentHandler instance between the hook and the exported HybridPaymentHandler, so the native per-instance guard covers both. Closes #8 --- docs/docs/api/hybrid-payment-handler.md | 2 + docs/docs/api/use-payment-checkout.md | 3 +- package/android/build.gradle | 2 + .../java/com/margelo/nitro/pay/Constants.kt | 3 + .../margelo/nitro/pay/HybridPaymentHandler.kt | 17 ++- .../com/margelo/nitro/pay/PendingPayment.kt | 31 +++++ .../margelo/nitro/pay/PendingPaymentTest.kt | 88 ++++++++++++++ package/src/__tests__/paymentHandler.test.ts | 109 ++++++++++++++++++ .../usePaymentCheckout.integration.test.ts | 104 +++++++++++++++++ package/src/hooks/usePaymentCheckout.ts | 22 ++-- package/src/index.ts | 7 +- package/src/paymentHandler.ts | 24 ++++ 12 files changed, 392 insertions(+), 20 deletions(-) create mode 100644 package/android/src/main/java/com/margelo/nitro/pay/PendingPayment.kt create mode 100644 package/android/src/test/java/com/margelo/nitro/pay/PendingPaymentTest.kt create mode 100644 package/src/__tests__/paymentHandler.test.ts create mode 100644 package/src/paymentHandler.ts diff --git a/docs/docs/api/hybrid-payment-handler.md b/docs/docs/api/hybrid-payment-handler.md index 8cd583d..270dc10 100644 --- a/docs/docs/api/hybrid-payment-handler.md +++ b/docs/docs/api/hybrid-payment-handler.md @@ -55,6 +55,8 @@ On iOS, the merchant ID is resolved from the app entitlements by default. Use `a Build the request yourself or use [createPaymentRequest](/docs/api/utils#createpaymentrequestoptions) for a single line item with defaults. +Only one payment can be in flight at a time. A call made while another payment is still in progress, including one started by `usePaymentCheckout`, resolves immediately with `{ success: false, error: 'A payment is already in progress' }` and does not affect the payment in flight. This applies on iOS and Android. + --- ### canMakePayments(usingNetworks) diff --git a/docs/docs/api/use-payment-checkout.md b/docs/docs/api/use-payment-checkout.md index 647377c..9dc290a 100644 --- a/docs/docs/api/use-payment-checkout.md +++ b/docs/docs/api/use-payment-checkout.md @@ -68,7 +68,8 @@ interface UsePaymentCheckoutReturn { - Returns `null` and sets `error` when the cart is empty. - Returns `PaymentResult` (including `{ success: false, error }`) when the native flow completes with a failure result. - Returns `null` if an exception is thrown while starting/processing the payment. -- **reset()** — Clears `isProcessing`, `result`, and `error` (does not clear cart). + - Returns `{ success: false, error: 'A payment is already in progress' }` without opening a second sheet if a payment is still in flight, including one started through `HybridPaymentHandler`. The in-flight payment and the hook state are not affected. +- **reset()** — Clears `result` and `error` (does not clear cart). It does not clear `isProcessing` while a payment is in flight; `isProcessing` becomes `false` when that payment settles. ## Example diff --git a/package/android/build.gradle b/package/android/build.gradle index eab7b2f..faf2de6 100644 --- a/package/android/build.gradle +++ b/package/android/build.gradle @@ -147,5 +147,7 @@ dependencies { // Google Pay dependencies implementation 'com.google.android.gms:play-services-wallet:19.3.0' + + testImplementation 'junit:junit:4.13.2' } diff --git a/package/android/src/main/java/com/margelo/nitro/pay/Constants.kt b/package/android/src/main/java/com/margelo/nitro/pay/Constants.kt index c7e5dcf..df56ec9 100644 --- a/package/android/src/main/java/com/margelo/nitro/pay/Constants.kt +++ b/package/android/src/main/java/com/margelo/nitro/pay/Constants.kt @@ -41,6 +41,9 @@ object PaymentConstants { // Supported networks (lowercase for comparison) val SUPPORTED_NETWORKS = listOf("visa", "mastercard", "amex", "discover") + // Errors (keep in sync with iOS `ErrorMessage`) + const val ERROR_PAYMENT_IN_PROGRESS = "A payment is already in progress" + // Logging tags const val TAG_PAYMENT_HANDLER = "HybridPaymentHandler" const val TAG_GOOGLE_PAY_BUTTON = "HybridGooglePayButton" diff --git a/package/android/src/main/java/com/margelo/nitro/pay/HybridPaymentHandler.kt b/package/android/src/main/java/com/margelo/nitro/pay/HybridPaymentHandler.kt index 8956a28..8a6b593 100644 --- a/package/android/src/main/java/com/margelo/nitro/pay/HybridPaymentHandler.kt +++ b/package/android/src/main/java/com/margelo/nitro/pay/HybridPaymentHandler.kt @@ -27,7 +27,7 @@ class HybridPaymentHandler : HybridPaymentHandlerSpec(), ActivityEventListener { ?: throw IllegalStateException("Application context is not ReactApplicationContext") } - private var paymentPromise: Promise? = null + private val pendingPayment = PendingPayment() private var currentEnvironment: Int = WalletConstants.ENVIRONMENT_TEST init { @@ -71,7 +71,14 @@ class HybridPaymentHandler : HybridPaymentHandlerSpec(), ActivityEventListener { override fun startPayment(request: PaymentRequest): Promise { val promise = Promise() - paymentPromise = promise + + // Reject overlaps without touching the in-flight payment, so its + // promise still settles and no second Google Pay sheet is requested. + if (!pendingPayment.tryBegin { result -> promise.resolve(result) }) { + Log.w(PaymentConstants.TAG_PAYMENT_HANDLER, PaymentConstants.ERROR_PAYMENT_IN_PROGRESS) + promise.resolve(PaymentMapper.createErrorResult(PaymentConstants.ERROR_PAYMENT_IN_PROGRESS)) + return promise + } try { val environment = determineEnvironment(request.googlePayEnvironment) @@ -183,14 +190,12 @@ class HybridPaymentHandler : HybridPaymentHandlerSpec(), ActivityEventListener { private fun handlePaymentSuccess(paymentData: PaymentData) { Log.d(PaymentConstants.TAG_PAYMENT_HANDLER, "Payment successful") val result = PaymentMapper.mapPaymentDataToResult(paymentData) - paymentPromise?.resolve(result) - paymentPromise = null + pendingPayment.settle(result) } private fun handlePaymentError(errorMessage: String) { val result = PaymentMapper.createErrorResult(errorMessage) - paymentPromise?.resolve(result) - paymentPromise = null + pendingPayment.settle(result) } private fun logPaymentRequest(request: org.json.JSONObject, environment: Int) { diff --git a/package/android/src/main/java/com/margelo/nitro/pay/PendingPayment.kt b/package/android/src/main/java/com/margelo/nitro/pay/PendingPayment.kt new file mode 100644 index 0000000..adfca9b --- /dev/null +++ b/package/android/src/main/java/com/margelo/nitro/pay/PendingPayment.kt @@ -0,0 +1,31 @@ +package com.margelo.nitro.pay + +import java.util.concurrent.atomic.AtomicReference + +/** + * Holds the settle callback of the single in-flight payment. + * + * `startPayment` runs on the JS thread while `onActivityResult` runs on the + * main thread, so claiming and settling go through one atomic slot. + */ +internal class PendingPayment { + private val settleCallback = AtomicReference<((T) -> Unit)?>(null) + + /** + * Claims the slot for a new payment. + * Returns `false` without touching the in-flight payment if one exists. + */ + fun tryBegin(settle: (T) -> Unit): Boolean { + return settleCallback.compareAndSet(null, settle) + } + + /** + * Settles the in-flight payment once and frees the slot. + * Returns `false` if there was nothing to settle. + */ + fun settle(result: T): Boolean { + val settle = settleCallback.getAndSet(null) ?: return false + settle(result) + return true + } +} diff --git a/package/android/src/test/java/com/margelo/nitro/pay/PendingPaymentTest.kt b/package/android/src/test/java/com/margelo/nitro/pay/PendingPaymentTest.kt new file mode 100644 index 0000000..5ccb022 --- /dev/null +++ b/package/android/src/test/java/com/margelo/nitro/pay/PendingPaymentTest.kt @@ -0,0 +1,88 @@ +package com.margelo.nitro.pay + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import java.util.concurrent.CountDownLatch +import java.util.concurrent.atomic.AtomicInteger + +class PendingPaymentTest { + + @Test + fun settlesTheClaimedPayment() { + val pending = PendingPayment() + val settled = mutableListOf() + + assertTrue(pending.tryBegin { settled.add(it) }) + assertTrue(pending.settle("done")) + + assertEquals(listOf("done"), settled) + } + + @Test + fun rejectsOverlapWithoutReplacingTheInFlightPayment() { + val pending = PendingPayment() + val first = mutableListOf() + val second = mutableListOf() + + assertTrue(pending.tryBegin { first.add(it) }) + assertFalse(pending.tryBegin { second.add(it) }) + pending.settle("done") + + assertEquals(listOf("done"), first) + assertEquals(emptyList(), second) + } + + @Test + fun settlesOnlyOnce() { + val pending = PendingPayment() + val settled = mutableListOf() + + pending.tryBegin { settled.add(it) } + + assertTrue(pending.settle("first")) + assertFalse(pending.settle("second")) + assertEquals(listOf("first"), settled) + } + + @Test + fun settleWithoutPaymentIsNoOp() { + val pending = PendingPayment() + + assertFalse(pending.settle("orphan")) + } + + @Test + fun acceptsNewPaymentAfterSettle() { + val pending = PendingPayment() + val settled = mutableListOf() + + pending.tryBegin { settled.add("a:$it") } + pending.settle("1") + + assertTrue(pending.tryBegin { settled.add("b:$it") }) + pending.settle("2") + + assertEquals(listOf("a:1", "b:2"), settled) + } + + @Test + fun concurrentBeginsClaimTheSlotOnce() { + val pending = PendingPayment() + val threads = 16 + val gate = CountDownLatch(1) + val claims = AtomicInteger(0) + + val workers = (0 until threads).map { + Thread { + gate.await() + if (pending.tryBegin { }) claims.incrementAndGet() + }.apply { start() } + } + gate.countDown() + workers.forEach { it.join() } + + assertEquals(1, claims.get()) + } +} diff --git a/package/src/__tests__/paymentHandler.test.ts b/package/src/__tests__/paymentHandler.test.ts new file mode 100644 index 0000000..f015883 --- /dev/null +++ b/package/src/__tests__/paymentHandler.test.ts @@ -0,0 +1,109 @@ +import { act, renderHook } from '@testing-library/react-native' + +const IN_PROGRESS_RESULT = { + success: false, + error: 'A payment is already in progress', +} + +const mockInstances: object[] = [] +const mockPresentations: Array<(result: any) => void> = [] + +jest.mock('react-native-nitro-modules', () => ({ + NitroModules: { + createHybridObject: () => { + // Mirrors the per-instance in-progress guard of the native handlers. + let inFlight = false + const instance = { + payServiceStatus: () => ({ + canMakePayments: true, + canSetupCards: true, + }), + startPayment: () => { + if (inFlight) { + return Promise.resolve({ + success: false, + error: 'A payment is already in progress', + }) + } + inFlight = true + return new Promise((resolve) => { + mockPresentations.push((result) => { + inFlight = false + resolve(result) + }) + }) + }, + } + mockInstances.push(instance) + return instance + }, + }, + getHostComponent: () => 'HostComponent', +})) + +// Required after the mock state above exists: importing the package creates +// the handler immediately. +const { HybridPaymentHandler, usePaymentCheckout, createPaymentRequest } = + require('../index') as typeof import('../index') + +const settlePresentations = () => + mockPresentations.splice(0).forEach((settle) => settle({ success: true })) + +describe('shared PaymentHandler instance', () => { + const directRequest = createPaymentRequest({ amount: 10, label: 'Direct' }) + + const renderCheckoutWithItem = () => { + const rendered = renderHook(() => usePaymentCheckout({})) + act(() => { + rendered.result.current.addItem('Order', 5) + }) + return rendered + } + + afterEach(settlePresentations) + + it('uses one native handler for the hook and HybridPaymentHandler', () => { + renderHook(() => usePaymentCheckout({})) + + expect(mockInstances).toHaveLength(1) + expect(mockInstances[0]).toBe(HybridPaymentHandler) + }) + + it('rejects a direct call while the hook payment is in flight', async () => { + const { result } = renderCheckoutWithItem() + + let hookPayment!: Promise + let directResult: any + await act(async () => { + hookPayment = result.current.startPayment() + directResult = await HybridPaymentHandler.startPayment(directRequest) + }) + + expect(mockPresentations).toHaveLength(1) + expect(directResult).toEqual(IN_PROGRESS_RESULT) + + await act(async () => { + settlePresentations() + await hookPayment + }) + + await expect(hookPayment).resolves.toEqual({ success: true }) + }) + + it('rejects a hook call while a direct payment is in flight', async () => { + const { result } = renderCheckoutWithItem() + + const directPayment = HybridPaymentHandler.startPayment(directRequest) + let hookResult: any + await act(async () => { + hookResult = await result.current.startPayment() + }) + + expect(mockPresentations).toHaveLength(1) + expect(hookResult).toEqual(IN_PROGRESS_RESULT) + expect(result.current.error?.message).toBe(IN_PROGRESS_RESULT.error) + + settlePresentations() + await expect(directPayment).resolves.toEqual({ success: true }) + }) +}) diff --git a/package/src/hooks/__tests__/usePaymentCheckout.integration.test.ts b/package/src/hooks/__tests__/usePaymentCheckout.integration.test.ts index aae2439..be72e0d 100644 --- a/package/src/hooks/__tests__/usePaymentCheckout.integration.test.ts +++ b/package/src/hooks/__tests__/usePaymentCheckout.integration.test.ts @@ -14,6 +14,19 @@ jest.mock('react-native-nitro-modules', () => ({ import { usePaymentCheckout } from '../usePaymentCheckout' +const IN_PROGRESS_RESULT = { + success: false, + error: 'A payment is already in progress', +} + +function createDeferred() { + let resolve!: (value: T) => void + const promise = new Promise((res) => { + resolve = res + }) + return { promise, resolve } +} + describe('usePaymentCheckout integration', () => { beforeEach(() => { mockPayServiceStatus.mockReturnValue({ @@ -245,4 +258,95 @@ describe('usePaymentCheckout integration', () => { expect(result.current.result).toBeNull() expect(result.current.isProcessing).toBe(false) }) + + it('starts one native payment when startPayment is called twice synchronously', async () => { + const native = createDeferred() + mockStartPayment.mockReturnValueOnce(native.promise) + const { result } = renderCheckout() + + act(() => { + result.current.addItem('Order', 5) + }) + + let first!: Promise + let second: any + await act(async () => { + first = result.current.startPayment() + second = await result.current.startPayment() + }) + + expect(mockStartPayment).toHaveBeenCalledTimes(1) + expect(second).toEqual(IN_PROGRESS_RESULT) + expect(result.current.isProcessing).toBe(true) + expect(result.current.error).toBeNull() + expect(result.current.result).toBeNull() + + const nativeSuccess = { success: true, transactionId: 'tx_1' } + await act(async () => { + native.resolve(nativeSuccess) + await first + }) + + await expect(first).resolves.toEqual(nativeSuccess) + expect(result.current.result).toEqual(nativeSuccess) + expect(result.current.isProcessing).toBe(false) + }) + + it('keeps the in-flight guard when reset() runs during a payment', async () => { + const native = createDeferred() + mockStartPayment.mockReturnValueOnce(native.promise) + const { result } = renderCheckout() + + act(() => { + result.current.addItem('Order', 5) + }) + + let first!: Promise + act(() => { + first = result.current.startPayment() + }) + act(() => { + result.current.reset() + }) + + expect(result.current.isProcessing).toBe(true) + + let second: any + await act(async () => { + second = await result.current.startPayment() + }) + + expect(second).toEqual(IN_PROGRESS_RESULT) + expect(mockStartPayment).toHaveBeenCalledTimes(1) + + await act(async () => { + native.resolve({ success: true }) + await first + }) + + expect(result.current.isProcessing).toBe(false) + }) + + it('accepts a new payment once the previous one settles', async () => { + mockStartPayment + .mockRejectedValueOnce(new Error('Native failure')) + .mockResolvedValueOnce({ success: true }) + const { result } = renderCheckout() + + act(() => { + result.current.addItem('Order', 5) + }) + + await act(async () => { + await result.current.startPayment() + }) + + let paymentResult: any = null + await act(async () => { + paymentResult = await result.current.startPayment() + }) + + expect(mockStartPayment).toHaveBeenCalledTimes(2) + expect(paymentResult).toEqual({ success: true }) + }) }) diff --git a/package/src/hooks/usePaymentCheckout.ts b/package/src/hooks/usePaymentCheckout.ts index 8e199f4..f3d3995 100644 --- a/package/src/hooks/usePaymentCheckout.ts +++ b/package/src/hooks/usePaymentCheckout.ts @@ -1,9 +1,5 @@ -import { useState, useCallback, useMemo, useEffect } from 'react' -import { NitroModules } from 'react-native-nitro-modules' -import type { PaymentHandler } from '../specs/PaymentHandler.nitro' - -const HybridPaymentHandler = - NitroModules.createHybridObject('PaymentHandler') +import { useState, useCallback, useMemo, useEffect, useRef } from 'react' +import { getPaymentHandler, PAYMENT_IN_PROGRESS_ERROR } from '../paymentHandler' import type { PaymentRequest, PaymentResult, @@ -183,9 +179,12 @@ export function usePaymentCheckout( const [result, setResult] = useState(null) const [error, setError] = useState(null) + // Set synchronously, so two calls in the same frame still see each other. + const isPaymentInFlight = useRef(false) + useEffect(() => { try { - const paymentStatus = HybridPaymentHandler.payServiceStatus() + const paymentStatus = getPaymentHandler().payServiceStatus() setStatus(paymentStatus) } catch (err) { console.error('Failed to check payment status:', err) @@ -270,19 +269,24 @@ export function usePaymentCheckout( }, []) const startPayment = useCallback(async (): Promise => { + if (isPaymentInFlight.current) { + return { success: false, error: PAYMENT_IN_PROGRESS_ERROR } + } + if (items.length === 0) { const emptyCartError = new Error('Cart is empty') setError(emptyCartError) return null } + isPaymentInFlight.current = true setIsProcessing(true) setResult(null) setError(null) try { const paymentResult = - await HybridPaymentHandler.startPayment(paymentRequest) + await getPaymentHandler().startPayment(paymentRequest) setResult(paymentResult) @@ -297,12 +301,12 @@ export function usePaymentCheckout( setError(paymentError) return null } finally { + isPaymentInFlight.current = false setIsProcessing(false) } }, [items, paymentRequest]) const reset = useCallback(() => { - setIsProcessing(false) setResult(null) setError(null) }, []) diff --git a/package/src/index.ts b/package/src/index.ts index 2f9438a..36c91b9 100644 --- a/package/src/index.ts +++ b/package/src/index.ts @@ -1,4 +1,4 @@ -import { NitroModules, getHostComponent } from 'react-native-nitro-modules' +import { getHostComponent } from 'react-native-nitro-modules' import type { ApplePayButtonProps, ApplePayButtonMethods, @@ -7,10 +7,9 @@ import type { GooglePayButtonProps, GooglePayButtonMethods, } from './specs/GooglePayButton.nitro' -import type { PaymentHandler } from './specs/PaymentHandler.nitro' +import { getPaymentHandler } from './paymentHandler' -export const HybridPaymentHandler = - NitroModules.createHybridObject('PaymentHandler') +export const HybridPaymentHandler = getPaymentHandler() export type { PaymentHandler } from './specs/PaymentHandler.nitro' diff --git a/package/src/paymentHandler.ts b/package/src/paymentHandler.ts new file mode 100644 index 0000000..bbd8df7 --- /dev/null +++ b/package/src/paymentHandler.ts @@ -0,0 +1,24 @@ +import { NitroModules } from 'react-native-nitro-modules' +import type { PaymentHandler } from './specs/PaymentHandler.nitro' + +/** + * Error returned when `startPayment` is called while another payment is + * still in flight. Matches the message used by the native handlers. + */ +export const PAYMENT_IN_PROGRESS_ERROR = 'A payment is already in progress' + +let paymentHandler: PaymentHandler | undefined + +/** + * Returns the package-wide `PaymentHandler` instance. + * + * The native in-progress guard lives on the instance, so the hook and the + * exported `HybridPaymentHandler` must share one. + */ +export function getPaymentHandler(): PaymentHandler { + if (paymentHandler === undefined) { + paymentHandler = + NitroModules.createHybridObject('PaymentHandler') + } + return paymentHandler +} From 077c1b8a5aab27bb0b087a8921ee79cc9104738c Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Mon, 19 Oct 2026 21:15:22 +0200 Subject: [PATCH 2/2] docs: clarify how the hook reports a payment started elsewhere The hook ignores overlaps with its own in-flight payment without touching state. A payment started through HybridPaymentHandler or another hook instance is rejected natively, and the hook records that like any failed payment. Spell this out instead of claiming hook state is never affected. --- docs/docs/api/use-payment-checkout.md | 4 +++- package/src/__tests__/paymentHandler.test.ts | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/docs/api/use-payment-checkout.md b/docs/docs/api/use-payment-checkout.md index 9dc290a..58a035a 100644 --- a/docs/docs/api/use-payment-checkout.md +++ b/docs/docs/api/use-payment-checkout.md @@ -68,7 +68,9 @@ interface UsePaymentCheckoutReturn { - Returns `null` and sets `error` when the cart is empty. - Returns `PaymentResult` (including `{ success: false, error }`) when the native flow completes with a failure result. - Returns `null` if an exception is thrown while starting/processing the payment. - - Returns `{ success: false, error: 'A payment is already in progress' }` without opening a second sheet if a payment is still in flight, including one started through `HybridPaymentHandler`. The in-flight payment and the hook state are not affected. + - Returns `{ success: false, error: 'A payment is already in progress' }` without opening a second sheet if a payment is still in flight: + - If the payment in flight was started by this hook, the hook state is not changed. + - If it was started elsewhere (`HybridPaymentHandler` or another `usePaymentCheckout`), the native handler rejects the call and the hook records it like any failed payment, in `result` and `error`. - **reset()** — Clears `result` and `error` (does not clear cart). It does not clear `isProcessing` while a payment is in flight; `isProcessing` becomes `false` when that payment settles. ## Example diff --git a/package/src/__tests__/paymentHandler.test.ts b/package/src/__tests__/paymentHandler.test.ts index f015883..15ec6ef 100644 --- a/package/src/__tests__/paymentHandler.test.ts +++ b/package/src/__tests__/paymentHandler.test.ts @@ -90,7 +90,7 @@ describe('shared PaymentHandler instance', () => { await expect(hookPayment).resolves.toEqual({ success: true }) }) - it('rejects a hook call while a direct payment is in flight', async () => { + it('records the native in-progress result when a direct payment is in flight', async () => { const { result } = renderCheckoutWithItem() const directPayment = HybridPaymentHandler.startPayment(directRequest) @@ -101,6 +101,7 @@ describe('shared PaymentHandler instance', () => { expect(mockPresentations).toHaveLength(1) expect(hookResult).toEqual(IN_PROGRESS_RESULT) + expect(result.current.result).toEqual(IN_PROGRESS_RESULT) expect(result.current.error?.message).toBe(IN_PROGRESS_RESULT.error) settlePresentations()