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

Bugfix FXIOS-10832 Possible workaround for FXIOS-10832 withCheckedContinuation crash in JumpBackInDataAdaptor #23678

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

ih-codes
Copy link
Collaborator

📜 Tickets

Jira ticket
Github issue

💡 Description

This is to fix the Sentry crash linked in the bug description related to JumpBackInDataAdaptor.

You'll notice that the Rust code is very... weird. It's strangely making a synchronous call asynchronous (completely unnecessary), and there's also a synchronous version already right underneath it...

By cleaning that up we can remove the withCheckedContinuation on line 128 of JumpBackInDataAdaptor where we were crashing, so this will hopefully fix that issue. 🤞

📝 Checklist

You have to check all boxes before merging

  • Filled in the above information (tickets numbers and description of your work)
  • Updated the PR name to follow our PR naming guidelines
  • Wrote unit tests and/or ensured the tests suite is passing
  • When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
  • If needed, I updated documentation / comments for complex code and public methods
  • If needed, added a backport comment (example @Mergifyio backport release/v120)

…or the hasAccount() call and cleaning up some weird code.
@ih-codes ih-codes requested a review from a team as a code owner December 10, 2024 22:27
@mobiletest-ci-bot
Copy link

Messages
📖 Project coverage: 33.57%
📖 Edited 5 files
📖 Created 0 files

Client.app: Coverage: 31.95

File Coverage
UpdateViewModel.swift 89.16%
BrowserViewController.swift 3.51% ⚠️
JumpBackInDataAdaptor.swift 97.37%
Profile.swift 22.27% ⚠️

CredentialProvider.appex: Coverage: 21.18

File Coverage
Profile.swift 22.27% ⚠️

NotificationService.appex: Coverage: 26.01

File Coverage
Profile.swift 22.27% ⚠️

ShareTo.appex: Coverage: 31.57

File Coverage
Profile.swift 22.27% ⚠️

libAccount.a: Coverage: 31.14

File Coverage
RustFirefoxAccounts.swift 15.48% ⚠️

Generated by 🚫 Danger Swift against 6ee5122

@ih-codes
Copy link
Collaborator Author

Please review carefully @adudenamedruby and @nbhasin2 for sanity's sake... I wasn't sure the test path for this code and it was very weird. 😕

@lmarceau lmarceau dismissed their stale review December 11, 2024 13:52

Use cases around jbi are ok!

Copy link
Contributor

@adudenamedruby adudenamedruby left a comment

Choose a reason for hiding this comment

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

We should make sure QA test this well, just to be sure.

@ih-codes
Copy link
Collaborator Author

We should make sure QA test this well, just to be sure.

I'm adding QA notes to the ticket for this!

@ih-codes ih-codes merged commit e4de9c8 into main Dec 11, 2024
10 checks passed
@ih-codes ih-codes deleted the ih/FXIOS-10832-fix-jump-adapter-continuation branch December 11, 2024 21:36
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.

5 participants