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

Issue 614507 link

Starred by 1 user
Status: Fixed
Owner:
█ mge...@chromium.org
Last visit > 30 days ago
Closed: Jul 2016
Cc:
█ mge...@chromium.org
█ sidv@chromium.org
█ rtenneti@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Feature



Sign in to add a comment

[Cronet] Expose the more detailed underlying error for QUIC_PROTOCOL_ERROR / expose QuicErrorCode

Project Member Reported by mef@chromium.org, May 24 2016 
QUIC_PROTOCOL_ERROR happens often and is currently hard to investigate. This is because 
there are many unrelated errors that are grouped and reported as QUIC_PROTOCOL_ERROR.

A potential strategy would be to include the quic error code within the UrlRequestException(/soon to be CronetException) that's returned in UrlRequest#onFailed() either within a cause exception (?QuicException) or less generally, within a field.

Comment 1 by mef@chromium.org, May 24 2016

Labels: -Type-Bug -Pri-3 Pri-1 Type-Feature
Status: Available (was: Untriaged)

Comment 2 by mef@chromium.org, Jun 6 2016

Cc: rtenneti@chromium.org mge...@chromium.org sidv@chromium.org
Brief code search shows that we should have NetErrorDetails available using net::UrlRequest::PopulateNetErrorDetails(): https://cs.chromium.org/chromium/src/net/url_request/url_request.h?rcl=0&l=482 and could expose its raw value from Cronet UrlRequestException https://cs.chromium.org/chromium/src/components/cronet/android/api/src/org/chromium/net/UrlRequestException.java?rcl=0&l=122 as some obscure method like getCronetInternalErrorCodeDetails() (so we could extend it beyond QUIC if necessary).

Getting this info is important for analysis of QUIC errors in Cronet, so we need to add this now.

Comment 3 by mge...@chromium.org, Jun 7 2016

Owner: mge...@chromium.org
Status: Assigned (was: Available)

Comment 4 by sidv@chromium.org, Jun 13 2016

Labels: M-53 OS-Android
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 1 2016 

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

commit a8228bdbdd1840d925baf93067057daccae1e60a
Author: mgersh <mgersh@chromium.org>
Date: Fri Jul 01 20:57:11 2016

Add new Cronet exception class for QUIC errors

Create a subclass of CronetException, called QuicException, which contains the
detailed QUIC error code when it exists.
Propagate the error code from NetErrorDetails through Cronet. For now,
only UrlRequest reports the detailed error, not BidirectionalStream.

BUG= 614507 

Review-Url: https://codereview.chromium.org/2069303002
Cr-Commit-Position: refs/heads/master@{#403525}

[modify] https://crrev.com/a8228bdbdd1840d925baf93067057daccae1e60a/components/cronet/android/BUILD.gn
[add] https://crrev.com/a8228bdbdd1840d925baf93067057daccae1e60a/components/cronet/android/api/src/org/chromium/net/QuicException.java
[modify] https://crrev.com/a8228bdbdd1840d925baf93067057daccae1e60a/components/cronet/android/api/src/org/chromium/net/UrlRequestException.java
[modify] https://crrev.com/a8228bdbdd1840d925baf93067057daccae1e60a/components/cronet/android/cronet_bidirectional_stream_adapter.cc
[modify] https://crrev.com/a8228bdbdd1840d925baf93067057daccae1e60a/components/cronet/android/cronet_url_request_adapter.cc
[modify] https://crrev.com/a8228bdbdd1840d925baf93067057daccae1e60a/components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java
[modify] https://crrev.com/a8228bdbdd1840d925baf93067057daccae1e60a/components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java
[modify] https://crrev.com/a8228bdbdd1840d925baf93067057daccae1e60a/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java
[modify] https://crrev.com/a8228bdbdd1840d925baf93067057daccae1e60a/components/cronet/android/url_request_error.cc
[modify] https://crrev.com/a8228bdbdd1840d925baf93067057daccae1e60a/components/cronet/android/url_request_error.h
[modify] https://crrev.com/a8228bdbdd1840d925baf93067057daccae1e60a/net/test/url_request/url_request_failed_job.cc
[modify] https://crrev.com/a8228bdbdd1840d925baf93067057daccae1e60a/net/test/url_request/url_request_failed_job.h

Comment 6 by mge...@chromium.org, Jul 1 2016

Status: Fixed (was: Assigned)

Comment 7 by mef@chromium.org, Jul 7 2016

Labels: Merge-Request-53
This change does not affect Chrome proper, just Cronet and is very important for QUIC error troubleshooting.

Comment 8 by dimu@google.com, Jul 7 2016

Labels: -Merge-Request-53 Merge-Approved-53 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M53 (branch: 2785)
Project Member

Comment 9 by sheriffbot@chromium.org, Jul 11 2016 

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 11 2016

Labels: -merge-approved-53 merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0c9d4e56fd928a79c5ec4f947771a5b9631f064a

commit 0c9d4e56fd928a79c5ec4f947771a5b9631f064a
Author: Misha Efimov <mef@chromium.org>
Date: Mon Jul 11 15:45:11 2016

Add new Cronet exception class for QUIC errors

Create a subclass of CronetException, called QuicException, which contains the
detailed QUIC error code when it exists.
Propagate the error code from NetErrorDetails through Cronet. For now,
only UrlRequest reports the detailed error, not BidirectionalStream.

BUG= 614507 

Review-Url: https://codereview.chromium.org/2069303002
Cr-Commit-Position: refs/heads/master@{#403525}
(cherry picked from commit a8228bdbdd1840d925baf93067057daccae1e60a)

R=mgersh@chromium.org, xunjieli@chromium.org

Review URL: https://codereview.chromium.org/2137133002 .

Cr-Commit-Position: refs/branch-heads/2785@{#81}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/0c9d4e56fd928a79c5ec4f947771a5b9631f064a/components/cronet/android/BUILD.gn
[add] https://crrev.com/0c9d4e56fd928a79c5ec4f947771a5b9631f064a/components/cronet/android/api/src/org/chromium/net/QuicException.java
[modify] https://crrev.com/0c9d4e56fd928a79c5ec4f947771a5b9631f064a/components/cronet/android/api/src/org/chromium/net/UrlRequestException.java
[modify] https://crrev.com/0c9d4e56fd928a79c5ec4f947771a5b9631f064a/components/cronet/android/cronet_bidirectional_stream_adapter.cc
[modify] https://crrev.com/0c9d4e56fd928a79c5ec4f947771a5b9631f064a/components/cronet/android/cronet_url_request_adapter.cc
[modify] https://crrev.com/0c9d4e56fd928a79c5ec4f947771a5b9631f064a/components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java
[modify] https://crrev.com/0c9d4e56fd928a79c5ec4f947771a5b9631f064a/components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java
[modify] https://crrev.com/0c9d4e56fd928a79c5ec4f947771a5b9631f064a/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java
[modify] https://crrev.com/0c9d4e56fd928a79c5ec4f947771a5b9631f064a/components/cronet/android/url_request_error.cc
[modify] https://crrev.com/0c9d4e56fd928a79c5ec4f947771a5b9631f064a/components/cronet/android/url_request_error.h
[modify] https://crrev.com/0c9d4e56fd928a79c5ec4f947771a5b9631f064a/net/test/url_request/url_request_failed_job.cc
[modify] https://crrev.com/0c9d4e56fd928a79c5ec4f947771a5b9631f064a/net/test/url_request/url_request_failed_job.h

Comment 11 by msrchandra@chromium.org, Jul 19 2016

Labels: Needs-Feedback
@mef -- Could you please provide any steps so that the issue could be verified.
Thanks in Advance.

Comment 12 by mef@chromium.org, Jul 19 2016

Labels: -Needs-Feedback
@msrchandra - This is not applicable to Chrome proper, just to consumers of Cronet API library. 

See components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java for the example of usage.

There is no need or way to verify it in Chrome.
►

Sign in to add a comment