Skip to content
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: TypeError: Cannot read property 'contains' of undefined, js engine: hermes, stack: #11803

Closed
sentry-io bot opened this issue May 1, 2020 · 20 comments

Comments

@sentry-io
Copy link

sentry-io bot commented May 1, 2020

Sentry Issue: WORDPRESS-ANDROID-8XJ

JavascriptException: TypeError: Cannot read property 'contains' of undefined, js engine: hermes, stack:
anonymous@1167:261
anonymous@1157:2480
forEach@-1
anonymous@1157:2451
*** TRUNCATED ***

    at com.facebook.react.modules.core.ExceptionsManagerModule.reportException(ExceptionsManagerModule.java:71)
    at java.lang.reflect.Method.invoke(Method.java)
    at com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:371)
    at com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:150)
    at com.facebook.react.bridge.queue.NativeRunnable.run(NativeRunnable.java)
...
(6 additional frame(s) were not displayed)
@designsimply
Copy link
Contributor

Events in the last 90 days: 33,000
Users affected in the last 90 days: 8,900
https://sentry.io/share/issue/f69cf2cbf05f4064a66748d515cab007/

While the total number of events recorded for and users affected is high for this crash, only 32 events have been tracked for it in 14.6.1 since it was released 9d ago on Apr 22.

@designsimply
Copy link
Contributor

Events: 35,000
Users affected: 9,100
Culprit: com.facebook.react.modules.core.ExceptionsManagerModule in reportException
Also see: this graph
https://sentry.io/share/issue/f69cf2cbf05f4064a66748d515cab007/

This one appears to affect A LOT of different users across a lot of different device families, though if you look at the last 90 days it looks like something in 14.5 (released Apr 6) affected this crash making the numbers go down in early April.

Screen Shot 2020-05-19 at May 19 2 22 27 PM

70% of the cases from the last 30 days happened for WPAndroid versions 14.3, 14.4, and 14.4.1 with the overall average number of events recorded falling from ~1,000 per day in late March to <100 per day in mid-May.

Note: some mentions of d/WordPress-MAIN: ConnectionChangeReceiver successfully unregistered in the breadcrumbs (ref) made me wonder if these users are possibly switching back and forth to different connections as one possible trigger. 🤔

I saw this in at least a few sets of breadcrumbs that started with d/WordPress-API: Dispatching action: PostAction-FETCHED_REVISIONS (ref):

i/WordPress-EDITOR: 'VirtualizedList: You have a large list that is slow to update - make sure your renderItem function renders components that follow React performance best practices like PureComponent, shouldComponentUpdate, etc.', { dt: 932, prevDt: 1220, contentLength: 4436.72705078125 }

editor_has_hw_disabled is mentioned in every single breadcrumb list I've looked at so far.

My thoughts: I think this crash happens mostly in the editor, often in revisions, and sometimes after tapping on notifications. It seems to happen after the user has been editing a post but does a lot of different activity which adds up until it overwhelms whatever list the editor using to track those actions.

@mchowning since the number of affected users is still not negligible (~3k in the last 30d) could you help me look into this one and also recommend a priority? Does a mention of hermes js engine in the stacktrace undoubtedly mean it's a crash related to the block editor?

@mchowning
Copy link
Contributor

since the number of affected users is still not negligible (~3k in the last 30d) could you help me look into this one and also recommend a priority?

It looks to me like this issue is continuing to decrease in frequency (503 times in the last 7 days compared to almost 3000 in the last month). That's not really surprising since over 80% of the crashes in the past month were on releases before 14.5 (i.e., users who haven't updated their app).

I think a better number to use in assessing the importance of fixing this is the number of crashes we're still seeing on versions >= 14.5. For the last 30 days, I'm seeing about 500 such crashes. That indicates to me that although this isn't as urgent as the 3000 affected users number indicates, but it is still an issue affecting many users and it's not going to go away on its own once everyone updates their app. This does seem like a medium or high priority bug to me.

Does a mention of hermes js engine in the stacktrace undoubtedly mean it's a crash related to the block editor?

Yes, it's the block editor.

@hypest, do you remember any changes with respect to hermes in WPAndroid 14.5 (Gutenberg 1.24.0? I did a quick check and nothing jumped out at me.

@hypest
Copy link
Contributor

hypest commented May 21, 2020

By the way, I think that "Hermes" being mentioned on the stacktraces is not revealing any particular information, other than yes indeed, this is the block editor since Hermes is only used in the block editor for the time being. We've switched to Hermes on WPAndroid v14.1 so, all stacktrace messages that care to report their JS engine will mention Hermes from that version on, and the issue will most likely not be related to Hermes itself.

That said, I'll also have a look at the changes that landed in v14.5 to see If I can spot something.

@mchowning
Copy link
Contributor

By the way, I think that "Hermes" being mentioned on the stacktraces is not revealing any particular information...all stacktrace messages that care to report their JS engine will mention Hermes from that version on, and the issue will most likely not be related to Hermes itself.

I didn't realize that. Thanks @hypest !

@designsimply
Copy link
Contributor

designsimply commented May 28, 2020

Still trending down, but still worth keeping an eye on.

Note: only 44 events were recorded so far for 14.8 since it was released 9d ago.

Looks like ~47% of the total crashes are on Android 9 devices and ~59% of the total crashes happened for WPAndroid 14.3 and 14.4.1 (for the last 90d).

@designsimply
Copy link
Contributor

Events: 36,000
Users affected: 4,300
Culprit: com.facebook.react.modules.core.ExceptionsManagerModule in reportException
WORDPRESS-ANDROID-8XJ: https://sentry.io/share/issue/f69cf2cbf05f4064a66748d515cab007/

image

Looking much better! But still seems there's a decent number incoming including some in the latest releases.

@hypest
Copy link
Contributor

hypest commented Jun 29, 2020

@mchowning happens to be on Groundskeeping role this week. Not sure what the rest of the groundskeeping list looks like in terms of gutenberg-mobile issue @designsimply but, maybe you can have a deeper look at this one Matt?

@mchowning
Copy link
Contributor

maybe you can have a deeper look at this one Matt?

👍 I'll add this to my list and take a look if there's time.

@mchowning
Copy link
Contributor

mchowning commented Jun 30, 2020

Looking much better! But still seems there's a decent number incoming including some in the latest releases.

I'm seeing around 8 crashes/day for 15.0 since it was released, so you're definitely right that this issue is still showing up with the latest release, although it's not too frequent.

I'm concerned, however, that this Sentry "issue" may actually grouping a bunch of crashes that are unrelated except for the fact that they originate from javascript. The most recent 20 or so crashes for this issue in WPAndroid 15.0 includes these different error messages:

  • JavascriptException: RangeError: Maximum call stack size exceeded (native stack depth)
  • JavascriptException: TypeError: undefined is not a function, js engine: hermes, stack:
  • JavascriptException: TypeError: Cannot read property 'attributes' of undefined, js engine: hermes, stack:
  • JavascriptException: TypeError: Cannot read property 'multiline' of undefined, js engine: hermes, stack:
  • JavascriptException: TypeError: Cannot read property 'textContent' of null

I'm actually noticing a similar pattern of multiple errors being grouped together with #10493, so maybe I am just misunderstanding what these mean.

@designsimply , would you mind taking a look and letting me know if you see the same thing or if perhaps I'm mis-reading Sentry? (I'm still learning how to use Sentry) If I'm not mis-reading this, then these separate crashes are even less common when considered individually. Also, it seems like we should see what we can do to get Sentry to stop grouping these all together.

@designsimply
Copy link
Contributor

I think the problem with grouping may be JN/RN specific. @mchowning would you mind adding your perspective to a new p2 post or to p77Llu-do3-p2 (internal reference) ?

I did look through the latest events inside the WORDPRESS-ANDROID-8XJ Sentry issues and noticed at least a few that mentioned possible correlations in the exceptions themselves but I am not sure if this is why Sentry is grouping unrelated events together:

Error: An error occurred while running 'mapSelect': Cannot set property 'block' of undefined
The error may be correlated with this previous error:
TypeError: Cannot set property 'block' of undefined

Related: https://docs.sentry.io/data-management/event-grouping/

I checked the Events tab on the Sentry issue just now and 43 out of 100 latest events showing on the first page are the same error mentioned in the title of this issue:

TypeError: Cannot read property 'contains' of undefined, js engine: hermes, stack:

And I think we should focus on that error for the purposes of this GitHub issue. wdyt?

In addition, I will also see what I can find out about what has been done already and how we can perhaps improve the grouping for issues related to the JS stack.

For reference, here are the latest stats on the Sentry issue currently:

Events last 90d: 37,000
Users affected last 90d: 786
Culprit: com.facebook.react.modules.core.ExceptionsManagerModule in reportException
WORDPRESS-ANDROID-8XJ: https://sentry.io/share/issue/f69cf2cbf05f4064a66748d515cab007/

image

@designsimply
Copy link
Contributor

Events last 90d: 38,000
Users affected last 90d: 606
WORDPRESS-ANDROID-8XJ: https://sentry.io/share/issue/f69cf2cbf05f4064a66748d515cab007/

image

@designsimply
Copy link
Contributor

Number of occurrences on this one seems really high compared to users affected. Why?

@geriux geriux self-assigned this Oct 19, 2020
@geriux
Copy link
Member

geriux commented Oct 25, 2020

Number of occurrences on this one seems really high compared to users affected. Why?

After a lot of investigation, I found most of these issues are related to pasting HTML content into the editor, there are cases that are breaking the parsing system so some users encounter this issue multiple times when trying to paste something making the number of occurrences to be higher.

Unfortunately, I wasn't able to reproduce them but I wanted to share what is happening so we can work on finding solutions now that it's investigated.

I took some of the errors and decoded them using the sourcemaps of the editor bundles:

Error: An error occurred while running 'mapSelect': Cannot set property 'block' of undefined

at packages/blocks/src/api/raw-handling/special-comment-converter.js:60
  at customText (packages/blocks/src/api/raw-handling/special-comment-converter.js:54)

TypeError: Cannot read property 'attributes' of undefined, js engine: hermes, stack:

at attributes (packages/blocks/src/api/parser.js:292)
  at packages/blocks/src/api/raw-handling/paste-handler.js:107
  at packages/blocks/src/api/raw-handling/paste-handler.js:98
  at htmlToBlocks (packages/blocks/src/api/raw-handling/paste-handler.js:266)

TypeError: Cannot set property '_parentNode' of undefined, js engine: hermes, stack:

  at node (packages/blocks/src/api/raw-handling/ms-list-converter.js:57)
  at node (packages/blocks/src/api/raw-handling/utils.js:116)
  at packages/blocks/src/api/raw-handling/utils.js:110

Other issues not related to parsing HTML

TypeError: undefined is not a function, js engine: hermes, stack:

at onDeleteBlock (packages/block-library/src/button/edit.native.js:206)
  at onDelete (packages/rich-text/src/component/index.native.js:397)

TypeError: Cannot convert null value to object

at packages/components/src/mobile/gradient/index.native.js:21
  at gradientValue (packages/components/src/mobile/gradient/index.native.js:67)

@designsimply
Copy link
Contributor

Events last 90d: 39,000
Users affected last 90d: 612
WORDPRESS-ANDROID-8XJ: https://sentry.io/share/issue/f69cf2cbf05f4064a66748d515cab007/

image

@geriux geriux removed their assignment Nov 26, 2020
@designsimply
Copy link
Contributor

@mchowning do you think @geriux's notes help confirm that this Sentry issue is grouping errors in a way that makes the crash report hard to work with or is it the opposite and the notes help us know that what we should be looking at is HTML parsing? I'm trying to work out how to make this issue actionable or if we should try to separate it into smaller ones.

@mchowning
Copy link
Contributor

mchowning commented Dec 3, 2020

do you think @geriux's notes help confirm that this Sentry issue is grouping errors in a way that makes the crash report hard to work with or is it the opposite and the notes help us know that what we should be looking at is HTML parsing?

It still seems to me that Sentry's grouping is making things more difficult since it is grouping other unrelated issues in with the HTML parsing.

I'm trying to work out how to make this issue actionable or if we should try to separate it into smaller ones.

Since @geriux's great investigation ❤️ found that most of the crashes are related to HTML parsing, my take would be to focus this GitHub issue only on resolving those HTML issues. Of course, if we can separate out the HTML parsing errors from the other errors within Sentry that would be great, but I suspect that doing that will require code changes (I'm definitely not a Sentry expert, so let me know if your think there might be a way to do this from within Sentry). In fact, updating the code to generate different exceptions for errors parsing manually edited or pasted html might end up being the best we can do to resolve this issue since some of these issues could from users inputting invalid html.

Once we "resolve" the HTML issue, it will be much easier to separate out any unrelated issues that Sentry happens to be grouping in here since the HTML problems will (hopefully) no longer be showing up. Of course, it only makes sense to delay addressing the other issues as long as the number of users that are being affected by this issue stays steady and relatively low--if the number of users affected by this issue starts dramatically increasing, there may a high-priority, unrelated issue being grouped here that we need to separate out and address sooner.

@designsimply
Copy link
Contributor

Ace. It's helpful to note that we should be watching the users affected and not the overall number of incidences for this crash, which is a special case. I will prioritize accordingly the next time I report top issues. 👍

@jkmassel we did discuss making some changes to help group these types of issues better, but I don't know the status of that change. Would you mind giving an update on that?

@designsimply
Copy link
Contributor

Events last 90d: 40,000
Users affected last 90d: 796
Culprit: com.facebook.react.modules.core.ExceptionsManagerModule in reportException
WORDPRESS-ANDROID-8XJ: https://sentry.io/share/issue/f69cf2cbf05f4064a66748d515cab007/

image

image

Trending up overall but pretty low numbers for the latest release which was 16.4 released 10 days ago on Jan 11.

@designsimply
Copy link
Contributor

Because this crash is grouped oddly in Sentry (and we made improvements to grouping after this one was already grouped) and because we already fixed the source of a major uptick we saw in Mar/Apr 2020, I am going to close this crash even though there is a very slight uptick over the last few months as it doesn't feel significant enough to spend time on right now considering other bigger crashes in play. What I am hoping is that we catch any remaining problems in subsequent crash reports if they get bigger.

Please please re-open this if you find it later and feel it should be looked at again! 🙏

WORDPRESS-ANDROID-8XJ: https://sentry.io/share/issue/f69cf2cbf05f4064a66748d515cab007/

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants