-
Notifications
You must be signed in to change notification settings - Fork 341
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clarify which network errors create a resource timing entry #1215
Comments
See comment: #1202 (comment) |
One simplified option would be that the caller is responsible to report the RT entry in case of a network/CORS error. Not sure if we want to expose connection info and other things when there was a network error, probably not. |
Yeah, if we report network errors we want to act as if they are cross-origin and lack TAO. We could store the relevant information on the network error for the caller (it's a type of response after all, albeit rather special). That's my option 1. Option 2 would be reporting all responses that are not network errors. Option 3 would be reporting all responses that the caller considers actionable (i.e., would exclude non-2xx in case of They are not in a preference order. I don't really know what would help web developers the most. Whichever option we decide to go with should not provide a new information channel. |
I noticed another problem, due to network errors having an empty URL list, finalize and report timing always returns early. So as currently specified option 1 does not work. |
We briefly discussed this in the WebPerf WG call today, it wasn't clear to me if there's a use case for entries from network errors. Also @nicjansma pointed out that there's already overlap with the Reporting API. |
You mean with https://w3c.github.io/network-error-logging/? That's true. If everyone is fine to align on no entries for network errors, let's do that. |
And hand processResponseDone a response. Resource Timing PR: w3c/resource-timing#266. Closes #1201. Follow-up: #1215.
@yoavweiss what's needed to reach consensus/conclusions on this? |
While there is overlap with the Reporting API (Network Error Logging), I think there's value in keeping network-level errors, since the ergonomics and delivery are different (e.g. out-of-band reports). If we're reporting network-level errors via Reporting API, is there still concern about surfacing them through ResourceTiming as well (which is what most browsers are doing today)? This is a lot of what the issue w3c/resource-timing#12 was about, getting consistency on network-level and non-200 errors among the browsers. I just did a quick refresh to see where browsers are today, and except for Safari, network-level errors are reported everywhere else: w3c/resource-timing#12 (comment) What we have today is aligned closer with @annevk 's Option 1, "... to act as if they are cross-origin and lack TAO". The entries exist (except in Safari), but have all breakdown timestamps zero'd. Just fetchStart/responseEnd/duration are set. |
I also find that option the most sensible. |
Anne's suggestion seems reasonable, but that still makes it easier to distinguish a fast network error from a timeout. Relatedly, should we also add an entry as if it were cross-origin and lacking TAO if there is no network error but the request times out? |
@achristensen07 - We could add an entry in such a case, but it won't be difficult to guess it's a timeout, would it?
@annevk - that sounds overall reasonable to me as well, but I'd like for us to spell out the reasoning. Since cross-origin network errors lack TAO anyway, I'm guessing you also want same-origin network errors to not expose details. Is that because you're concerned they may expose details on the user's network? Or some other reason? |
Yeah, basically until you have established a TLS connection, you don't really know if it's same-origin or not. And post-connection network errors are always about policy enforcement and we do not want to surface that to the caller either. I think a system timeout should be the same as a network error, information-wise. |
Have we reached consensus on this, @yoavweiss? |
I believe we have agreement to expose all network errors as if they are cross-origin with no TAO. |
Closing the issue, as it doesn't have direct impact on the FETCH spec at this point, but rather on its callers. |
I don't think that's correct. We'd have to at least start storing some information on network errors related to timing. |
I don't believe so. The start time is stored on the request, and the end time is the time of calling the report function. But we can leave this open until we have the first reporter of network error (I guess that would be the fetch API, which we can track here). |
I'm pretty sure we don't store anything about timing on requests, except for the timing allow failed flag bookkeeping detail. |
An initial test for whatwg/fetch#1215
Also, report timing info for network errors for the fetch() APIs. The timing info attached to a network error is always "opaque", containing only start/end time and the original request URL. This change currently only applies to the fetch API, and should be applied to the different callers of FETCCH as part of [this](whatwg/html#6542) and [this](whatwg/xhr#319) work. Closes whatwg#1215
This is needed for whatwg/html#7104 and #1215.
Set an opaque timing info and the original request URL to the error resposne, and call the processResponseDone callback to allow the caller to report it. This should already be the case for reporting network errors in the fetch() API. Closes whatwg#1215
Set an opaque timing info and the original request URL to the error resposne, and call the processResponseDone callback to allow the caller to report it. This should already be the case for reporting network errors in the fetch() API. Closes whatwg#1215
Set an opaque timing info and the original request URL to the error resposne, and call the processResponseDone callback to allow the caller to report it. This should already be the case for reporting network errors in the fetch() API. Closes whatwg#1215
Set an opaque timing info and the original request URL to the error resposne, and call the processResponseDone callback to allow the caller to report it. This should already be the case for reporting network errors in the fetch() API. Closes whatwg#1215
Set an opaque timing info and the original request URL to the error resposne, and call the processResponseDone callback to allow the caller to report it. This should already be the case for reporting network errors in the fetch() API. Closes whatwg#1215 Set network error timing info and URL at the start of fetch finale Rearrange fetch finale step Make sure we don't report network errors twice Use queues for synchronization
An initial test for whatwg/fetch#1215
Set an opaque timing info and the original request URL to the error resposne, and call the processResponseDone callback to allow the caller to report it. This should already be the case for reporting network errors in the fetch() API. Closes whatwg#1215 Set network error timing info and URL at the start of fetch finale Rearrange fetch finale step Make sure we don't report network errors twice Use queues for synchronization
* Create resource timing entries for fetch network errors An initial test for whatwg/fetch#1215 * Remove a few ambiguous errors * Remove timeout scenario * Remove unnecessary lines * Added mixed content case * Try to make test less flaky * Make test not flaky * Use attribute_test * lint
Set an opaque timing info and the original request URL to the error resposne, and call the processResponseDone callback to allow the caller to report it. This should already be the case for reporting network errors in the fetch() API. Closes whatwg#1215 Set network error timing info and URL at the start of fetch finale Rearrange fetch finale step Make sure we don't report network errors twice Use queues for synchronization
… network errors, a=testonly Automatic update from web-platform-tests Create resource timing entries for fetch network errors (#30970) * Create resource timing entries for fetch network errors An initial test for whatwg/fetch#1215 * Remove a few ambiguous errors * Remove timeout scenario * Remove unnecessary lines * Added mixed content case * Try to make test less flaky * Make test not flaky * Use attribute_test * lint -- wpt-commits: 029d8d6672ceaed11b0cfef841621ece520a78ec wpt-pr: 30970
… network errors, a=testonly Automatic update from web-platform-tests Create resource timing entries for fetch network errors (#30970) * Create resource timing entries for fetch network errors An initial test for whatwg/fetch#1215 * Remove a few ambiguous errors * Remove timeout scenario * Remove unnecessary lines * Added mixed content case * Try to make test less flaky * Make test not flaky * Use attribute_test * lint -- wpt-commits: 029d8d6672ceaed11b0cfef841621ece520a78ec wpt-pr: 30970
- Some tests in wpt/preload use Resource Timing entries to make sure that no requests are made. We're changing that (Resource Timing entries should be created even when blocked by CSP - see whatwg/fetch#1215). Stop using Resource Timing entries and check that with server side scripts. - http/tests/preload/preload-csp.html is covered by some WPTs. Let's remove it. Change-Id: I3c2cdfa2459d212657be7569c5290c48b39d6f05 Bug: 1275564
- Some tests in wpt/preload use Resource Timing entries to make sure that no requests are made. We're changing that (Resource Timing entries should be created even when blocked by CSP - see whatwg/fetch#1215). Stop using Resource Timing entries and check that with server side scripts. - http/tests/preload/preload-csp.html is covered by some WPTs. Let's remove it. Change-Id: I3c2cdfa2459d212657be7569c5290c48b39d6f05 Bug: 1275564
- Some tests in wpt/preload use Resource Timing entries to make sure that no requests are made. We're changing that (Resource Timing entries should be created even when blocked by CSP - see whatwg/fetch#1215). Stop using Resource Timing entries and check that with server side scripts. - http/tests/preload/preload-csp.html is covered by some WPTs. Let's remove it. Change-Id: I3c2cdfa2459d212657be7569c5290c48b39d6f05 Bug: 1275564
- Some tests in wpt/preload use Resource Timing entries to make sure that no requests are made. We're changing that (Resource Timing entries should be created even when blocked by CSP - see whatwg/fetch#1215). Stop using Resource Timing entries and check that with server side scripts. - http/tests/preload/preload-csp.html is covered by some WPTs. Let's remove it. Change-Id: I3c2cdfa2459d212657be7569c5290c48b39d6f05 Bug: 1275564 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3708287 Reviewed-by: Yoav Weiss <[email protected]> Commit-Queue: Yutaka Hirano <[email protected]> Cr-Commit-Position: refs/heads/main@{#1019490}
- Some tests in wpt/preload use Resource Timing entries to make sure that no requests are made. We're changing that (Resource Timing entries should be created even when blocked by CSP - see whatwg/fetch#1215). Stop using Resource Timing entries and check that with server side scripts. - http/tests/preload/preload-csp.html is covered by some WPTs. Let's remove it. Change-Id: I3c2cdfa2459d212657be7569c5290c48b39d6f05 Bug: 1275564 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3708287 Reviewed-by: Yoav Weiss <[email protected]> Commit-Queue: Yutaka Hirano <[email protected]> Cr-Commit-Position: refs/heads/main@{#1019490}
- Some tests in wpt/preload use Resource Timing entries to make sure that no requests are made. We're changing that (Resource Timing entries should be created even when blocked by CSP - see whatwg/fetch#1215). Stop using Resource Timing entries and check that with server side scripts. - http/tests/preload/preload-csp.html is covered by some WPTs. Let's remove it. Change-Id: I3c2cdfa2459d212657be7569c5290c48b39d6f05 Bug: 1275564 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3708287 Reviewed-by: Yoav Weiss <[email protected]> Commit-Queue: Yutaka Hirano <[email protected]> Cr-Commit-Position: refs/heads/main@{#1019490}
Automatic update from web-platform-tests Fix up wpt/preload - Some tests in wpt/preload use Resource Timing entries to make sure that no requests are made. We're changing that (Resource Timing entries should be created even when blocked by CSP - see whatwg/fetch#1215). Stop using Resource Timing entries and check that with server side scripts. - http/tests/preload/preload-csp.html is covered by some WPTs. Let's remove it. Change-Id: I3c2cdfa2459d212657be7569c5290c48b39d6f05 Bug: 1275564 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3708287 Reviewed-by: Yoav Weiss <[email protected]> Commit-Queue: Yutaka Hirano <[email protected]> Cr-Commit-Position: refs/heads/main@{#1019490} -- wpt-commits: ba22f229dfafa51c637aa02957f8b9f330f1cfa3 wpt-pr: 34462
Automatic update from web-platform-tests Fix up wpt/preload - Some tests in wpt/preload use Resource Timing entries to make sure that no requests are made. We're changing that (Resource Timing entries should be created even when blocked by CSP - see whatwg/fetch#1215). Stop using Resource Timing entries and check that with server side scripts. - http/tests/preload/preload-csp.html is covered by some WPTs. Let's remove it. Change-Id: I3c2cdfa2459d212657be7569c5290c48b39d6f05 Bug: 1275564 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3708287 Reviewed-by: Yoav Weiss <[email protected]> Commit-Queue: Yutaka Hirano <[email protected]> Cr-Commit-Position: refs/heads/main@{#1019490} -- wpt-commits: ba22f229dfafa51c637aa02957f8b9f330f1cfa3 wpt-pr: 34462
- Some tests in wpt/preload use Resource Timing entries to make sure that no requests are made. We're changing that (Resource Timing entries should be created even when blocked by CSP - see whatwg/fetch#1215). Stop using Resource Timing entries and check that with server side scripts. - http/tests/preload/preload-csp.html is covered by some WPTs. Let's remove it. Change-Id: I3c2cdfa2459d212657be7569c5290c48b39d6f05 Bug: 1275564 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3708287 Reviewed-by: Yoav Weiss <[email protected]> Commit-Queue: Yutaka Hirano <[email protected]> Cr-Commit-Position: refs/heads/main@{#1019490} NOKEYCHECK=True GitOrigin-RevId: aae9fb0d93e91afcc6ad38ea668bcc9f905f0fe6
It's not clear from the spec/implementation which errors should or shouldn't create an RT entry.
CORS errors explicitly don't, though in the original Resource Timing spec they should create an RT entry. However, the discrepancy between different error types might create a case where it could be detected whether an error is of CORS nature, which should be avoided.
See #1202 (review)
The text was updated successfully, but these errors were encountered: