-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JavascriptException: RangeError: TaskQueue: Error with task : Maximum call stack size exceeded (native stack depth) #18750
JavascriptException: RangeError: TaskQueue: Error with task : Maximum call stack size exceeded (native stack depth) #18750
Comments
Quick guess, from the trace, it's possible this is related with a reanimated (software-mansion/react-native-reanimated#4151) issue. |
Update: this seems to be a grouping issue on Sentry, as the link back is to many differerent Javascript issues, which all share We do not have symbolicating properly configured (possibly due to the way our app bundles are generated 🤔 ). This would help with many of these issues, and possibly with the way that Sentry is erroneously grouping them together. |
Good point @mkevins. I tried to unravel the grouped issues and collected the following list (ordered by number of events):
The next step would be to symbolize the stack traces and proceed to debug them. Probably we could do this in separate GitHub issues 😅. NOTE: I can't point to the public event reference for each, as they are grouped. |
@fluiddot - on release rotation this sprint, can I assign this to you and remove the triage label? |
@zwarm Yeah, in fact, I'm planning to take a look this week to provide more context about the crash. It's likely that we tackle all the crashes as a mini-project, so can't really tell if it's going to be me in charge of fixing it, but we can re-assign whoever will execute. Thanks! |
I managed to symbolicate the stack trace and added it to the issue's description. Besides, I reproduce it using the post content from one of the Sentry events. I'll share further findings once I narrow down the culprit. |
I've tested the post content from other Sentry events and also crashed in most cases. The common factor is that they have a long nested structure. Using a Samsung Galaxy S20 FE 5G (Android 13), I'm experiencing a crash after nesting 14 Group blocks. However, it's unclear to me why it only crashes when closing the editor, this might be a clue to identifying a potential fix. |
Internal reference: p9ugOq-3WG-p2 I explored the following approaches to address this crash: 1 - Flatten inner blocksIn most cases, the group blocks like Group or Column are rendering their content linearly. There's no visual difference between rendering the blocks nested and in a single block list. This approach considers that fact and tries to flatten all nested inner blocks when being rendered in a non-root This approach seems feasible but we have to be careful to only flatten when it's allowed. For instance, there are some cases like the Columns block, where visualizing the editor in landscape mode we display the content in two columns. In this case, we should preserve the layout by rendering its inner blocks, but we could flatten the deeper inner blocks. As mentioned earlier, there's no visual difference but let me share the render tree as an example. Using the flattened approachThe following render tree (order is descended) is the one obtained when opening a post with 20 group blocks nested. Click here to display the HTML content
Render tree: 1 - Paragraph block located at the deepest level
2 - Parent group block (this is the only inner block list rendered)
3 - Root block list
4 - Editor layout
Using the flattened approach (selecting a group block at level 10)When selecting a block, we force rendering it to display the block outline. Render tree: 1 - Paragraph block located at the deepest level
2 - Parent group block (the one that is selected)
3 - Parent group block
4 - Root block list
5 - Editor layout
Before using the approachThe original render tree is similar but in this case, the components rendered by the Group block are repeated 13 times. This would increase linearly as more blocks are nested. Render tree: 1 - Paragraph block located at the deepest level
2 - Parent group blocks (this sequence is repeated 13 times)
15 - Root block list
16 - Editor layout
2 - Limiting block lists based on nesting depthThis approach is more basic but also provides a worse user experience. The idea is to simply stop rendering block lists that exceed an established maximum nesting depth. In that case, we'll present a warning message (see attached screenshot). I explored this in the branch This option can work in combination with the former, as there might be cases where we can't flatten inner blocks and we still need to limit the depth. Here is an example of rendering a post with 20 group blocks nested (the maximum depth is set to ten levels). |
Similar to wordpress-mobile/gutenberg-mobile#6123 (comment), the fix didn't cover all the cases of the crash. We have reports in version
Same as shared in wordpress-mobile/gutenberg-mobile#6123 (comment), seems we'd need to review the pasting functionality to avoid processing deeply nested structures. |
I wonder if this is partially because the |
Good catch @mkevins! Definitely, the use of
It could be increased but we'd need to compile Hermes, generate the binaries, and integrate the ad-hoc version into the build process. We currently rely on the Hermes binaries provided by the React Native version, so applying this approach won't be trivial. As an alternative, I'd like to explore potential adjustments to the |
Probably this is possible, but keep in mind most of the It is also conceivable to avoid native calls (I think 🤔), by recreating |
Heads up that I'm working on this PR to update the |
Nice! Good idea to try this. If |
I ran some of the unit tests related to paste functionality, as well as manual testing, and so far Regarding the unit tests, I noticed that some are failing in the native version of the editor, and confirmed that a couple of them produce a crash 😮. As a follow-up, I'll create a tracking issue with all the information I captured after testing. |
Similar to #18750 (comment), we spotted new events in
I will follow-up on this in a new GitHub issue in the GBM repository. |
Sentry Issue: JETPACK-ANDROID-7XY
Symbolicated stack trace:
The text was updated successfully, but these errors were encountered: