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

[fluiddot]

Sentry Url: https://sentry.io/share/issue/f69cf2cbf05f4064a66748d515cab007/

NOTE: The following values are not exact because the issue is including other events not related to this one. In fact, the numbers for this issue should be way low as I checked in the events.

User Count: 4010 Count: 52909 First Release: org.wordpress.android-14.2-rc-1 First Seen: Feb 11, 2020 11:41:54 PM UTC Last Seen: Jun 4, 2021 5:13:43 PM UTC 24 Hours: 243 30 Days: 7932

Stacktrace

``` JavascriptException: TypeError: Cannot set property '_parentNode' of undefined, js engine: hermes, stack: removeChild (address at@1:849347 anonymous (address at@1:2413629 anonymous (address at@1:2455004 forEach@-1 anonymous (address at@1:2454952 forEach@-1 b (address at@1:2454885 anonymous (address at@1:2454938 forEach@-1 b (address at@1:2454885 anonymous (address at@1:2455203 anonymous (address at@1:2409430 Mt (address at@1:1923044 Tf (address at@1:1960293 anonymous (address at@1:1972649 anonymous (address at@1:2409027 anonymous (address at@1:3048972 value (address at@1:2080439 apply@-1 b (address at@1:156900 apply@-1 k (address at@1:156957 apply@-1 C (address at@1:156989 N (address at@1:157130 A (address at@1:157576 call@-1 z (address at@1:157492 anonymous (address at@1:160654 _e (address at@1:210979 Ne (address at@1:160144 Ue (address at@1:160477 receiveEvent (address at@1:205395 apply@-1 value (address at@1:115550 anonymous (address at@1:113971 value (address at@1:115147 value (address at@1:113929 ```

[fluiddot]

I haven't managed to reproduce it but as I can check from the stacktracebreadcrumbs, it happens right after the editor is initialized.

Additionally, in the last release (17.4), so far, we only received 34 events (this can be checked by using the following query in the events tag: version:"17.4" "Cannot set property '_parentNode").

[fluiddot]

@hypest As part of the crash triaging process, the issue has to be assigned to a developer but in this case, as it's a crash upon editor startup, I'm not sure who would be the best one to assign. I'd appreciate if you could point me out how to process this, thanks 🙇 !

[hypest]

as I can check from the stacktrace, it happens right after the editor is initialized.

Hmm, what's on the stacktrace that shows the crash happens right after editor init? Is it the stacktrace or the breadcrumps perhaps?

[fluiddot]

as I can check from the stacktrace, it happens right after the editor is initialized.

Hmm, what's on the stacktrace that shows the crash happens right after editor init? Is it the stacktrace or the breadcrumps perhaps?

Oh sorry, I meant the breadcrumbs (I'll update the previous comment), the error is thrown between 5-10 seconds after the event editor_session_start is triggered so I had the impression that it might be related to the initialization although it's hard to know because we don't have much information about what happens after that.

[hypest]

I think the only place we have _parentNode is in jsdom-patches and @mkevins I think you've worked on that (not so) recently. Can you have a look? Thanks! cc @chipsnyder for visibility.

[mkevins]

Some context:

From what I recall, the insertBefore function was "monkey-patched" as a kind of polyfill from jsdom-jscore to our more "slimmed-down" import of jsdom-jscore-rn. There was a modification, since that implementation did not keep up with web standards, so this code was commented out which removes the WrongDocumentError.

This error

After investigating this one a bit, I've found some things that might be helpful. First, I examined the reported trace:

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

which suggests that this occurs while trying to set the _parentNode property on an variable which is undefined (which I guess we expect to be an object — probably a Node). The only such place in "our" code is here. However, the trace points to removeChild in the call stack, so this doesn't seem to be where it is coming from (as this would be in the scope of insertBefore).

But there are some possible clues nearby, since insertBefore calls removeChild in two places. One invocation happens within a conditional, which seems less likely to be the culprit, since the newChild must be defined (and even have the property _parentNode as a condition for invocation.

Another invocation occurs without such a condition, and looks like a suspect.

Following up on this clue leads us to the removeChild implementation, which does indeed include a statement setting _parentNode. Also, suspisciouly, there is a conspicuous comment nearby: // TODO - if (!oldChild) then?.

Supposing oldChild was undefined at this point, we'd reach assignment and throw, since the _indexOf neither throws nor returns -1 for an undefined child.

Back-tracking, could removeChild have been called with undefined? I'm not sure where this could come from. :thinking: One possibility is on line 110 described earlier, where the outer condition is that newChild.nodeType === newChild.DOCUMENT_FRAGMENT_NODE. But, in this case, somehow the firstChild would have to be undefined while newChild._childNodes.length is greater than zero, which seems incorrect. Then again, this could have come from any of the other calls to removeChild. If we had steps to reproduce it, that is the first place I'd put a breakpoint for debugging further.

[hypest]

@mkevins , can we add some more logging for debugging, to test some of the possible cases here?

[mkevins]

I spent some additional time searching the breadcrumbs for clues, but did not find anything. I did notice, as Carlos also mentioned, that some other issues seem to be grouped with this one in Sentry, even though the appear to be unrelated (aside from being JavascriptExceptions).

Regarding more logging or debugging, I'm not sure where to add something. My hope was that we could use source-maps, but that doesn't seem possible at the moment, so things like console.trace() probably wouldn't get us better than what we already have. A quick grep of the jsdom-jscore-rn dependency that we are using gives this for removeChild:

./lib/builtins/nwmatcher/src/nwmatcher.js:198:      root.removeChild(a);
./lib/builtins/nwmatcher/src/nwmatcher.js:283:        while (context.firstChild) { context.removeChild(context.firstChild); }
./lib/builtins/nwmatcher/src/modules/nwmatcher-cache.js:56:      root.removeChild(div);
./lib/builtins/nwmatcher/src/modules/nwmatcher-cache.js:156:        mutationTest('DOMNodeRemoved', function(e) { e.removeChild(e.appendChild(document.createElement('div'))); })) {
./lib/jsdom/level3/core.js:59:    Node               removeChild(in Node oldChild)
./lib/jsdom/level3/core.js:178:    this.removeChild(this.childNodes.item(i));
./lib/jsdom/level3/xpath.js:30:    document.documentElement.removeChild(script);
./lib/jsdom/level2/html.js:708:        el._parentNode.removeChild(el);
./lib/jsdom/level2/html.js:1309:        this.removeChild(this.childNodes[0]);
./lib/jsdom/level2/html.js:1373:        el._parentNode.removeChild(el);
./lib/jsdom/level2/html.js:1387:        el._parentNode.removeChild(el);
./lib/jsdom/level2/html.js:1401:        c._parentNode.removeChild(c);
./lib/jsdom/level2/html.js:1435:      tr._parentNode.removeChild(tr);
./lib/jsdom/level2/html.js:1503:      this.removeChild(tr);
./lib/jsdom/level2/html.js:1559:      this.removeChild(td);
./lib/jsdom/browser/index.js:142:      this.removeChild(child);
./lib/jsdom/browser/index.js:163:      this.removeChild(child);
./lib/jsdom/level1/core.js:536:        tmpNode = newChild.removeChild(newChild.firstChild);
./lib/jsdom/level1/core.js:544:        newChild._parentNode.removeChild(newChild);
./lib/jsdom/level1/core.js:591:    return this.removeChild(oldChild);
./lib/jsdom/level1/core.js:619:  removeChild : function(/* Node */ oldChild){
./lib/jsdom/level1/core.js:762:        this.removeChild(child);
./lib/jsdom/level1/core.js:777:          this.removeChild(child);
./lib/jsdom/level1/core.js:1639:  removeChild : function(/* Node */ arg) {
./lib/jsdom/level1/core.js:1640:    var ret = core.Node.prototype.removeChild.call(this, arg);

And it's not clear which of those it could be coming from. We could monkey-patch some of those implementations, but I have doubts about what we'd learn from that..

If we can find steps to reproduce it, we could bisect to pinpoint when it was introduced, or debug by placing a breakpoint at removeChild. I think the best approach would be to work towards fixing the source-maps, which would remove a lot of the guess-work, though.

[fluiddot]

Looks like this error is not happening yet in beta version of 17.7 (17.7-rc-1 and 17.7-rc-2), as that version has the RN upgrade to 0.64 which includes a lot of fixes, maybe this is already fixed 🤔 .

I think the best approach would be to work towards fixing the source-maps, which would remove a lot of the guess-work, though.

I totally agree, for now, I only managed to integrate the iOS part but it's still on review. Hopefully, once we merge it, we could focus on the Android side which I think shouldn't be too hard to implement.

[mkevins]

Hopefully, once we merge it, we could focus on the Android side which I think shouldn't be too hard to implement.

Awesome! Great work on the PR for supporting source maps on iOS. That will help a lot with these kinds of issues!