Call the completion handler on the main thread

[alobaili]

Issue

I don't have access to GitHub issues for this repo, but I have created an official support ticket with reference number 45249 by contacting Checkout's support email.

Proposed changes

This fixes a crash when using the Frames iOS SDK in a SwiftUI app by wrapping the returned UIViewController from PaymentFormFactory.buildViewController(configuration:style:completionHandler:) in a SwiftUI UIViewControllerRepresentable.

Test Steps

I created a small Xcode project to reproduce the issue (Xcode 16.2):
FramesCheckoutTestSwiftUI.zip

If you don't have Xcode 16.2 installed try setting the minimum iOS target to 18.0 or 18.1.

You can verify this issue is fixed by tapping the Pay button on the payment form and observing that no crash occurs and the completionHandler is running successfully.

Checklist

• Reviewers assigned
• I have performed a self-review of my code and manual testing
• Lint and unit tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if applicable)

Further comments

The crash that was occurring on line 185 of DefaultPaymentViewModel was EXC_BREAKPOINT and was occurring on a thread other than the main thread. For some reason this was not an issue when the client integrating this library uses UIKit. This issue only surfaces when attempting to integrate the library on a SwiftUI app. In both cases, I think it's incorrect to complete outside the main thread. So I added the fix.

I also digged a little deeper in the call stack trace and noticed that this issue might be fixed in CheckoutAPIService (by calling completion for CheckoutAPIService.callRiskSDK(tokenDetails:completion:) on the main thread, but since CheckoutAPIService is not directly related or responsible for UI, I think it doesn't need to run on the main thread.

I don't have enough experience with the source code to write a unit test to test for this specific scenario (make sure the completionHandler is always running on the main thread). If you can guide me or add to this pull request yourself, I would appreciate that!

[alobaili]

Hello @okhan-okbay-cko I would appreciate your review for this PR. It fixes a crash, so I think it will be important for other clients of this library.

[okhan-okbay-cko]

Hey @alobaili,

Thanks for raising this.

I wonder if the crash still happens if you don't create the additional Payment SPM package and rather delete it and move the files of it under FramesCheckoutTestSwiftUI folder?

Or, was there a specific reason for you to create it in the first place?

Looking forward to your reply,
Thanks,
Okhan

[alobaili]

Hi @okhan-okbay-cko,

Moving the code directly to the app target indeed prevents the crash from occurring. This is surprising.

There is an architectural requirement in our project to have the code inside of a SwiftPM module. In our project, logic related to payment (communicating with certain backend services, setup frames SDK logic, handling response) all of this must be in its own local SwiftPM package (in this example I call it Payment).

I don't know why the fact that adding the code in a local SwiftPM package causes the crash, this is interesting and is worth investigating IMO.

In the meantime, I think this PR fixes this scenario without affecting existing clients and is worth adding in a patch release.

What do you think?

[okhan-okbay-cko]

Hi @alobaili,

Yes, it's an interesting instance too see the crash happens for this specific use case.

I believe we shouldn't switch back to main thread inside the SDK and leave that decision to clients' discretion. Dispatching to main thread would need to occur right before an UI-altering code piece, in my personal opinion.

Unfortunately, I can't find a better recommendation for you to point the SPM to source the package off of your fork that has this code change inside.

We don't plan releasing a new version of Frames, so, you shouldn't get any conflicts in the future.

Thanks for understanding,
Okhan

[alobaili]

@okhan-okbay-cko The issue is that the crash occurs before the completion is called from the library side. Wrapping the code from the client side with DispatchQueue.main.async does not prevent the crash.

I still see value in releasing this as a patch or accepting PRs from the community that don't break existing tests even if you don't plan on releasing major or minor versions.

For now, I will consider using my fork. Thanks for the suggestion.

Regards,