Bug 188385 – Web Inspector: XHR content sometimes shows as error even though load succeeded

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

XHR content sometimes shows as error even though load succeeded

Steps to Reproduce:
1. Inspect <https://3medu.com/>
2. Reload
3. Show XHRs in Network Tab
4. Select an XHR
5. Show Preview
  => Error message instead of content

Compare to loading the content directly, the content is just some JSON:
https://3medu.com/rs/navigator/find?code=MAP

[Joseph Pecoraro]

<rdar://problem/22395159>

[Joseph Pecoraro]

So I'm seeing the load happen successfully:

    [Log] event: {
        "method": "Network.requestWillBeSent",
        "params": {
            "requestId": "0.119",
            "frameId": "0.2",
            "loaderId": "0.10",
            "documentURL": "https://3medu.com/",
            "type": "XHR",
            "request": {
                "url": "https://3medu.com/rs/navigator/find?code=NAVBAR",
                "method": "GET",
                "headers": {...}
            },
            ...
        }
    }

    [Log] event: {
        "method": "Network.responseReceived",
        "params": {
            "requestId": "0.119",
            "frameId": "0.2",
            "loaderId": "0.10",
            "timestamp": 475.15031187294517,
            "type": "XHR",
            "response": {
                "url": "https://3medu.com/rs/navigator/find?code=NAVBAR",
                "status": 200,
                "statusText": "OK",
                "headers": {...},
                "mimeType": "application/json",
                "source": "network"
            }
        }
    }

    [Log] event: {
        "method": "Network.loadingFinished",
        "params": {
            "requestId": "0.119",
            "timestamp": 475.15040089702234,
            "metrics": {
                "protocol": "http/1.1",
                "priority": "low",
                "remoteAddress": "120.55.57.48:443",
                "connectionIdentifier": "86A10B88-2F3E-4433-AA5E-4BC5E3316DD0",
                "requestHeaders": {...},
                "requestHeaderBytesSent": 550,
                "requestBodyBytesSent": 0,
                "responseHeaderBytesReceived": 286,
                "responseBodyBytesReceived": 664,
                "responseBodyDecodedSize": 664
            }
        }
    }

But when the frontend tries to request the data for this request (0.119) we get a bad response:


    [Log] request: {
        "id": 163,
        "method": "Network.getResponseBody",
        "params": {
            "requestId": "0.119"
        }
    }

    [Log] response: {
        "error": {
            "code": -32000,
            "message": "No data found for resource with given identifier",
            "data": [{
                "code": -32000,
                "message": "No data found for resource with given identifier"
            }]
        },
        "id": 163
    }

So now I have to step through InspectorNetworkAgent::getResponseBody and see what's up. I'd expect there to be ResourceData that can be decoded to text. The response was `"Content-Type": "application/json"` so it should be decoded as text pretty easily.

[Joseph Pecoraro]

There is a NetworkResourceData, but it has no content:

    (lldb) p *this
    (WebCore::NetworkResourcesData::ResourceData) $2 = {
      ...
      m_url = { length = 47, contents = 'https://3medu.com/rs/navigator/find?code=NAVBAR' } {
        m_impl = { m_ptr = 0x000000044c0ce168 { length = 47, is8bit = 1, contents = 'https://3medu.com/rs/navigator/find?code=NAVBAR' } }
      }
      m_content = { length = 0, contents = '' } {
        m_impl = { m_ptr = 0x0000000000000000 }
      }
      m_textEncodingName = { length = 0, contents = '' } {
        m_impl = { m_ptr = 0x0000000000000000 }
      }

Something should have set this earlier.

[Joseph Pecoraro]

XMLHttpRequest::didFinishLoading used to call through to InspectorInstrumentation::didFinishXHRLoading. I removed that in r225546 expecting to go down the normal InspectorNetworkAgent::didReceiveData addResourceData path.

Interesting that path gets avoided if the XHR is synchronous (which may be a problem for some of the missing XHRs).

[Joseph Pecoraro]

Heh, in this case we are loading synchronously.

[Joseph Pecoraro]

Allowing synchronous XHR to behave the same as async here looks to be correct behavior now that `didLoadXHRSynchronously` doesn't exist to force setting the content. It also corrects the behavior of the https://3medu.com/ XHRs.

[Joseph Pecoraro]

<rdar://problem/42646160>

[Joseph Pecoraro]

Created attachment 346744 [details]
[PATCH] Proposed Fix

[Devin Rousso]

Comment on attachment 346744 [details]
[PATCH] Proposed Fix

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

r=me

Just out of curiosity, does this do anything for that longstanding bug about base64 content from XHRs not being previewable?

> LayoutTests/http/tests/inspector/network/xhr-response-body.html:7
> +function xhrGet(url, asyncFlag=true) {

Style: do we prefer to add spaces around the `=` for optionals?

> Source/WebCore/inspector/NetworkResourcesData.h:127
> +    ResourceData const* maybeAddResourceData(const String& requestId, const char* data, size_t dataLength);

Style: Is this a new style "rule"?  IIRC, this is the same as `const ResourceData*` since `const` applies to the left 🤔

> Source/WebCore/inspector/agents/InspectorNetworkAgent.cpp:493
> +        // For non-sync data would be transferred from a cached resource, but sync XHRs may not have those.

This reads awkwardly.  "Sync XHRs might not have a cached resource, while non-sync XHRs usually transfer the data over on completion." or something like that.

[Joseph Pecoraro]

Comment on attachment 346744 [details]
[PATCH] Proposed Fix

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

>> Source/WebCore/inspector/NetworkResourcesData.h:127
>> +    ResourceData const* maybeAddResourceData(const String& requestId, const char* data, size_t dataLength);
> 
> Style: Is this a new style "rule"?  IIRC, this is the same as `const ResourceData*` since `const` applies to the left 🤔

I was matching another function in this file. I think here the "pointer" is const, but the ResourceData is not. So the pointer is not allowed to change. Probably not necessary.

[Joseph Pecoraro]

(In reply to Devin Rousso from comment #9)
> Comment on attachment 346744 [details]
> [PATCH] Proposed Fix
> 
> Just out of curiosity, does this do anything for that longstanding bug about
> base64 content from XHRs not being previewable?

I think what you are describing was addressed a while ago when reworking the Network Tab. Turns out I broke Synchronous XHR for text content, and here I'm fixing Synchronous XHR for text and binary content.

Namely:
https://trac.webkit.org/r225546
<https://webkit.org/b/141389> Web Inspector: content views for resources loaded through XHR do not reflect declared mime-type

You can probably view the history of the test file for more history:
LayoutTests/http/tests/inspector/network/xhr-response-body.html

[Joseph Pecoraro]

<https://trac.webkit.org/changeset/234702/webkit>