<a class="header-button" href="https://bugzilla.mozilla.org/home" title="Go to home page"> Bugzilla

Comment 2

•

5 years ago

Thanks Harald for the report!

Some instructions:

The client side calls this method to ask the backend to resend specific request
https://searchfox.org/mozilla-central/rev/153feabebc2d13bb4c29ef8adf104ec1ebd246ae/devtools/client/netmonitor/src/connector/firefox-connector.js#287
This is the backend piece that does the actual resending
https://searchfox.org/mozilla-central/rev/153feabebc2d13bb4c29ef8adf104ec1ebd246ae/devtools/server/actors/webconsole.js#1834

We need to check both places for the missing headers and see whether this is a problem with DevTools code or the platform

Honza

Flags: needinfo?(odvarko)

Keywords: good-first-bug

Updated

•

5 years ago

Blocks: netmonitor-edit-and-resend

Comment 3

•

5 years ago

Hi Jan/Herald! I'd like to take a look at this and see if I can fix this. This would be my first contribution here.

Comment 4

•

5 years ago

(In reply to Shobhit Chittora from comment #3)

Hi Jan/Herald! I'd like to take a look at this and see if I can fix this. This would be my first contribution here.

OK, sounds great!

There are some instructions in comment #2 so, that should help you to know where to start.
Also you might want to read:
https://docs.firefox-dev.tools/getting-started/

Please post any of your findings here in the report.
Also use Request information from field (at the bottom of this page) when you have questions.

Honza

Hemakumar

Comment 5

•

5 years ago

Hi, i couldn't able to reproduce this issue any more. Is this still exist?

Comment 6

•

5 years ago

Attached image image.png — Details

Yes, I can still reproduce the issue. See my screenshot

the first request is 200
the second one (resent) is 415 - Unsupported Media Type

Honza

Hemakumar

Comment 7

•

5 years ago

hmm. Please find the attached screenshots. Am i missing something. I am using firefox 69 version with MacOS Sierra.

Comment 8

•

5 years ago

@chittorashobhit@gmail.com: can you reproduce the issue?

Honza

Flags: needinfo?(chittorashobhit)

Comment 9

•

5 years ago

@chittorashobhit@gmail.com: can you reproduce the issue?

Couldn't reproduce this on the first try. Steps I tried

In new tab made an XHR in the console.
Went to network tab to resend the same.

System detail -
OS - MacOS mojave 10.14
Firefox - 69.0.1

Flags: needinfo?(chittorashobhit)

Comment 10

•

5 years ago

Attached image Screenshot 2019-09-26 at 4.33.01 PM.png — Details

Adding my result of trying to reproduce this.

Comment 11

•

5 years ago

@chittorashobhit@gmail.com: can you reproduce the issue?

Do you have any other browser specific setting enabled which might lead to this?

Comment 12

•

5 years ago

(In reply to Shobhit Chittora from comment #9)

Firefox - 69.0.1

Can you please try Firefox Nightly? Fx71
https://www.mozilla.org/en-US/firefox/nightly/all/

Honza

Flags: needinfo?(chittorashobhit)

Comment 13

•

5 years ago

Attached image Screenshot 2019-09-26 at 5.00.10 PM.png — Details

Diff of the headers in requests.

Flags: needinfo?(chittorashobhit)

Comment 14

•

5 years ago

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #12)

(In reply to Shobhit Chittora from comment #9)

Firefox - 69.0.1

Can you please try Firefox Nightly? Fx71
https://www.mozilla.org/en-US/firefox/nightly/all/

Honza

Hi Honza! My bad! I was trying something else. I can totally reproduce this in Fx71.

Comment 15

•

5 years ago

Hi Honza Odvarko [:Honza],

I've tried looking at the root cause of this. I peeked in the file below ->
request .js -> https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/actions/requests.js#93

I tried to resend a request, which ( as per my investigation ) is done using the the action below ->
api.js -> https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/api.js#231

This looks for a requestID, which is our first request. It then looks for the requestID in the redux store to using the selector below ->
requests.js -> https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/selectors/requests.js#164

On inspecting this retrieved request from the store, I don't see the property we're looking for i.e. request.requestHeaders.headers

Also attaching below a sample request retrieved from the store.

{
  "id": "server0.conn0.netEvent4",
  "startedMs": 1569543578970,
  "method": "GET",
  "url": "https://api.github.com/search/topics?q=firefox",
  "isXHR": true,
  "cause": {
    "type": "fetch",
    "loadingDocumentUri": "about:home",
    "stacktraceAvailable": true
  },
  "stacktrace": "(void 0)",
  "fromCache": "(void 0)",
  "fromServiceWorker": "(void 0)",
  "isThirdPartyTrackingResource": false,
  "referrerPolicy": "no-referrer-when-downgrade",
  "blockedReason": "(void 0)",
  "channelId": 342442037477378,
  "urlDetails": {
    "baseNameWithQuery": "topics?q=firefox",
    "host": "api.github.com",
    "scheme": "https",
    "unicodeUrl": "https://api.github.com/search/topics?q=firefox",
    "isLocal": null,
    "url": "https://api.github.com/search/topics?q=firefox"
  },
  "requestHeadersAvailable": true,
  "requestCookiesAvailable": true,
  "httpVersion": "HTTP/1.1",
  "remoteAddress": "13.234.168.60",
  "remotePort": 443,
  "status": "200",
  "statusText": "OK",
  "headersSize": 1109,
  "securityState": "secure",
  "isRacing": "(void 0)",
  "securityInfoAvailable": true,
  "responseHeadersAvailable": true,
  "responseCookiesAvailable": true,
  "totalTime": 429,
  "eventTimingsAvailable": true,
  "contentSize": 12690,
  "transferredSize": 3501,
  "mimeType": "application/json; charset=utf-8",
  "responseContentAvailable": true
}

Hope this is the right direction to keep looking for this? Thanks for any and all help.

Comment 16

•

5 years ago

Also I see a lot of (void 0) in the request object. Why is that so? Are these values coming from the c++ objects or something like that?

Comment 17

•

5 years ago

Attached patch temp.diff — Details — Splinter Review

(In reply to Shobhit Chittora from comment #15)

On inspecting this retrieved request from the store, I don't see the property we're looking for i.e. request.requestHeaders.headers

Ah, that's the problem. Thanks for analysis!

The issue is that we are fetching headers on demand i.e. when the user actually want to see them in the Headers side panel. It's fetched here:
https://searchfox.org/mozilla-central/rev/f1e99da78fe6c3c68696358dac06aed90f8112d3/devtools/client/netmonitor/src/components/HeadersPanel.js#115

When cloning the request we need to do the same (even if the Headers panel wasn't opened yet)
Also, when cloning the request we need to also clone the requestHeadersAvailable field that indicates that headers are ready to be fetched from the backed.

Please see my patch, understand, add comments if you find them useful and upload to Phab

(In reply to Shobhit Chittora from comment #16)

Also I see a lot of (void 0) in the request object. Why is that so? Are these values coming from the c++ objects or something like that?

Some field can be undefined, that's ok.

This also needs a test.

The test can be very similar to what we have
https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/test/browser_net_resend_xhr.js#7

But, the test should not click on the original request (the one being resent)
https://searchfox.org/mozilla-central/rev/f1e99da78fe6c3c68696358dac06aed90f8112d3/devtools/client/netmonitor/test/browser_net_resend_xhr.js#22

Clicking causes the Headers panel to open and fetch the Headers automatically

You can extend the request and do another resend without clicking on the original request and checking that expected header is there.

Honza

Updated

•

5 years ago

Assignee: nobody → chittorashobhit

Status: NEW → ASSIGNED

Has STR: --- → yes

Comment 18

•

5 years ago

Thanks Honza!
Understood the changes in the patch you've posted. Let me check the changes once more and write test around it. Thanks for this.

Comment 19

•

5 years ago

Hey Honza! Why is a reload / re-select of the request required in your patch.

// Reload the request from the store to get the headers.
    request = getRequestById(getState(), request.id);

Comment 20

•

5 years ago

(In reply to Shobhit Chittora from comment #19)

Hey Honza! Why is a reload / re-select of the request required in your patch.
// Reload the request from the store to get the headers.
    request = getRequestById(getState(), request.id);

This is because we use immutable Redux store. That means the the request variable (referencing the entire request object) is not modified and still points to (immutable) data/request. So, we need to get the request from the store again (using getRequestById) to get new reference pointing to the new data (which contains the headers)

You can read about Redux immutability here:
https://redux.js.org/faq/immutable-data

Honza

Comment 21

•

5 years ago

Also Honza! The test you pointed at is failing after our patch due to the below code awaits on fetching the headers.

   // Fetch request headers and post data from the backend.
   await fetchNetworkUpdatePacket(connector.requestData, request, [
     "requestHeaders",
     "requestPostData",
   ]);

devtools/client/netmonitor/test/browser_net_resend_xhr.js
  FAIL A promise chain failed to handle a rejection: Connection closed, pending request to server0.conn0.child1/consoleActor2, type sendHTTPRequest failed

Comment 22

•

5 years ago

Thanks for the update. Can you please look at the failure and fix that test?
Honza

Flags: needinfo?(chittorashobhit)

Patrick Brosset <:pbro>

Comment 23

•

5 years ago

Unassigning the bug for now as it looks like Shobhit isn't working on it anymore. Please feel free to claim it again should you resume work on this.

Assignee: chittorashobhit → nobody

Status: ASSIGNED → NEW

Flags: needinfo?(chittorashobhit)

Assignee

Comment 24

•

5 years ago

Attached file Bug 1583397 - Ensures headers are fetched before trying to resend request. r=Honza — Details

Phabricator Automation

Updated

•

5 years ago

Assignee: nobody → thiago.arrais

Status: NEW → ASSIGNED

Comment 25

•

5 years ago

Just Note about the STR from comment #0. It needs to be done in the Console panel.

If you open the Details sidebar for the original request (the Network panel) or if you expand the request in the Console before resending - it'll work.

Honza

Arthur Iakab [arthur_iakab]

Assignee

Comment 26

•

5 years ago

Attached file Bug 1583397 - Part 2/: Format as per linting (obsolete) — Details

Depends on D61381

Phabricator Automation

Updated

•

5 years ago

Attachment #9123870 - Attachment description: Bug 1583397 - Part 1/: Ensures headers are fetched before trying to resend request. r=Honza → Bug 1583397 - Ensures headers are fetched before trying to resend request. r=Honza

Phabricator Automation

Updated

•

5 years ago

Attachment #9125573 - Attachment is obsolete: true

Pulsebot

Comment 27

•

5 years ago

Pushed by jodvarko@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/44c99468c2fc Ensures headers are fetched before trying to resend request. r=Honza

Comment 28

•

5 years ago

Backed out changeset 44c99468c2fc (bug 1583397) for causing devtools failures on browser_net_resend_cors.js.

Backout revision https://hg.mozilla.org/integration/autoland/rev/52311fe6f39a2ba3554f703b92db59b2646ab5a5

Failed push https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=44c99468c2fcf40ef0b82d8dc18e208574bb154b&searchStr=devtools&selectedJob=288694083

Failure logs https://treeherder.mozilla.org/logviewer.html#?job_id=288694083&repo=autoland

Thiago can you please take a look?

Flags: needinfo?(thiago.arrais)

Comment 29

•

5 years ago

Thiago, this test seems to be failing
devtools/client/netmonitor/test/browser_net_resend_cors.js

Honza

Assignee

Comment 31

•

5 years ago

Yup. I am aware of that. But I unfortunately can't find a way to fix it. Any ideas?

By the way, I had pointed that out in https://phabricator.services.mozilla.com/D61381#1905363 . But that didn't block the review. What should I have done differently?

Flags: needinfo?(thiago.arrais) → needinfo?(odvarko)

Bogdan Tara[:bogdan_tara | bogdant]

Assignee

Comment 32

•

5 years ago

I've managed to fix browser_net_resend_cors.js. It is no longer failing, but now I'm not really sure it tests what it should. Please review accordingly.

Pulsebot

Comment 33

•

5 years ago

Pushed by jodvarko@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9ed20f401a70 Ensures headers are fetched before trying to resend request. r=Honza

Comment 34

•

5 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/9ed20f401a70

Status: ASSIGNED → RESOLVED

Closed: 5 years ago

status-firefox75: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → Firefox 75

Updated

•

5 years ago

Flags: needinfo?(odvarko)