New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 801669 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
On parental leave until 3/15/19
Closed: Mar 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Provide more detailed QUIC errors in har files and for failed page loads

Project Member Reported by ianswett@google.com, Jan 12 2018

Issue description

Chrome Version: (copy from chrome://version)
OS: (e.g. Win7, OSX 10.9.5, etc...)

What steps will reproduce the problem?
(1) Produce a NET_QUIC_PROTOCOL_ERROR, such as by suddenly unplugging the internet.
(2) See that no details are printed.

The same is true for exported HAR files.

What is the expected result?

A more detailed error based on quic_error_codes.h is printed.

What happens instead?

Just NET_QUIC_PROTOCOL_ERROR is printed.

 

Comment 1 by ianswett@google.com, Jan 12 2018

Components: Internals>Network>QUIC

Comment 2 by mmenke@chromium.org, Jan 31 2018

We need to be a bit careful about privacy here - I'm not sure we'd want to expose proxy information, for instance, to an untrusted process (Though perhaps we already do, through devtools - if so, should mirror whatever checks devtools has for cookie access and the like).
I believe the plan is to add a detailed QUIC error anytime NET_QUIC_PROTOCOL_ERROR occurs.  This should not be a privacy problem.

Brad and I also thought we should be translating some QUIC errors into generic network errors when they're more appropriate.
If we just want to provide a more detailed error code, we can just bifurcate QUIC_PROTOCOL_ERRORs into other (existing/new) network error codes, and we're done (Modulo perhaps adding some new error text), as we already send network error codes to the renderer process.  So I assume this issue is about adding some sort of metadata about the error - IP that we were trying to connect to is one such piece of metadata, with privacy implications, so just wanted to make sure we were careful here.
Not to harp on it too much, but NetErrorDetails does indeed contain proxy server info, remote IP info, AuthChallengeInfo, SSLCertRequestInfo, SSLInfo, and HttpResponseHeaders, which are probably not things we should be sending to untrusted renderers, at least for proxy connections, and probably not for third party requests, either (With or without CORs).  Not sure what sort of sanitizing we normally do with set-cookie headers for third-party sites, for instance, but don't want to add another path for a compromised renderer to get that sort of data.
The approach I took was to add an extended error code member and propagate the quic error codes though that to WebURLError. Let me know what you think of that approach. Another alternative would be to create net error equivalents for all of the QUIC error codes.

https://chromium-review.googlesource.com/#/c/chromium/src/+/897706
Status: Assigned (was: Untriaged)
WebURLError used to have an error domain, which was a higher level than the error code, though it was removed relatively recently.  My preference would be to bring that back, and use a domain of "net" for network error codes, and "quic" for QUIC error codes (It could even be an enum, though the enum would have to be in services/network, if we wanted to make this general).  The nice thing about this approach is it doesn't make net/ the error code arbiter, so we could have ServiceWorker provide its own error domain, for instance.

I think domain was used because we weren't using it?

Anyhow, I can certainly live with that approach, and adding a more specific field than the net error is less likely to break existing consumers compared to adding a field that indicates the other value is a net error code.
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 19 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/16c13f7f7a45117af4d03b2eb85316744f689993

commit 16c13f7f7a45117af4d03b2eb85316744f689993
Author: Brad Lassey <lassey@chromium.org>
Date: Mon Mar 19 19:37:50 2018

Plumbed QUIC errors through to WebURLError which is where HAR files and error pages get them from.

Added extended error codes to WebURLError, URLLoaderCompletionStatus and
ResourceError as well as adding a new ExtendedErrorToString method in
net_errors.h to take the extended error code and include the quic error
if applicable.

Bug:  801669 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I503c0197a8e5835be6bbbc2a7209c4373e7a08fd
Reviewed-on: https://chromium-review.googlesource.com/897706
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Brad Lassey <lassey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544125}
[modify] https://crrev.com/16c13f7f7a45117af4d03b2eb85316744f689993/content/browser/loader/mojo_async_resource_handler.cc
[modify] https://crrev.com/16c13f7f7a45117af4d03b2eb85316744f689993/content/renderer/loader/sync_load_context.cc
[modify] https://crrev.com/16c13f7f7a45117af4d03b2eb85316744f689993/content/renderer/loader/sync_load_response.h
[modify] https://crrev.com/16c13f7f7a45117af4d03b2eb85316744f689993/content/renderer/loader/web_url_loader_impl.cc
[modify] https://crrev.com/16c13f7f7a45117af4d03b2eb85316744f689993/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/16c13f7f7a45117af4d03b2eb85316744f689993/net/BUILD.gn
[modify] https://crrev.com/16c13f7f7a45117af4d03b2eb85316744f689993/net/base/net_errors.cc
[modify] https://crrev.com/16c13f7f7a45117af4d03b2eb85316744f689993/net/base/net_errors.h
[modify] https://crrev.com/16c13f7f7a45117af4d03b2eb85316744f689993/net/quic/core/quic_error_codes.h
[modify] https://crrev.com/16c13f7f7a45117af4d03b2eb85316744f689993/services/network/public/cpp/url_loader_completion_status.cc
[modify] https://crrev.com/16c13f7f7a45117af4d03b2eb85316744f689993/services/network/public/cpp/url_loader_completion_status.h
[modify] https://crrev.com/16c13f7f7a45117af4d03b2eb85316744f689993/services/network/url_loader.cc
[modify] https://crrev.com/16c13f7f7a45117af4d03b2eb85316744f689993/third_party/WebKit/Source/platform/exported/WebURLError.cpp
[modify] https://crrev.com/16c13f7f7a45117af4d03b2eb85316744f689993/third_party/WebKit/Source/platform/loader/fetch/ResourceError.cpp
[modify] https://crrev.com/16c13f7f7a45117af4d03b2eb85316744f689993/third_party/WebKit/Source/platform/loader/fetch/ResourceError.h
[modify] https://crrev.com/16c13f7f7a45117af4d03b2eb85316744f689993/third_party/WebKit/public/platform/WebURLError.h

Comment 9 by lassey@google.com, Mar 28 2018

Status: Fixed (was: Assigned)

Sign in to add a comment