IuliiaHerets commented 2 weeks ago

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: 9.0.61-0 Reproducible in staging?: Y Reproducible in production?: N If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y Email or phone of affected tester (no customers): gocemate+a2809@gmail.com Issue reported by: Applause Internal Team

Action Performed:

Login with gmail account
Go to FAB> Start chat> Add email address> Finish the flow
Go to FAB> Start chat

Expected Result:

App should not crash

Actual Result:

App crashes when create second new chat from FAB

Workaround:

Unknown

Platforms:

[x] Android: Standalone
[x] Android: HybridApp
[ ] Android: mWeb Chrome
[ ] iOS: Standalone
[ ] iOS: HybridApp
[ ] iOS: mWeb Safari
[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

1311.txt

https://github.com/user-attachments/assets/7522d213-88a9-428b-ae8c-1394d035b9b1

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~021856802999983156406
Upwork Job ID: 1856802999983156406
Last Price Increase: 2024-11-13
Automatic offers:

ahmedGaber93 | Reviewer | 104951617
QichenZhu | Contributor | 104951622

Issue Owner

Current Issue Owner: @

Issue Owner

Current Issue Owner: @VictoriaExpensify

melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @rlinoz (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @VictoriaExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

melvin-bot[bot] commented 2 weeks ago

💬 A slack conversation has been started in #expensify-open-source

github-actions[bot] commented 2 weeks ago

:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

Identify the pull request that introduced this issue and revert it.
Find someone who can quickly fix the issue.
Fix the issue yourself.

francoisl commented 2 weeks ago

So far I'm unable to repro on my physical device, let's look at the logs though.

rlinoz commented 2 weeks ago

yeah, me neither, tried straight from the staging branch and from the app on the store and nothing.

rlinoz commented 2 weeks ago

Ok it just happened to me, not sure how yet, so let's not remove the blocker label for now.

francoisl commented 2 weeks ago

Was it on HybridApp or standalone?

The crash file attached in the OP seems to be from HybridApp, but I'm also unable to repro there, and don't see any fresh crash reports in Firebase.

11-13 18:52:06.116 W/ActivityManager( 1781): Missing app error report, app = org.me.mobiexpensifyg crashing = false notResponding = true
11-13 18:52:06.176 E/ActivityManager( 1781): ANR in org.me.mobiexpensifyg (org.me.mobiexpensifyg/yapl.android.ReactNativeActivity)
11-13 18:52:06.176 E/ActivityManager( 1781): PID: 8936
11-13 18:52:06.176 E/ActivityManager( 1781): Reason: Input dispatching timed out (4f4765e org.me.mobiexpensifyg/yapl.android.ReactNativeActivity (server) is not responding. Waited 10004ms for MotionEvent)
11-13 18:52:06.176 E/ActivityManager( 1781): Parent: org.me.mobiexpensifyg/yapl.android.ReactNativeActivity
11-13 18:52:06.176 E/ActivityManager( 1781): Frozen: false
11-13 18:52:06.176 E/ActivityManager( 1781): Load: 5.37 / 5.55 / 6.35

11-13 18:52:06.181 D/ActivityManager( 1781): Completed ANR of org.me.mobiexpensifyg in 13576ms, latency 11ms
11-13 18:52:06.181 D/Debug   ( 1781): low && ship && 3rdparty app crash, do not dump
11-13 18:52:06.183 I/Dialog  ( 1781): mIsDeviceDefault = true, mIsSamsungBasicInteraction = false, isMetaDataInActivity = false
11-13 18:52:06.192 I/DecorView( 1781): [INFO] isPopOver=false, config=false
11-13 18:52:06.193 I/DecorView( 1781): updateCaptionType >> DecorView@3f7e4b4[mobiexpensifyg], isFloating=true, isApplication=false, hasWindowControllerCallback=false, hasWindowDecorCaption=false
11-13 18:52:06.193 D/DecorView( 1781): setCaptionType = 0, this = DecorView@3f7e4b4[mobiexpensifyg]
11-13 18:52:06.206 I/DropBoxManagerService( 1781): add tag=data_app_anr isTagEnabled=true flags=0x6
11-13 18:52:06.208 D/ScrollView( 1781): initGoToTop
11-13 18:52:06.213 D/ScrollView( 1781): initGoToTop
11-13 18:52:06.226 D/RestrictionPolicy( 1781): isGoogleCrashReportAllowed : ret=true userId =0
11-13 18:52:06.232 I/SurfaceFlinger( 1022): id=35590 createSurf (0x0),-1 flag=80004, WindowToken{e0005ae android.view.ViewRootImpl$W@66e9e29}#0
11-13 18:52:06.235 D/InputTransport( 1781): Input channel constructed: 'a519f4f', fd=625
11-13 18:52:06.235 D/InputTransport( 1781): Input channel constructed: 'a519f4f', fd=644
11-13 18:52:06.235 D/InputTransport( 1781): Input channel constructed: 'a519f4f', fd=649
11-13 18:52:06.236 I/SurfaceFlinger( 1022): id=35591 createSurf (0x0),-1 flag=80004, a519f4f Application Not Responding: org.me.mobiexpensifyg#0
11-13 18:52:06.241 V/WindowManager( 1781): Changing focus from Window{4f4765e u0 org.me.mobiexpensifyg/yapl.android.ReactNativeActivity} to Window{a519f4f u0 Application Not Responding: org.me.mobiexpensifyg} displayId=0 Callers=com.android.server.wm.RootWindowContainer.updateFocusedWindowLocked:583 com.android.server.wm.WindowManagerService.updateFocusedWindowLocked:6497 com.android.server.wm.WindowManagerService.addWindow:2050 com.android.server.wm.Session.addToDisplayAsUser:211

rlinoz commented 2 weeks ago

standalone built from the staging branch on a physical device

rlinoz commented 2 weeks ago

Yeah this is hard to repro, I feel like there is some kind of race condition happening

rlinoz commented 2 weeks ago

Steps:

1 - Create button > Start chat 2 - Go back using the gesture to go back (the back button on the device probably also works) 3 - Repeat until it freezes

rlinoz commented 2 weeks ago

I thought it was the KeyboardAvoidingView PR, but it is not, tested a revert locally and didn't fix the issue.

rlinoz commented 2 weeks ago

I gotta go soon, will add the help wanted label

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

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

rlinoz commented 2 weeks ago

Following this steps https://github.com/Expensify/App/issues/52496#issuecomment-2474713301 I was able to reproduce it in the production branch

cc: @francoisl

rlinoz commented 2 weeks ago

Reproduced in version 9.0.47-1 downloaded from the store, which I believe it is the prod version right now.

So I think it should be safe to remove the deploy blocker, what do you think @francoisl ?

I gotta go now though

francoisl commented 2 weeks ago

Ah ok then yes, sounds good, let's demote. Also seems to only happen in this specific flow of opening chats back to back, which is not a core flow. Thanks for looking.

MFaheemRajput commented 2 weeks ago

model mapping. I have to check that model keys are allowed to update.

melvin-bot[bot] commented 2 weeks ago

📣 @MFaheemRajput! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

Make sure you've read and understood the contributing guidelines.
Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Format:
```
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>
```

QichenZhu commented 2 weeks ago

This might help:

https://github.com/software-mansion/react-native-reanimated/issues/6418#issuecomment-2296107100

This problems occurs, because when the user is moving an item, it triggers an update. This update synchronously triggers the logic responsible for scrolling the list (so it triggers another update). This means that there is a recursive call to the ShadowTree::commit function in RN. Currently such recursive calls lead to a deadlock (on Android). The issue seems to have been noticed by the RN team and had been addressed in https://github.com/facebook/react-native/pull/44725, but the fix is currently behind a feature flag. I tested it, and it seems to resolve our issue. I'm not sure when this behavior will be made default.

rlinoz commented 2 weeks ago

Interesting, maybe we can try a patch with that feature flag on

rlinoz commented 2 weeks ago

adding this one to quality since it is flaky

melvin-bot[bot] commented 2 weeks ago

Current assignee @ahmedGaber93 is eligible for the External assigner, not assigning anyone new.

jacobkim9881 commented 2 weeks ago

Steps: 1 - Create button > Start chat 2 - Go back using the gesture to go back (the back button on the device probably also works) 3 - Repeat until it freezes

That indeed makes a crash. It feels like letting the app load twice though loading isn't finished.

QichenZhu commented 2 weeks ago

Proposal

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

App occasionally freezes after entering Start Chat screen.

What is the root cause of that problem?

This is an upstream issue described in this comment https://github.com/software-mansion/react-native-reanimated/issues/6418#issuecomment-2296107100 and this PR https://github.com/facebook/react-native/pull/44725.

These recursive commit+mount operations happen because it's possible to trigger state updates while we mount changes in the host platform (e.g.: we create the scroll view and we update the state to set the content offset). Those state updates trigger more mount operations, which deadlock in the mentioned place.

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

The upstream fix is currently behind a feature flag, so we could put this on hold until they make it the default, then upgrade React Native.

What alternative solutions did you explore? (Optional)

Enable the feature by changing the condition in the lines below to true.

node_modules/react-native/ReactAndroid/src/main/jni/react/fabric/Binding.cpp

  if (true /* ReactNativeFeatureFlags::
          allowRecursiveCommitsWithSynchronousMountOnAndroid() */) {

PS: I remember several freeze issues came up after PR https://github.com/Expensify/App/pull/48772 was merged, so it was reverted. Those issues might share the same root cause as this one, but PR https://github.com/Expensify/App/pull/48772 wasn’t the root cause.

ahmedGaber93 commented 2 weeks ago

@QichenZhu Thanks for the proposal.

I tested your proposal, and it fixes the issue, also react-native-reanimate team mention this fix https://github.com/software-mansion/react-native-reanimated/issues/6418#issuecomment-2298202842, it allows a piece of code that depend on a feature flag to run but not enabled the feature on the whole app. I tested it as possible, and it works well, But I don't know if there are any consequences of this or not, so let's see what @rlinoz think?

rlinoz commented 2 weeks ago

Asked internally here for thoughts on enabling the feature flag.

rlinoz commented 2 weeks ago

Apparently the feature flag will be enabled by default in RN 0.77 so let's go with the patch enabling the flag for now

melvin-bot[bot] commented 2 weeks ago

📣 @ahmedGaber93 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

melvin-bot[bot] commented 2 weeks ago

📣 @QichenZhu 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

melvin-bot[bot] commented 1 week ago

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot[bot] commented 1 week ago

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

https://github.com/Expensify/App/pull/52732

If no regressions arise, payment will be issued on 2024-11-29. :confetti_ball:

For reference, here are some details about the assignees on this issue:

@ahmedGaber93 requires payment automatic offer (Reviewer)
@QichenZhu requires payment automatic offer (Contributor)

melvin-bot[bot] commented 1 week ago

@ahmedGaber93 @VictoriaExpensify @ahmedGaber93 The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

VictoriaExpensify commented 1 day ago

Hey @ahmedGaber93 - can you please complete the checklist in the comment above and accept our offer in Upwork so I can pay you? https://www.upwork.com/nx/wm/offer/104951617

Contributor: @QichenZhu paid $250 via Upwork

ahmedGaber93 commented 1 day ago

BugZero Checklist:

[x] [Contributor] Classify the bug:

Bug classification

Source of bug: - [ ] 1a. Result of the original design (eg. a case wasn't considered) - [ ] 1b. Mistake during implementation - [ ] 1c. Backend bug - [x] 1z. Other: external libraries `react-native-reanimated` and `react-native` Where bug was reported: - [ ] 2a. Reported on production - [x] 2b. Reported on staging (deploy blocker) - [ ] 2c. Reported on both staging and production - [ ] 2d. Reported on a PR - [ ] 2z. Other: Who reported the bug: - [ ] 3a. Expensify user - [ ] 3b. Expensify employee - [ ] 3c. Contributor - [ ] 3d. QA - [x] 3z. Other: Applause Internal Team

[x] [Contributor] 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: N/A. It is a known issue external library https://github.com/software-mansion/react-native-reanimated/issues/6418#issuecomment-2296107100
[x] [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: N/A.
[x] [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.
[ ] [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

Link to issue:

Regression Test Proposal

Precondition:

Test:

Android native only

Tap FAB > Start chat > Go back by swiping or using header back button.
Repeat the above steps multiple times (~10 times) and verify the app is not crashed.

Do we agree 👍 or 👎

ahmedGaber93 commented 1 day ago

can you please complete the checklist in the comment above and accept our offer in Upwork so I can pay you?

@VictoriaExpensify Checklist have been completed, and I will get paid via NewDot

Expensify / App

[HOLD for payment 2024-11-29] [$250] Android - App crashes when create new chat #52496

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

BugZero Checklist:

Regression Test Proposal

Precondition:

Test:

Android native only