[HOLD for payment 2022-12-05] [$500] Bug: DEV - [iOS] dev console error on app launch reported by @aimane-chnaif

[kavimuru]

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

---

Action Performed:

1. Open the app

Expected Result:

No console error

Actual Result:

Console error: Warning: RCTBridge required dispatch_sync to load REAModule. This may lead to deadlocks

Workaround:

unknown

Platform:

Where is this issue occurring?

• iOS

Version Number: v1.2.19-2
Reproducible in staging?: Needs reproduction
Reproducible in production?: Needs reproduction
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Expensify/Expensify Issue URL:
Issue reported by: @aimane-chnaif
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1666759616071549

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @MitchExpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[dhairyasenjaliya]

Proposal

• This warning is caused by the react-native-reanimated package below is the PR that recently they fixed in the package
  PR: https://github.com/software-mansion/react-native-reanimated/pull/3698/files

• But we are currently using 3.0.0-rc.3 so I think we need to add the below changes to the patch here https://github.com/Expensify/App/tree/main/patches

file : REAModule.mm


#ifdef RCT_NEW_ARCH_ENABLED
+ (BOOL)requiresMainQueueSetup
{
  return YES;
}
#endif // RCT_NEW_ARCH_ENABLED

Result fix

Issue

[MitchExpensify]

How do you pull up the console warning screen? Trying to reproduce

[melvin-bot]

@MitchExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

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

[MitchExpensify]

Hey @kavimuru, mind helping me with this question?: How do you pull up the console warning screen?

Thanks!

[kavimuru]

@MitchExpensify I can't check Dev only issue. So I added Needs reproduction label. @aimane-chnaif can clarify your question.

[aimane-chnaif]

@MitchExpensify were you not able to see this warning on iOS simulator?

https://user-images.githubusercontent.com/47522946/198195342-d6e6faca-0100-4cd7-b1e2-d63d39c17afe.png

[MitchExpensify]

@aimane-chnaif How exactly do I pull up the console warning screen?

[aimane-chnaif]

reproduce.step.mp4

cc: @MitchExpensify

[MitchExpensify]

Sorry, I'm asking how you pull up that console warning as I am not sure when trying to reproduce @aimane-chnaif

[michaelhaxhiu]

Hey @MitchExpensify, just dropping a note as a reminder to keep the pressure on finding a contributor and get this one closed out.

PS whatever you learn from this about the "console" question let's post in Stack Overflow for the future. I don't remember the answer myself (and I'm sure I asked before too 😨)

[michaelhaxhiu]

Just bumped the associated slack thread to make sure we have movement!

[melvin-bot]

@MitchExpensify Huh... This is 4 days overdue. Who can take care of this?

[MitchExpensify]

Asked for help reproducing in #contributor-plus

[melvin-bot]

@MitchExpensify Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.32-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [No QA] Update Reanimated Package + Remove patch  #12817

If no regressions arise, payment will be issued on 2022-12-05. 🎊

After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@eVoloshchak / @Luke9389] The PR that introduced the bug has been identified. Link to the PR: Fixed retain cycle of REAModule causing memory leak software-mansion/react-native-reanimated#3555
• [@eVoloshchak / @Luke9389] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@eVoloshchak / @Luke9389] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@MitchExpensify] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here:

[MitchExpensify]

Made a note to pay on 2022-12-05

[MitchExpensify]

@isagoico Regarding the addition of a regression test, I'm curious if you think a new section for console errors (I don't see one) or adding to the existing login tests makes more sense

[isagoico]

We could ask the testers that to execute the Web tests with the js console open to review if there are any JS errors in all tests. What do you think?

[MitchExpensify]

Asked in Slack here

[MitchExpensify]

Bumped

[MitchExpensify]

All payments made! Thanks everyone

[MitchExpensify]

@eVoloshchak / @Luke9389 - Curious which PR might have introduced this.

[MitchExpensify]

Not adding a regression test for console errors seeing as they should be caught in the PR checklists

[eVoloshchak]

@eVoloshchak / @Luke9389 - Curious which PR might have introduced this.

This bug was introduced by an external library (react-native-reanimated), judging by this comment, PR that caused this is software-mansion/react-native-reanimated#3555

[MitchExpensify]

Nice, thanks @eVoloshchak !

@eVoloshchak / @Luke9389 mind completing the other checks here and we can close this out once done?

[melvin-bot]

@eVoloshchak, @MitchExpensify, @dhairyasenjaliya, @Luke9389 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[MitchExpensify]

Bump on the checks @eVoloshchak & @Luke9389

[eVoloshchak]

The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:

This wasn't caused by PR in our codebase, so not applicable I guess

A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

We already have the following checklist item: I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed), this should have caught the warning

[MitchExpensify]

Thank you! If you agree @Luke9389 I think we can close this out

[MitchExpensify]

Actually, lets just reopen if you disagree @Luke9389 :)

I think we're good here and I'm heading ooo tomorrow so looking to square this off

[Luke9389]

Perfect thanks! I was OOO