Skip to content

fix: restore pending request before completing PayPalButton flow - #1748

Open
agedd wants to merge 6 commits into
mainfrom
fix-restore-pending-request-paypal
Open

agedd wants to merge 6 commits into
mainfrom
fix-restore-pending-request-paypal

Conversation

@agedd

@agedd agedd commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Summary of changes

  • Restore the stored pending request on the launcher before completing the flow in the XML PayPalButton.
  • Add RestorePendingRequest.kt with a small helper, and unit tests for it.

Why

PayPalLauncher uses Chrome Auth Tab for the web flow and only keeps the pending request in memory. When the
Activity is recreated while the browser is open (for example "Don't keep activities", or a low-memory device), the
new launcher has no pending request, so the result can't be completed and the button reports a cancel instead of
returning a nonce, even though the buyer finished PayPal. Restoring the stored request first fixes it, which is the
same step the SDK's demo screens already do.

Testing

  • Unit tests for the helper (nothing stored, restore succeeds, restore fails).
  • Emulator, "Don't keep activities" on: XML "UI Components" PayPal flow returns a nonce (previously
    "User did not complete payment flow").
  • Cancel from the browser still reports Cancel.

AI Usage

code generation and refactor

Checklist

  • Added a changelog entry
  • Tested and confirmed payment flows affected by this change are functioning as expected

Authors

@agedd

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@agedd
agedd requested review from a team and mzlangreder November 2, 2026 20:03
@agedd
agedd requested a review from a team as a code owner November 2, 2026 20:03
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities found.

Scanned Files

None

@agedd agedd mentioned this pull request Nov 2, 2026
3 of 7 tasks

@saralvasquez saralvasquez left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. I tested on emulator and physical device with "do not keep activities" and the PayPalButton flow works as expected. With green tests I think this is ready to go in

Comment on lines +16 to +23
): BrowserSwitchException? {
if (pendingRequestString.isEmpty()) return null
return try {
payPalLauncher.restorePendingRequest(pendingRequestString)
null
} catch (e: BrowserSwitchException) {
e
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function signature is a bit unusual being a nullable exception as the return type. Is it going to be re-used elsewhere? I think my preference would be to lift the try/catch to consumers, but then I don't know what utility this function would bring at all then either.

callback.onPayPalResult(PayPalResult.Failure(error))
setButtonReEnabled()
return
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should have tests to cover the behavior of the PayPalButton.kt too, since that's what we're altering the behavior of here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, added PayPalButtonUnitTest, using an internal constructor like VenmoButton. it covers restore running before handleReturnToApp, the restore-failure path, and the success, no result and failure results.

callback: PayPalTokenizeCallback
) {
restorePendingRequest(payPalLauncher, pendingRequest.pendingRequestString)?.let { error ->
callback.onPayPalResult(PayPalResult.Failure(error))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to emit any failure analytic event here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah good callout, we don’t need one here, since the error still reaches the merchant via PayPalResult.Failure and the other UI components don't emit their own failure events.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants