Open cortinico opened 1 year ago
@wouterds can we get this repro on a vanilla React Native App on 0.71?
@cortinico I'm gonna have a look this weekend to create a reproducible example for a vanilla react-native
app
Hello, is there a solution for this. I have also encountered many times @wouterds
@hangjunping there is no solution as far as I know (and it is imo a bug inside react-native). I still have to create that reproducible demo in a vanilla react native project though. But feel free to create one if you can quickly and easily reproduce it on a simple demo project.
Hello! I'm facing the same issue on Android 12. So far, I've encountered the problem on the following devices: Motorola, Samsung, and Xiaomi. My application is experiencing several errors daily related to this issue.
Hello, is there a solve for this crash at the moment, or is it something we've to live with..
Please, i'm getting many firebase crashes on Android 12 and can't reproduce. It started after an OS update without any app update. Any idea?
@wouterds Does this crash still occur if you add the enableSynchronizationForAnimated
flag as originally suggested here?
@wilav-dev
Please, i'm getting many firebase crashes on Android 12 and can't reproduce. It started after an OS update without any app update. Any idea?
It only happens on Android 12 as far as I can see, we also discovered it that way.
@nemzutkovic
@wouterds Does this crash still occur if you add the
enableSynchronizationForAnimated
flag as originally suggested here?
Yea, doesn't help.
it's so frustrated to suddenly have many crashes without uploading a new app version. i can't find solutions, and it's not an option to upgrade all my apps to a new RN version right now.
@skantus one question here? wont that cause performance issues going forward since the animations will now run on the JS thread, and the delta in frame calculation values will now keep getting sent over the bridge?
@skantus one question here? wont that cause performance issues going forward since the animations will now run on the JS thread, and the delta in frame calculation values will now keep getting sent over the bridge?
To date, I haven't encountered any performance issues.
Setting useNativeDriver: false
is not a solution + you would need to patch *all* your sub packages, not just your code. It is very visible on budget Android devices, even on simple navigation transitions.
I tried it previously and would not recommend it, although it does reduce crash rate if you are ok with a significant worse UX.
Certainly, in the case of highly intricate animations, there may be an impact on performance. However, rather than emphasizing the potential performance issues, it is advisable to prioritize stability and prevent crashes as a short-term solution. It is worth noting that I am eagerly anticipating a resolution from the RN team to address this matter.
Possible related fix #37927
Possible related fix #37927
We tried it. It worked, It's probably an android bug,Refer to this: https://issuetracker.google.com/issues/261481042
@free46000 When you say it worked, does it mean you stopped getting the crash entirely?? Also since it tweaks the internal implementation of animations, were there cases where you observed frame drops drop in UI FPS, or ANRs increased ?
Like a lot of people, we're seeing our react-native 0.70.5 apps crashing on Android 12 devices. Until there's an official fix, I'm about to release a new version of our apps, patching react-native's NativeAnimatedNodesManager.java to "log and return" rather than throwing exceptions. I know it's a bad idea. It's been a pain to setup with CircleCi/Crashlytics. Our limited internal automation tests show that the crashes aren't occurring anymore, so I'll do a slow roll out in the coming weeks and I will update this thread with the results.
Like a lot of people, we're seeing our react-native 0.70.5 apps crashing on Android 12 devices. Until there's an official fix, I'm about to release a new version of our apps, patching react-native's NativeAnimatedNodesManager.java to "log and return" rather than throwing exceptions. I know it's a bad idea. It's been a pain to setup with CircleCi/Crashlytics. Our limited internal automation tests show that the crashes aren't occurring anymore, so I'll do a slow roll out in the coming weeks and I will update this thread with the results.
@yrichard I tried that but that caused code crashing in other places for me, but maybe I was just too sloppy with patching NativeAnimatedNodesManager.java. Do you mind sharing what you commented out exactly?
@yrichard Would you mind sharing the commit or code changes that you did as a patch fix..
Possible related fix #37927
We tried it. It worked, It's probably an android bug,Refer to this: https://issuetracker.google.com/issues/261481042
How did you apply this patch ?
Thanks
@free46000 When you say it worked, does it mean you stopped getting the crash entirely?? Also since it tweaks the internal implementation of animations, were there cases where you observed frame drops drop in UI FPS, or ANRs increased ?
Yes, there was no performance testing because only the implementation of the queue was changed
Possible related fix #37927
We tried it. It worked, It's probably an android bug,Refer to this: https://issuetracker.google.com/issues/261481042
How did you apply this patch ?
Thanks
We are directly modified source code
@wouterds Here's the patch I created via patch-package
.
react-native+0.70.5.patch
You need to make sure you can build React Native with your app first, so I'd suggest adding a Log.d
statement in NativeAnimatedNodesManager
's constructor, then check that you can build and see the log prior to apply the patch.
Edit: Looks like the patch avoids some crashes but that eventually the app crashes somewhere else.
You'd still need to rebuild RN to get the updated AAR right? That's the only annoying part of needing to patch the Android side IMHO.
@saghul Yes you will have to rebuild the AARs
@cortinico could this be released as a feature flag the same as the previous fix was? I would like to test this in our app but only for certain devices. This is our largest crash on android and is caused only by a handful of phones.
We are on RN 70 and have a blocker to get to 71.
Thanks again for all the help
@cortinico could this be released as a feature flag the same as the previous fix was?
Can we get this as a PR so we can do a pass on it?
Do you need this PR updated with a flag?
Nope I meant @yrichard fix. The problem is that those are all best-guess fix and without a repro this is impossible to effectively fix
Isn't that just a crash avoidance? What is the desired type of fix here?
Isn't that just a crash avoidance? What is the desired type of fix here?
Returning early to avoid throwing or disabling all native animations are indeed not fixes addressing the underlying issue.
While I definitely agree that these options are complete guesses and not the ideal end goal, I do think having either option behind a flag to test would provide some useful information. This issue is nearly impossible to reproduce and there isn't much to go on. One thing I noticed about the crash is it seems to occur when the user transitions from screen to screen and tries to interact with the screen before it's fully ready. My guess is it's more prevalent on slower devices, specifically the ones in the screenshot. This issue is significantly impacting our crash free metrics and is almost 3.5x our next crash.
Our team is really focused on stability so we are ok with a degraded performance or limited animations for these devices. We discussed how we could remove animations, specifically native animations, for these devices but discovered this issue with some traction.
Here is a look at this crash over the last 30 days:
Will the patch be available for RN 0.67.4?
@yrichard Was going through the patch
if (parentNode == null) {
- throw new JSApplicationIllegalArgumentException(
- "connectAnimatedNodes: Animated node with tag (parent) ["
+ ReactSoftExceptionLogger.logSoftException(
+ TAG,
+ new ReactNoCrashSoftException(
+ "connectAnimatedNodes: Animated node with tag (parent) ["
+ parentNodeTag
- + "] does not exist");
+ + "] does not exist")
+ );
+ dropAnimatedNode(childNodeTag);
+ return;
}
Here shouldnt the dropAnimatedNode be the parentNode instead of the childNodeTag.. Was this intentional?
In our company, we also struggle with this random issue. As pointed by @free46000 This root cause really seems in android itself... We apply following patch and we didn't see the issue anymore after 3 days of testing (where it was reproduced sometimes 10 times in 30 minutes).
Notice that in this patch there was a first try to remove exceptions as proposed here, but in that case we had some components not rendered.
Additional information 1) , our app is mainly tested on AndroidTV 12 as we work with STB manufacturers. Additional information 2) , similar patch was applied in react native reanimated as they also use ConcurrentLinkedQueue. Additional information 3) , I can open a pull request if you need.
edit: 04/07: "Mapped property node does not exists" has been reproduced once
shouldnt the dropAnimatedNode be the parentNode instead of the childNodeTag.
@nitish24p I assumed that if the animated node's parent node wasn't available, then we should skip animating the child node by dropping it from animated node list via dropAnimatedNode(childNodeTag);
but I may totally be wrong!
@freeboub Yeah, I experimented with this patch as well but it seems unreliable. Sometimes it looks like it can recover from it and you see the soft crash passing in the logs and the app continues working - but often I see it just crashing further down the line with the same exception as you reported.
java.lang.IllegalArgumentException: Mapped property node does not exists
Wouldn't recommend this patch as a workaround.
--
@cortinico The issue can be reproduced relatively easily with this project on this branch. While react-native-tvos is a fork and this specific example is a little more complex it is the fasted way I found to reproduce the issue, the same bug resides in react-native core, and code crashes somewhere in NativeAnimatedNodesManager.java.
To reproduce just run the example and switch between screens on the main navigation until it crashes. For me it takes about 5 - 10 times switching screens (while running on a real Android (12) TV device, the crappier the better).
I'm currently trying to create a reproducible example with a vanilla react-native project on Android but so far no luck.
I'm currently trying to create a reproducible example with a vanilla react-native project on Android but so far no luck
This would greatly help both me and my colleagues to investigate further what's going on
@wouterds I agree this patch (bypassing throw) should not be integrated in official release, this is an ugly workaround, and it causes components not to be displayed. Anyway, the change from ConcurrentLinkedQueue to LinkedBlockingQueue fixes the connectAnimatedNode issue, but not the Mapped property node does not exists.
This patch seems just like what we did 1 year ago in https://github.com/facebook/react-native/issues/33375#issuecomment-1074627421 ... It's not a SOLUTION, so we are still waiting ...
It's not a SOLUTION, so we are still waiting ...
For context, I'm also still waiting for a reproducer from someone.
@freeboub @coolguy001tv @yrichard Feel free to try to create a reproducible example, I have this but so far I haven't found a way to consistently and quickly reproduce it on Android 12 devices react-native-animated-nodes-crash-example.
As mentioned in https://github.com/facebook/react-native/issues/37267#issue-1697375660 there is a reproducible example on the react-native-tvos fork (that *is* relatively easy to reproduce on an Android 12 device), but as I understood from @cortinico that is not good enough as reproducible example.
@wouterds @cortinico It is really hard to reproduce it in any simulators even we use a real device.
It happened if the device used falls into these criteria:
I reported it in React Navigation before and even the example app provided by React Navigation can replicate it easily in Android Go devices https://github.com/react-navigation/react-navigation/issues/11180
Something update ? Some patch?
:warning: | Missing Reproducible Example |
---|---|
:information_source: | We could not detect a reproducible example in your issue report. Please provide either:
|
In our prod app this crash is what most affects our customers with Android 12. I would like to help in some way with this
:warning: | Missing Reproducible Example |
---|---|
:information_source: | We could not detect a reproducible example in your issue report. Please provide either:
|
Happening after upgrading to 0.71.7 from 0.66.3
Hello guys, any updates on this, this is happening for lot of android 12 users
I was able to create a reproducible example in a test build where I added 20 swimlanes of which 2 out of 3 randomly animate, the screen is quite heavy but if you navigate a few times the app eventually crashes quite quickly with little effort.
Mostly with this exception:
But sometimes also with this one:
Both seem to originate from NativeAnimatedNodesManager.java.
https://user-images.githubusercontent.com/1210628/234848355-ed5b4d7c-8d4d-43fe-bba7-286567cbc70b.mp4
A reproducible example can be found here: https://github.com/wouterds/react-native-tv-example/tree/bug/animation-crash
I can't attach the .apk because it's above the limits of GitHub file upload, but crashlog, video & apk can be found here: https://drops.evix.io/rn-crash-connectAnimatedNodes/
Originally posted by @wouterds in https://github.com/facebook/react-native/issues/33686#issuecomment-1525541443