Should 404 codes be considered aborts? #165

npm1 · 2018-09-24T18:30:45Z

I'd like some clarification regarding whether ResourceTiming should consider this to be an abort or not. This is important due to the spec's current language saying that the user agent may or may not report entries for fetches that were initiated but then aborted. On a related note, it feels like we should just agree on a more strict specification: either aborts should be reported by all user agents or they should not be reported by any user agent.

igrigorik · 2018-11-25T04:02:53Z

What's the motivation for treating 404 as an abort? It's a valid and successful server response that should be surfaced to the application — e.g. an API request for a non-existent resource.

npm1 · 2018-11-26T18:47:08Z

That sounds reasonable to me. But then I think the spec needs to be clearer. Currently, we have "Resources for which the fetch was initiated, but was later aborted (e.g. due to a network error) MAY be included as PerformanceResourceTiming objects in the Performance Timeline and MUST contain initialized attribute values for processed substeps of the processing model."

So there's a couple of options here but either way this needs a spec change:

Keep aborts optional. In this case, there is a difference in how RT handles aborts vs non-aborts. This means we need to properly define what an abort is in this context. Then an implementor can identify that 404 is an error code that does not correspond to an abort and needs to be reported always.
Make aborts mandatory. This means removing the "MAY" keyword from the spec, so implementors can understand that all aborts should be reported as well. In this case we wouldn't need to define what an abort is since all cases must be reported.

igrigorik · 2018-11-27T07:40:38Z

For context, past related discussion(s): #12

The tl;dr is that the current wording is (unfortunately) intentionally vague due to differences in implementations, and our goal is to address this in L3. See the linked discussions and PR's in above thread for the related PR's and reverts where we pushed this to L3.

I propose we merge this into #39?

npm1 · 2018-11-27T21:18:21Z

I'd suggest we keep it separate to ensure it gets addressed when integrating with Fetch. As discussed in the call, it makes sense to consider this a L3 issue.

mikesalvia · 2019-09-09T18:20:54Z

According to a newer discussion, this will be changed in future specs.

#165
https://bugs.chromium.org/p/chromium/issues/detail?id=883400&can=4&q=&colspec=ID%20Pri%20M%20Stars%20ReleaseBlock%20Component%20Status%20Owner%20Summary%20OS%20Modified

Currently we don't report performance entries with failing status codes. From the spec's perspective, reporting aborts is a MAY, but failing status code responses should not be considered aborts. [1] Chromium is the only engine which doesn't report those entries. This CL fixes that to report them similarly to successful status codes. Bug: 883400, 990849 Change-Id: Ic5e99e3df77f3869aa0dd70f0141d88016fdb972 [1] w3c/resource-timing#165 (comment)

Currently we don't report performance entries with failing status codes. From the spec's perspective, reporting aborts is a MAY, but failing status code responses should not be considered aborts. [1] Chromium is the only engine which doesn't report those entries. This CL fixes that to report them similarly to successful status codes. Bug: 883400, 990849 Change-Id: Ic5e99e3df77f3869aa0dd70f0141d88016fdb972 [1] w3c/resource-timing#165 (comment) Change-Id: Ic5e99e3df77f3869aa0dd70f0141d88016fdb972

Currently we don't report performance entries with failing status codes. From the spec's perspective, reporting aborts is a MAY, but failing status code responses should not be considered aborts. [1] Chromium is the only engine which doesn't report those entries. This CL fixes that to report them similarly to successful status codes. Bug: 883400, 990849 Change-Id: Ic5e99e3df77f3869aa0dd70f0141d88016fdb972 [1] w3c/resource-timing#165 (comment) Change-Id: Ic5e99e3df77f3869aa0dd70f0141d88016fdb972 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1796544 Commit-Queue: Yoav Weiss <yoavweiss@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Cr-Commit-Position: refs/heads/master@{#695596}

yoavweiss · 2019-09-11T13:50:59Z

Just landed a Chromium change to expose resource entries regardless of status code

…ries with failing status codes, a=testonly Automatic update from web-platform-tests [resource-timing] Report performance entries with failing status codes Currently we don't report performance entries with failing status codes. From the spec's perspective, reporting aborts is a MAY, but failing status code responses should not be considered aborts. [1] Chromium is the only engine which doesn't report those entries. This CL fixes that to report them similarly to successful status codes. Bug: 883400, 990849 Change-Id: Ic5e99e3df77f3869aa0dd70f0141d88016fdb972 [1] w3c/resource-timing#165 (comment) Change-Id: Ic5e99e3df77f3869aa0dd70f0141d88016fdb972 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1796544 Commit-Queue: Yoav Weiss <yoavweiss@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Cr-Commit-Position: refs/heads/master@{#695596} -- wpt-commits: 807956fd31824995580dcf36661520452083f9ef wpt-pr: 18987

https://bugs.webkit.org/show_bug.cgi?id=202040 Patch by Alex Christensen <achristensen@webkit.org> on 2019-09-20 Reviewed by Joseph Pecoraro. LayoutTests/imported/w3c: * web-platform-tests/resource-timing/resource_ignore_failures-expected.txt: Removed. * web-platform-tests/resource-timing/resource_ignore_failures.html: Removed. This test is no longer in wpt and it would regress with this change, so we remove it. * web-platform-tests/resource-timing/resources/status-code.py: Added. (main): * web-platform-tests/resource-timing/status-codes-create-entry-expected.txt: Added. * web-platform-tests/resource-timing/status-codes-create-entry.html: Added. Source/WebCore: This follows a Chromium change at https://chromium-review.googlesource.com/c/chromium/src/+/1796544 The spec change is being discussed at w3c/resource-timing#165 Test: imported/w3c/web-platform-tests/resource-timing/status-codes-create-entry.html I had to slightly modify the test to make sure the entry count was > 0 instead of == 1 to reduce flakyness because sometimes we load the 200 image twice. I'll submit a PR to WPT, too. * loader/ResourceTimingInformation.cpp: (WebCore::ResourceTimingInformation::shouldAddResourceTiming): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@250167 268f45cc-cd09-0410-ab3c-d52691b4dbfc

yoavweiss · 2019-09-29T08:34:39Z

Seems like this was fixed for WebKit as well. Closing.

…ries with failing status codes, a=testonly Automatic update from web-platform-tests [resource-timing] Report performance entries with failing status codes Currently we don't report performance entries with failing status codes. From the spec's perspective, reporting aborts is a MAY, but failing status code responses should not be considered aborts. [1] Chromium is the only engine which doesn't report those entries. This CL fixes that to report them similarly to successful status codes. Bug: 883400, 990849 Change-Id: Ic5e99e3df77f3869aa0dd70f0141d88016fdb972 [1] w3c/resource-timing#165 (comment) Change-Id: Ic5e99e3df77f3869aa0dd70f0141d88016fdb972 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1796544 Commit-Queue: Yoav Weiss <yoavweisschromium.org> Reviewed-by: Yutaka Hirano <yhiranochromium.org> Reviewed-by: Mike West <mkwstchromium.org> Cr-Commit-Position: refs/heads/master{#695596} -- wpt-commits: 807956fd31824995580dcf36661520452083f9ef wpt-pr: 18987 UltraBlame original commit: 4505f6c2d2041449bde579b4909491f8e0c97047

…s codes Currently we don't report performance entries with failing status codes. From the spec's perspective, reporting aborts is a MAY, but failing status code responses should not be considered aborts. [1] Chromium is the only engine which doesn't report those entries. This CL fixes that to report them similarly to successful status codes. Bug: 883400, 990849 Change-Id: Ic5e99e3df77f3869aa0dd70f0141d88016fdb972 [1] w3c/resource-timing#165 (comment) (cherry picked from commit 5e556dd) Change-Id: Ic5e99e3df77f3869aa0dd70f0141d88016fdb972 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1796544 Commit-Queue: Yoav Weiss <yoavweiss@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#695596} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1829742 Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/branch-heads/3865@{#857} Cr-Branched-From: 0cdcc61-refs/heads/master@{#681094}

[M77] [resource-timing] Report performance entries with failing status codes Currently we don't report performance entries with failing status codes. From the spec's perspective, reporting aborts is a MAY, but failing status code responses should not be considered aborts. [1] Chromium is the only engine which doesn't report those entries. This CL fixes that to report them similarly to successful status codes. Bug: 883400, 990849 Change-Id: Ic5e99e3df77f3869aa0dd70f0141d88016fdb972 [1] w3c/resource-timing#165 (comment) (cherry picked from commit 5e556dd80e03b7a217e10990d71be25d07e1ece7) Change-Id: Ic5e99e3df77f3869aa0dd70f0141d88016fdb972 Commit-Queue: Yoav Weiss <yoavweiss@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#695596} Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/branch-heads/3865@{#857} Cr-Branched-From: 0cdcc6158160790658d1f033d3db873603250124-refs/heads/master@{#681094} Reviewed-by: Michal Klocek <michal.klocek@qt.io>

…s codes Currently we don't report performance entries with failing status codes. From the spec's perspective, reporting aborts is a MAY, but failing status code responses should not be considered aborts. [1] Chromium is the only engine which doesn't report those entries. This CL fixes that to report them similarly to successful status codes. Bug: 883400, 990849 Change-Id: Ic5e99e3df77f3869aa0dd70f0141d88016fdb972 [1] w3c/resource-timing#165 (comment) (cherry picked from commit 5e556dd) Change-Id: Ic5e99e3df77f3869aa0dd70f0141d88016fdb972 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1796544 Commit-Queue: Yoav Weiss <yoavweiss@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#695596} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1805723 Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/branch-heads/3904@{chromium#216} Cr-Branched-From: 675968a-refs/heads/master@{#693954}

Manual backport. [M77] [resource-timing] Report performance entries with failing status codes Currently we don't report performance entries with failing status codes. From the spec's perspective, reporting aborts is a MAY, but failing status code responses should not be considered aborts. [1] Chromium is the only engine which doesn't report those entries. This CL fixes that to report them similarly to successful status codes. Bug: 883400, 990849 Change-Id: Ic5e99e3df77f3869aa0dd70f0141d88016fdb972 [1] w3c/resource-timing#165 (comment) Commit-Queue: Yoav Weiss <yoavweiss@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#695596} Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/branch-heads/3865@{#857} Cr-Branched-From: 0cdcc6158160790658d1f033d3db873603250124-refs/heads/master@{#681094} Reviewed-by: Michal Klocek <michal.klocek@qt.io> Change-Id: I5251942aa8061c3e93a4ae5a664fa81034df5395

https://bugs.webkit.org/show_bug.cgi?id=202040 Patch by Alex Christensen <achristensen@webkit.org> on 2019-09-20 Reviewed by Joseph Pecoraro. LayoutTests/imported/w3c: * web-platform-tests/resource-timing/resource_ignore_failures-expected.txt: Removed. * web-platform-tests/resource-timing/resource_ignore_failures.html: Removed. This test is no longer in wpt and it would regress with this change, so we remove it. * web-platform-tests/resource-timing/resources/status-code.py: Added. (main): * web-platform-tests/resource-timing/status-codes-create-entry-expected.txt: Added. * web-platform-tests/resource-timing/status-codes-create-entry.html: Added. Source/WebCore: This follows a Chromium change at https://chromium-review.googlesource.com/c/chromium/src/+/1796544 The spec change is being discussed at w3c/resource-timing#165 Test: imported/w3c/web-platform-tests/resource-timing/status-codes-create-entry.html I had to slightly modify the test to make sure the entry count was > 0 instead of == 1 to reduce flakyness because sometimes we load the 200 image twice. I'll submit a PR to WPT, too. * loader/ResourceTimingInformation.cpp: (WebCore::ResourceTimingInformation::shouldAddResourceTiming): Canonical link: https://commits.webkit.org/215668@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@250167 268f45cc-cd09-0410-ab3c-d52691b4dbfc

npm1 changed the title ~~Should 404 codes by considered aborts?~~ Should 404 codes be considered aborts? Nov 26, 2018

yoavweiss added this to the Level 3 milestone Nov 27, 2018

yoavweiss added the requires Fetch integration label Nov 28, 2018

chromium-wpt-export-bot mentioned this issue Sep 11, 2019

[resource-timing] Report performance entries with failing status codes web-platform-tests/wpt#18987

Merged

yoavweiss closed this as completed Sep 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Should 404 codes be considered aborts? #165

Should 404 codes be considered aborts? #165

npm1 commented Sep 24, 2018

igrigorik commented Nov 25, 2018

npm1 commented Nov 26, 2018

igrigorik commented Nov 27, 2018

npm1 commented Nov 27, 2018

mikesalvia commented Sep 9, 2019

yoavweiss commented Sep 11, 2019

yoavweiss commented Sep 29, 2019

Should 404 codes be considered aborts? #165

Should 404 codes be considered aborts? #165

Comments

npm1 commented Sep 24, 2018

igrigorik commented Nov 25, 2018

npm1 commented Nov 26, 2018

igrigorik commented Nov 27, 2018

npm1 commented Nov 27, 2018

mikesalvia commented Sep 9, 2019

yoavweiss commented Sep 11, 2019

yoavweiss commented Sep 29, 2019