Skip to content
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

Bail to web-based Link when needed #4435

Merged
merged 19 commits into from
Jan 8, 2025
Merged

Conversation

davidme-stripe
Copy link
Contributor

@davidme-stripe davidme-stripe commented Dec 24, 2024

Summary

Add support for bailing back to the web Link flow when attestation fails. This makes launching Link more responsive for the expected case (that attestation will succeed if the app has attestation enabled and the device supports attestation), while allowing a graceful fallback in the unexpected case (the attestation or assertion fails). We'll bail if either:

  1. Initial attestation fails just after the sheet opens
  2. If any asserted network request fails

Motivation

Handle unexpected errors in a more user-friendly way.

Testing

Tested manually by intentionally breaking assertion and attestation in both cases.
Added a test that uses mocks to fail attestation and check that the ASWebAuthenticationSession is presented.

Changelog

None

Copy link

github-actions bot commented Dec 24, 2024

🚨 New dead code detected in this PR:

PayWithLinkViewController-SignUpViewController.swift:244 warning: Parameter 'attestionError' is unused
PayWithLinkViewController-SignUpViewModel.swift:23 warning: Parameter 'attestionError' is unused
PayWithNativeLinkController.swift:126 warning: Parameter 'payWithLinkWebController' is unused

Please remove the dead code before merging.

If this is intentional, you can bypass this check by adding the label skip dead code check to this PR.

ℹ️ If this comment appears to be left in error, double check that the flagged code is actually used and/or make sure your branch is up-to-date with master.

@davidme-stripe davidme-stripe force-pushed the davidme/fix-concurrent-assertions branch from 3bb4621 to 7df4065 Compare January 3, 2025 00:28
Base automatically changed from davidme/fix-concurrent-assertions to master January 3, 2025 01:16
@davidme-stripe davidme-stripe marked this pull request as ready for review January 6, 2025 22:33
@davidme-stripe davidme-stripe requested review from a team as code owners January 6, 2025 22:33
wooj-stripe
wooj-stripe previously approved these changes Jan 7, 2025
@davidme-stripe davidme-stripe merged commit 1122d94 into master Jan 8, 2025
7 checks passed
@davidme-stripe davidme-stripe deleted the davidme/link-bail-to-web branch January 8, 2025 00:07
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.

2 participants