[$2000] Investigate: 💥 Crash 💥 com.facebook.react.uimanager.ViewManagersPropertyCache$FloatPropSetter.getValueOrDefault

[melvin-bot]

Firebase has reported a new crash that we need to fix, here are all the details we found:

Fatal Exception: com.facebook.react.bridge.JSApplicationIllegalArgumentException

Error while updating property 'zIndex' of a view managed by: RCTView

ViewManagersPropertyCache.java line 250 com.facebook.react.uimanager.ViewManagersPropertyCache$FloatPropSetter.getValueOrDefault

Number of Crashes: 4

Device Information

• Platforms: Android
• App Versions: 1.2.3-0
• Devices: Google AOSP on IA Emulator 9

Stacktraces

Android 1.2.3-0

com.facebook.react.uimanager.ViewManagersPropertyCache$PropSetter.updateViewProp (ViewManagersPropertyCache.java:101)
com.facebook.react.uimanager.ViewManagerPropertyUpdater$FallbackViewManagerSetter.setProperty (ViewManagerPropertyUpdater.java:136)
com.facebook.react.uimanager.ViewManagerPropertyUpdater.updateProps (ViewManagerPropertyUpdater.java:56)
com.facebook.react.uimanager.ViewManager.updateProperties (ViewManager.java:48)
com.facebook.react.uimanager.ViewManager.createViewInstance (ViewManager.java:144)
com.facebook.react.uimanager.ViewManager.createView (ViewManager.java:77)
com.facebook.react.uimanager.NativeViewHierarchyManager.createView (NativeViewHierarchyManager.java:281)
com.facebook.react.uimanager.UIViewOperationQueue$CreateViewOperation.execute (UIViewOperationQueue.java:194)
com.facebook.react.uimanager.UIViewOperationQueue$DispatchUIFrameCallback.dispatchPendingNonBatchedOperations (UIViewOperationQueue.java:1110)
com.facebook.react.uimanager.UIViewOperationQueue$DispatchUIFrameCallback.doFrameGuarded (UIViewOperationQueue.java:1081)
com.facebook.react.uimanager.GuardedFrameCallback.doFrame (GuardedFrameCallback.java:29)
com.facebook.react.modules.core.ReactChoreographer$ReactChoreographerDispatcher.doFrame (ReactChoreographer.java:175)
com.facebook.react.modules.core.ChoreographerCompat$FrameCallback$1.doFrame (ChoreographerCompat.java:85)
android.view.Choreographer$CallbackRecord.run (Choreographer.java:947)
android.view.Choreographer.doCallbacks (Choreographer.java:761)
android.view.Choreographer.doFrame (Choreographer.java:693)
android.view.Choreographer$FrameDisplayEventReceiver.run (Choreographer.java:935)
android.os.Handler.handleCallback (Handler.java:873)
android.os.Handler.dispatchMessage (Handler.java:99)
android.os.Looper.loop (Looper.java:193)
android.app.ActivityThread.main (ActivityThread.java:6669)
java.lang.reflect.Method.invoke
com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:493)
com.android.internal.os.ZygoteInit.main (ZygoteInit.java:858)

---

Internal Firebase Info

💥 DO NOT EDIT THIS SECTION 💥

Crash IDs: b4b64ee516fd3cc09bb472809f4686ce
Exception: Error while updating property 'zIndex' of a view managed by: RCTView

[melvin-bot]

Triggered auto assignment to @arielgreen (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

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

[melvin-bot]

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

[arielgreen]

https://www.upwork.com/jobs/~01bc72f85491adcfc9

[AwaisKhan128]

Hi there,
I found your job proposal and jumped out here. The crash has mentioned that "Error while updating property 'zIndex' of a view managed by: RCTView" which implies that all story evolves around zIndex.

I researched several issues that were caused in time back reflecting that there are mostly three possibilities that might ensure its existence.

1. ViewManager has no ZIndex property sounds like it would never scaleable in the z direction.
2. Property param type was supplied wrong while updating zIndex ( e.g required int but supplied string).
3. ViewManager was not initialized before updating its zIndex but caused crash.

Over 4 years of React Native development mobile application development. I can ensure its resolution soon.
Lets discuss if anything I got missed and something you would like to add.

I will be waiting for your response.
Thanks,

Regards,
Awais Khan

[mananjadhav]

Thanks for the interest @AwaisKhan128, please refer our contributing guidelines, how to write a proposal and how we work on the jobs here.

We would need the technical proposal crisp and solid enough to solve the issue.

[Uros787]

Proposal

The only place where we are setting/updating zIndex is on ReportActionsList.js

App/src/pages/home/report/ReportActionsList.js

Lines 159 to 166 in 2ffd0e2

	renderCell({item, style, ...props}) {
	const cellStyle = [
	style,
	{zIndex: item.action.sequenceNumber},
	];
	// eslint-disable-next-line react/jsx-props-no-spreading
	return <View style={cellStyle} {...props} />;
	}

Possible problems:

1. String is passed instead of a number
2. Current implementation is passing 2 arrays inside of one array which could possibly confuse the RCTView

Proposed solution:
Lets make sure that string is not passed, and lets make sure that we don't have confusion regarding the props passed

const cellStyle = [
  ...style,
  { zIndex: parseInt(item.action.sequenceNumber, 10) },
];

[mananjadhav]

Thanks for the proposal @Uros787. Were you able to reproduce this at your end? If yes then can you share them. Without proper reproduction, it will be difficult for us to figure out if the solution works.

[Uros787]

Hi @mananjadhav . No, I haven't succeeded in reproducing this crash on my machine.
Here is what I recommend.

Let's create a pr for this, merge it and follow it next week if there are no crashes. I am okay with not taking payment for the week we are testing this.

This fix can only improve performance, so even if it doesn't solve the crash, we bumped the performance a bit.

[mananjadhav]

It would be difficult for me and the QA to test this if we can't reproduce this. Waiting for reproduction steps and corresponding proposals.

[arielgreen]

Price upped.

[AndrewGable]

Should we double again? @arielgreen why is this daily?

[arielgreen]

Discussion ongoing

[arielgreen]

@mananjadhav what's the status here?

[mananjadhav]

@arielgreen Let me check the thread and update here

[arielgreen]

@mananjadhav update please

[mananjadhav]

I checked the thread and there were some reservations on removing removeClippedSubviews which could cause regression related to missing Avatars.

@Uros787 do you have any other way to fix this? or ensure that removeClippedSubviews doesn't impact Avatars in a long group chat?

[AndrewGable]

Agree, I think we are still waiting on proposals.

[Uros787]

@mananjadhav
Could you please provide me with steps on how to reproduce this Avatars problem in group chats

How would you guys feel about implementing spotify flashlist?
https://shopify.github.io/flash-list/

[mananjadhav]

@Uros787 I might not be able to help you reproduce this soon. Can you check the PR you linked in the thread? and check for the reproduction?

How would you guys feel about implementing spotify flashlist?

I'll let @AndrewGable give his feedback on this one. But my opinion is we've invested heavily on FlatList and worked on different items related to scroll, inverted list, performance. I don't see a strong argument of replacing all of that with a new component just for the sake of a crash?

[AndrewGable]

Agree with @mananjadhav - We are not open to switching components at this time.

[Uros787]

@mananjadhav i tried following the steps from that pr. There was nothing specific
I scrolled up and down in long and short chats but didn't notice avatars disappearing

[mananjadhav]

Hmm. @Uros787 Honestly I have had some issues with Android not working fine for me last few days. Can you share a screencast of the long chat where avatars are not missing?

[Uros787]

@mananjadhav Same here with the issues, but I've managed to run it.
Ill try send the screenshot as soon as I can

[arielgreen]

@Uros787 were you able to grab a screenshot?

[Uros787]

Yup, sorry busy week

Here is the video. As you can see this is a pretty long chat, and scrolling up and down fast or slow made no issue. Avatars were there

Screen.Recording.2022-11-09.at.22.28.05.mov

[trjExpensify]

As per this issue, we're going to close this out as it has only happened 4 times. Moving forward, we'll endeavour to only create issues if the crash happens more than 10 times. Thanks!

[Uros787]

can we get the latest updates from firebase on the number of crashes? @trjExpensify

[AndrewGable]

The issue is up to date.

[Uros787]

cool thanks

[roryabraham]

This is happening a lot more now:

Possibly related to this change – but apparently setting removeClippedSubviews={false} didn't fix it

[roryabraham]

Actually, we were just holding it wrong. removeClippedSubviews={false} seemed to do the trick

[Uros787]

well, I guess in that case, my proposal still stands. I said that we should set it to false on android. And as I can see it is only happening on android

[Uros787]

also, i would recommend we revert this change from the pr you sent @roryabraham . I remember I was testing this particular thing, where I would use index instead of the sequence number and I remember there was some weird behavior. Not sure exactly what but I think some indexes were missing, or being duplicated, or skipped. Not sure exactly sorry

[roryabraham]

We ended up going with a different solution altogether:

• Remove renderCell method as it's no longer needed and causes errors #12688
• Remove unnecessary shouldRemoveClippedSubviews prop #12686

Slack thread: https://expensify.slack.com/archives/C01GTK53T8Q/p1668199114530519