[$500] Android - Settings - App crashes when changing device font size and then tapping on Profile icon

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.8-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1. Launch New Expensify app
2. Background the app
3. Change the device font size
4. Return to the app
5. Tap on the profile icon

Expected Result:

App does not crash

Actual Result:

App crashes

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6302273_1701804101481.Screen_Recording_20231206_002437_New_Expensify.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01977dd9f154104e64
• Upwork Job ID: 1732120049690255360
• Last Price Increase: 2023-12-05

[melvin-bot]

Triggered auto assignment to @bfitzexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01977dd9f154104e64

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @allroundexperts (External)

[bernhardoj]

Proposal

Please re-state the problem that we are trying to solve in this issue.

After changing the device font size, coming back to the app and open the Setting page, the app crashes.

What is the root cause of that problem?

The crash stack trace shows that the error comes from an undefined root state.

App/src/libs/Navigation/linkTo.ts

Lines 92 to 94 in ea6aac1

	} else if (action.payload.name === NAVIGATORS.RIGHT_MODAL_NAVIGATOR && rootState.routes.at(-1)?.name !== NAVIGATORS.RIGHT_MODAL_NAVIGATOR) {
	action.type = CONST.NAVIGATION.ACTION_TYPE.PUSH;
	}

After we change the font device, it's normal that the app is restarted, but somehow this makes the root state becomes undefined

What changes do you think we should make in order to solve the problem?

Safely access the root state.

-rootState.routes.at(-1)?.name
+rootState?.routes.at(-1)?.name

Just like we did with getTopmostReportId.

App/src/libs/Navigation/getTopmostReportId.ts

Lines 14 to 17 in f80fbe3

	function getTopmostReportId(state: NavigationState | NavigationState<RootStackParamList> | PartialState<NavigationState>): string | undefined {
	if (!state) {
	return;
	}

[allroundexperts]

@bernhardoj's proposal looks good!

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @iwiznia, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[0xmiros]

Seems like this was fixed already

[iwiznia]

It was? Where?

[allroundexperts]

I think @situchan fixed it yesterday #32525. But, it seems like he missed to change it in /src/libs/Navigation/Navigation.ts.

[suneox]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Android - Settings - App crashes when changing device font size and then tapping on Profile icon

What is the root cause of that problem?

App crashed at this line by rootState is undefined

The root cause is after changing the font-size config app will be re-render at NavigationRoot component after re-render navigationRef cannot call resetRoot at this line due to the value of dependency [isSmallScreenWidth, authenticated] doen't change so useEffect not call resetRoot function

What changes do you think we should make in order to solve the problem?

The func onReady callback at NavigationContainer just only pass to parent
So we will handle setIsReady state on NavigationRoot before calling onReady and then add isReady into dependency of useEffect to resetRoot

    ....
    const {isSmallScreenWidth} = useWindowDimensions();
+   const [isReady, setIsReady] = useState(false);
    ....

    useEffect(() => {
        if (!navigationRef.isReady() || !authenticated) {
            return;
        }
        // We need to force state rehydration so the CustomRouter can add the CentralPaneNavigator route if necessary.
        navigationRef.resetRoot(navigationRef.getRootState());
+   }, [isSmallScreenWidth, authenticated, isReady]);


    ....
      <NavigationContainer
      onStateChange={handleStateChange}
+     onReady={() => {
+         setIsReady(true);
+         onReady()
+     }}

Then when Navigation call linkTo we can get rootState of navigation instead of undefined

What alternative solutions did you explore? (Optional)

[situchan]

Ah, so the crash really happened on production app?

[situchan]

My fix was not fixing the root cause, but just to avoid crash. As seen in typing, rootState is not optional so should not be undefined

[situchan]

But, it seems like he missed to change it in /src/libs/Navigation/Navigation.ts.

It's redundant. cc: @blazejkustra

Are you still able to reproduce crash on latest main?

[iwiznia]

I think @situchan fixed it yesterday #32525. But, it seems like he missed to change it in /src/libs/Navigation/Navigation.ts.

Ah ok, given it's the same error, I think @situchan should fix it as part of that other issue, agree?

[allroundexperts]

Are you still able to reproduce crash on latest main?

Seems to be fixed on the latest main. I can also no longer reproduce this today. @suneox I see you had posted a proposal recently. Can you reproduce it?

[suneox]

@allroundexperts

Currently it not happens on latest main but the root cause doesn't resolve.

My fix was not fixing the root cause, but just to avoid crash. As seen in typing, rootState is not optional so should not be undefined

My proposal can be resolve the root cause

[melvin-bot]

@iwiznia, @allroundexperts, @bfitzexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[iwiznia]

@allroundexperts do you agree we should make the fix @suneox proposes?

[bfitzexpensify]

Bump @allroundexperts - do you think the fix suggested by @suneox is worthwhile, even though this particular bug is no longer happening?

[situchan]

I think we can close this as no longer crash happening. If any critical issue happens in the future with the same root cause, we can always reopen.

[suneox]

Let me check another places same with this issue I'll feedback on tomorrow

[melvin-bot]

@iwiznia @allroundexperts @bfitzexpensify this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[bfitzexpensify]

Any updated thoughts @suneox?

[suneox]

Any updated thoughts @suneox?

I have tested and could not found any issue related with the root cause for this issue

[bfitzexpensify]

Thanks @suneox. OK, let's close this one out then.