1406100 - The 'responseContent' network event update is missing sometimes

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P3)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I am experiencing a case when the "responseContent" network event update is not received from the backend. As a consequence, the response body is not fetched and the (Response tab) UI is empty.

Honza

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

See also: bug 1404227 comment #5

STR: 

1) Open test file: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_message
s_expand.js
2) Uncomment code starring with `responseTab.click();` and also the entire `waitForSourceEditor` method.
3) Run the test in 'verify' mode as follows:

$ ./mach test devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_message
s_expand.js --verify

4) Wait for failure...

I can only see failure in my Ubuntu VM box (not in Win10)

Honza

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

@Alex, do you have an idea why "responseContent" network event update is *not* sent from the backend?

Honza

Comment 3

[Alexandre Poirot [:ochameau]]

(In reply to Jan Honza Odvarko [:Honza] from comment #2)
> @Alex, do you have an idea why "responseContent" network event update is
> *not* sent from the backend?

Isn't it received, but only later, after the switch to response tab?


I'm really lost here.
It looks like we use netmonitor ResponsePanel:
  http://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/components/response-panel.js
and not:
  http://searchfox.org/mozilla-central/source/devtools/client/webconsole/net/components/response-tab.js
???

But at the same time, I do not see FirefoxConnector being involved in this test?
  http://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/connector/firefox-connector.js#48
At least I can't see it being instanciated?
And we would really miss its networkEvent/networkEventUpdate listeners...

So how is responseContent defined at all?

And also I is pretty sad to see that ResponseTab used to be correct regarding fetching data lazily.
It uses this:
  http://searchfox.org/mozilla-central/source/devtools/client/webconsole/net/data-provider.js#25
Here:
  http://searchfox.org/mozilla-central/source/devtools/client/webconsole/net/components/response-tab.js#210
It does what I'm trying to do in netmonitor ResponsePanel...

Also, if I look at that:
  http://searchfox.org/mozilla-central/source/devtools/client/webconsole/new-console-output/store.js#181-201
It seems to force load everything, which is what netmonitor also do and is a very bad practice.

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Alexandre Poirot [:ochameau] from comment #3)
> (In reply to Jan Honza Odvarko [:Honza] from comment #2)
> > @Alex, do you have an idea why "responseContent" network event update is
> > *not* sent from the backend?
> 
> Isn't it received, but only later, after the switch to response tab?
No, not now.

> I'm really lost here.
> It looks like we use netmonitor ResponsePanel:
>  
> http://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/
> components/response-panel.js
> and not:
>  
> http://searchfox.org/mozilla-central/source/devtools/client/webconsole/net/
> components/response-tab.js
> ???

This directory:
source/devtools/client/webconsole/net/

... contains old implementation of the HTTPi in the Console panel
(the old Console front-end). It is *not* used in the new UI.

> But at the same time, I do not see FirefoxConnector being involved in this
> test?
>  
> http://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/
> connector/firefox-connector.js#48
> At least I can't see it being instanciated?
> And we would really miss its networkEvent/networkEventUpdate listeners...
The new Console UI uses directly FirefoxDataProvider (not the generic connector)
http://searchfox.org/mozilla-central/rev/b53e29293c9e9a2905f4849f4e3c415e2013f0cb/devtools/client/webconsole/new-console-output/store.js#173


> So how is responseContent defined at all?
> 
> And also I is pretty sad to see that ResponseTab used to be correct
> regarding fetching data lazily.
> It uses this:
>  
> http://searchfox.org/mozilla-central/source/devtools/client/webconsole/net/
> data-provider.js#25
> Here:
>  
> http://searchfox.org/mozilla-central/source/devtools/client/webconsole/net/
> components/response-tab.js#210
> It does what I'm trying to do in netmonitor ResponsePanel...
Yes, exactly! The old HTTPi did support lazy load and we want that back.
All the HTTP details should be requested from the backend when the user
actually wants to see it. E.g. when the side bar (e.g. the ResponsePanel)
is selected.

> Also, if I look at that:
>  
> http://searchfox.org/mozilla-central/source/devtools/client/webconsole/new-
> console-output/store.js#181-201
> It seems to force load everything, which is what netmonitor also do and is a
> very bad practice.
Yes, we need to change that.

Honza

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

@Alex, I am still struggling with network-event updates sent from the backend (see also comment #2)

There are following network update events:
"requestHeaders"
"requestCookies"
"requestPostData"
"securityInfo"
"responseHeaders"
"responseCookies"
"responseStart"
"responseContent"
"eventTimings"

These are usually handled using webConsoleClient. Something like as follows:
this.webConsoleClient.on("networkEventUpdate", () => {});

The question is: Should the "responseContent" event update be always sent for every request?

What about other events are they always sent?
If not, how a test can determine that there is no other event coming and fetching data (from the backend) is finished?

Honza

Comment 6

[Alexandre Poirot [:ochameau]]

(In reply to Jan Honza Odvarko [:Honza] from comment #4)
> > But at the same time, I do not see FirefoxConnector being involved in this
> > test?
> >  
> > http://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/
> > connector/firefox-connector.js#48
> > At least I can't see it being instanciated?
> > And we would really miss its networkEvent/networkEventUpdate listeners...
> The new Console UI uses directly FirefoxDataProvider (not the generic
> connector)
> http://searchfox.org/mozilla-central/rev/
> b53e29293c9e9a2905f4849f4e3c415e2013f0cb/devtools/client/webconsole/new-
> console-output/store.js#173

I think that's your main issue... I don't think it has anything to do with backend.
Only the generic connector listen for networkEventUpdate[1] and you just said in comment 5 that you were missing them.

I think the overall integration of netmonitor panels into console is to be reviewed.

[1] http://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/connector/firefox-connector.js#50-51
    this.webConsoleClient.on("networkEventUpdate",
      this.dataProvider.onNetworkEventUpdate);

Comment 7

[Alexandre Poirot [:ochameau]]

Note that my work on lazy response content fetching may help here.

Comment 8

[Jan Honza Odvarko [:Honza] (always need-info? me)]

@Alex, thanks for the input.

@Nicolas, I am attaching a patch that is again making sure the response body is tested.
I figured the problem in bug 1404227 eventually.

The issue happened when a network log was opened too soon (can happen especially in a test) and the `message.updates` array (controlled by WebConsoleClient) isn't complete yet. In other words not all `networkEventUpdate` events has been received yet. So, it's important to listen for the rest of `networkEventUpdate` events (yes, especially the missing `responseContent`) and properly handle it

It happens here:
http://searchfox.org/mozilla-central/rev/1c1a5cef772214e9cff487040ac3068d56e96cc6/devtools/client/webconsole/new-console-output/store.js#200

And this part has been fixed in bug 1404227.

Honza

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: Bug 1406100 - Test also response body;

Review commit: https://reviewboard.mozilla.org/r/189392/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/189392/

Comment 10

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Try is green
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b731bbead6f46d4a734c5d769634cb9c2f33de72

Honza

Comment 11

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8918529 [details]
Bug 1406100 - Test also response body;

https://reviewboard.mozilla.org/r/189392/#review194736

Thanks honza !

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/74d7beed711a
Test also response body; r=nchevobbe

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/74d7beed711a