Bug 204519 – Web Inspector: Uncaught Exception: Missing node for given nodeId

• Component: Web Inspector
• Status: RESOLVED FIXED

[Antoine Quint]

Uncaught Exception in Web Inspector.

If I reload the page with the Web Inspector open, I get this error every time which makes it impossible to diagnose my issue :(

Uncaught Exceptions:
-----------------------
 - Missing node for given nodeId (at Connection.js:147:29)
    _dispatchResponseToPromise @ Connection.js:147:29
    _dispatchResponse @ Connection.js:109:44
    dispatch @ Connection.js:62:35
    dispatchMessageFromTarget @ TargetManager.js:159:35
    dispatchMessageFromTarget @ TargetObserver.js:47:51
    _dispatchEvent @ Connection.js:195:26
    dispatch @ Connection.js:64:32
    dispatch @ InspectorBackend.js:219:52
    ? @ MessageDispatcher.js:42:34
-----------------------

Notes:
Inspected URL:        https://secure1.store.apple.com/shop/checkout?_s=Fulfillment-init
Loading completed:    true
Frontend User Agent:  Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/605.1.15 (KHTML, like Gecko)

[Devin Rousso]

Created attachment 384184 [details]
Patch

[Blaze Burg]

Comment on attachment 384184 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=384184&action=review

> Source/WebInspectorUI/ChangeLog:27
> +        (WI.DOMNode.prototype.querySelectorAll): Added.

Please note in the ChangeLog that many callers have been switched to call via WI.DOMNode so as to receive the benefits of this marking.

> Source/WebInspectorUI/ChangeLog:100
> +        Web Inspector: REGRESSION(r251227): dashboard no longer shows page weight

Oops.

> Source/WebInspectorUI/UserInterface/Models/DOMNode.js:295
> +        this._destroyed = true;

Assert that this hasn't already been marked?

> Source/WebInspectorUI/UserInterface/Models/DOMNode.js:391
>      {

How does this not break everything when early returns do not call the callback? It seems like the moral equivalent of never calling a completionHandler in an error case.

> Source/WebInspectorUI/UserInterface/Models/DOMNode.js:556
> +        if (typeof callback !== "function") {

This looks more like I had expected.

> Source/WebInspectorUI/UserInterface/Models/DOMNode.js:616
> +        if (this._destroyed)

Ditto (DOMNode.js:380)

> Source/WebInspectorUI/UserInterface/Models/DOMNode.js:684
> +            return;

Ditto (DOMNode.js:380)

> Source/WebInspectorUI/UserInterface/Views/BoxModelDetailsSectionRow.js:117
> +        let node = showHighlight ? this.nodeStyles.node : null;

Nice simplification.

> Source/WebInspectorUI/UserInterface/Views/LayerTreeDetailsSidebarPanel.js:-236
> -            WI.domManager.highlightDOMNode(layer.nodeId);

E

[Devin Rousso]

Comment on attachment 384184 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=384184&action=review

>> Source/WebInspectorUI/ChangeLog:100
>> +        Web Inspector: REGRESSION(r251227): dashboard no longer shows page weight
> 
> Oops.

Oops indeed :P

>> Source/WebInspectorUI/UserInterface/Models/DOMNode.js:295
>> +        this._destroyed = true;
> 
> Assert that this hasn't already been marked?

I could've sworn that I added this 🤦‍♂️

>> Source/WebInspectorUI/UserInterface/Models/DOMNode.js:391
>>      {
> 
> How does this not break everything when early returns do not call the callback? It seems like the moral equivalent of never calling a completionHandler in an error case.

Theoretically, these should never get called, so it probably won't break anything, but I see your point.  I'll add some assertions and a `callback("ERROR: node is destroyed");`.

[Devin Rousso]

Created attachment 384211 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 384211 [details]
Patch

Clearing flags on attachment: 384211

Committed r253242: <https://trac.webkit.org/changeset/253242>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/57723648>