1916356 - responseStatus on PerformanceResourceTimings is sometimes wrong

Status: RESOLVED FIXED

(Core :: DOM: Performance APIs, defect)

Description

[simon.schatka]

Attachment: Wrong responseStatus

Steps to reproduce:

We have automatic tests that started to fail with Firefox 129.
What we are doing:

• Make an XHR to an endpoint that takes 3 seconds to respond
• Abort the XHR after 1 second (we also have one that uses the timeout property)
• Check the resourcetimings information for this request.

Actual results:

The responseStatus for this request shows 70 (in CI it shows 25) on the resourcetimings information (performance.getEntriesByType("resource"))

Expected results:

responseStatus should show 0

Comment 1

[simon.schatka]

An additional find I made:
We have a test that infinitely redirects. This will then be stopped by Firefox after a certain amount of redirects (didn't count them, maybe around 20?), which is fine and expected.
The responseStatus in this case is 302. I would rather expect it to be 0 as well, as the request got aborted.

Comment 2

[BugBot [:suhaib / :marco]]

The Bugbug bot thinks this bug should belong to the 'Core::DOM: Networking' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[Mayank Bansal]

Can you:

1. Attach a testcase on this bug that reproduces the issue for you?
2. Type "about:support" in the Firefox browser and paste its contents to this bug?
3. If the usecase worked fine for you before Firefox V129, can you use mozregression to do a bisection to narrow down the change that caused this issue? (Link: https://mozilla.github.io/mozregression/)

Comment 4

[simon.schatka]

Attachment: xhrabort.html

attached a reproducer file

Comment 5

[simon.schatka]

Attachment: about_Support.txt

attached output from about:support

Comment 6

[simon.schatka]

I wanted to be smart and replace our url ("/slow/3000/xhrabort.html") in the XHR to use something that is generally available, but turns out that fixes the behavior. Might have to do with localhost URLs or with the way we are slowing down requests.

I ran mozregression and it gave me this:
2024-09-04T16:11:50.116000: INFO : Narrowed integration regression window from [38b98882, e17d8ab7] (3 builds) to [1bfb0710, e17d8ab7] (2 builds) (~1 steps left)
2024-09-04T16:11:50.137000: DEBUG : Starting merge handling...
2024-09-04T16:11:50.137000: DEBUG : Using url: https://hg.mozilla.org/releases/mozilla-release/json-pushes?changeset=e17d8ab7474d5f73f30c33d6e2cb822e49a6d853&full=1
2024-09-04T16:11:50.138000: DEBUG : redo: attempt 1/3
2024-09-04T16:11:50.144000: DEBUG : redo: retry: calling _default_get with args: ('https://hg.mozilla.org/releases/mozilla-release/json-pushes?changeset=e17d8ab7474d5f73f30c33d6e2cb822e49a6d853&full=1',), kwargs: {}, attempt #1
2024-09-04T16:11:50.152000: DEBUG : urllib3.connectionpool: Resetting dropped connection: hg.mozilla.org
2024-09-04T16:11:52.057000: DEBUG : urllib3.connectionpool: https://hg.mozilla.org:443 "GET /releases/mozilla-release/json-pushes?changeset=e17d8ab7474d5f73f30c33d6e2cb822e49a6d853&full=1 HTTP/11" 200 None
2024-09-04T16:11:56.267000: DEBUG : Found commit message:
Update configs. IGNORE BROKEN CHANGESETS CLOSED TREE NO BUG a=release ba=release

2024-09-04T16:11:56.267000: DEBUG : Did not find a branch, checking all integration branches
2024-09-04T16:11:56.273000: INFO : The bisection is done.
2024-09-04T16:11:56.276000: INFO : Stopped

From what I can see, it did not break at a special point, the initial implementation just is not correct. The Firefox versions 128 do not contain the responseStatus property, versions 129 contain it for the first time.

Lastly, I have also seen responseStatus codes like 208 and something above 32k, so it really is all over the place.

Comment 7

[Tooru Fujisawa [:arai]]

I suppose it's caused by the following code:

https://searchfox.org/mozilla-central/rev/cc01f11adfacca9cd44a75fd140d2fdd8f9a48d4/dom/performance/PerformanceTiming.cpp#300-302

uint32_t responseStatus;
Unused << aHttpChannel->GetResponseStatus(&responseStatus);
mResponseStatus = static_cast<uint16_t>(responseStatus);

where the implementation of GetResponseStatus is the following:

https://searchfox.org/mozilla-central/rev/cc01f11adfacca9cd44a75fd140d2fdd8f9a48d4/netwerk/protocol/http/HttpBaseChannel.cpp#2240-2243

HttpBaseChannel::GetResponseStatus(uint32_t* aValue) {
  if (!mResponseHead) return NS_ERROR_NOT_AVAILABLE;
  *aValue = mResponseHead->Status();
  return NS_OK;

So, if mResponseHead is falsy, responseStatus is left uninitialized.

sefeng, can you take a look?

Comment 8

[Sean Feng [:sefeng211]]

Thanks Tooro, that sounds right!

Tom, do you mind to fix this? Thanks!

Comment 9

[simon.schatka]

any updates on this?

Comment 10

[Tooru Fujisawa [:arai]]

Attachment: Bug 1916356 - Properly initialize the response status value even if the response head is null. r?sefeng!

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/da6165abc794
Properly initialize the response status value even if the response head is null. r=sefeng

Comment 12

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/da6165abc794