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

Fix navigation crash #2963

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

vargaat
Copy link
Contributor

@vargaat vargaat commented Nov 8, 2024

What happened?

  • There were two crashes related to a navigation controller being pushed into another navigation controller.
  • One crash was because of interactions between UIPrint* classes and based on the crash logs they no longer appear on iOS 18 so most probably this is an iOS level bug that got fixed.
  • The other variant was most probably from our code but I couldn't reproduce the error not even based on the routes the user visited.
  • To avoid such crashes I added a safeguard not to allow our nav controller to push another nav controller.
  • I refactored our error / breadcrumb reporting to Crashlytics. My main concern there was that developer related log and error functions were mixed with the regular usage statistics methods and it was hard to understand what goes to which service. I separated developer related debug helping reporting methods into a dedicated entity to make things more easy. There are many code changes but most of them just about moving code around and renaming.
  • I also changed when a new route is reported as a breadcrumb. Previously it was after presentation and in case the presentation crashed we lost the log. Now the route is logged before the actual routing takes place.

Checklist

  • Tested on phone
  • Tested on tablet

@inst-danger
Copy link
Contributor

inst-danger commented Nov 8, 2024

Warnings
⚠️ This pull request will not generate a release note.
⚠️ One or more files are below the minimum test coverage 50%

Affected Apps: Student, Teacher, Parent

MBL-18060

Coverage New % Master % Delta
Canvas iOS 91.42% 91.43% -0.01%
Core/Core/Assignments/AssignmentList/ViewModel/AssignmentListViewModel.swift 47.8% 47.8% 0%

Generated by 🚫 dangerJS against d699dde

Copy link
Contributor

@suhaibabsi-inst suhaibabsi-inst left a comment

Choose a reason for hiding this comment

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

Thanks @vargaat !

Code +1

P.S. I was not able to test this on Firebase for lack of permission to DebugView.
Will be working on fixing that issue.

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