-
Notifications
You must be signed in to change notification settings - Fork 657
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Keep reference to pending payment result #7704
Keep reference to pending payment result #7704
Conversation
67cfa84
to
a3799dc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments that could maybe improve things, but nothing blocking.
) | ||
} | ||
|
||
private fun handlePaymentSheetStateLoadFailure(error: Throwable) { | ||
val pendingResult = pendingPaymentResult |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to set pendingPaymentResult
to null
after this? Or do we just assume the view model will go away?
if (intent == null) { | ||
// We're recovering from process death. Wait for the pending payment result | ||
// to be handled after re-loading. | ||
pendingPaymentResult = launcherResult |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this live in the view model? Seems like we could model this to be opaque to the view model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will follow up with a better abstraction.
e8b88e8
Summary
This pull request makes a first improvement to how we handle process death during the auth flow, with more improvements such as full state restoration still to come.
In the case of process death, the
paymentLauncher
used inPaymentSheetViewModel
will return anInternalPaymentResult
upon process recreation. However, the ViewModel is not yet in the correct state to handle this, as it hasn’t reloaded theStripeIntent
. TherequireNotNull(stripeIntent.value)
will throw.With this change, we’re delaying the handling of the
InternalPaymentResult
until the intent has been loaded again. If the intent has been confirmed by the user, then loading the intent will fail with aPaymentIntentInTerminalState
exception. This is where we intercept: Instead of returning a fatal error to the caller, we handle the completed payment and return aPaymentResult.Completed
.Due to how
PaymentLauncher
works, this is unfortunately very difficult to test 😔Motivation
Resolves #7590
Testing
Screenshots
Changelog