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

Issue 624942 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
On parental leave until 3/15/19
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Feature



Sign in to add a comment

Add support for NetErrorDetails to BidirectionalStream, and expose from Cronet

Project Member Reported by mge...@chromium.org, Jun 30 2016

Issue description

Follow-up to  issue 614507 . Once that issue is fixed, QUIC error details will be available from Cronet only for UrlRequest. Embedders are likely to also want them for BidirectionalStream.
 
Components: Internals>Network>Library

Comment 2 by mef@chromium.org, Mar 13 2017

Status: Fixed (was: Assigned)
CronetBidirectionalStream.onError reports QuicException with QUIC error code if applicable.

Comment 3 by mge...@chromium.org, Mar 13 2017

Status: Assigned (was: Fixed)
It still isn't implemented. That field will always be QUIC_NO_ERROR for BidirectionalStream. There's a TODO somewhere for this.

Comment 4 by mef@chromium.org, Jun 27 2017

Cc: xunji...@chromium.org
Labels: OS-Android
It looks like we need to add a method 

void PopulateNetErrorDetails(NetErrorDetails* details)

to net::BidirectionalStream, net::BidirectionalStreamImpl, net::BidirectionalStreamQuicImpl and BidirectionalStreamSpdyImpl. 

Most of those would be empty, except for BidirectionalStreamQuicImpl:

void BidirectionalStreamQuicImpl::PopulateNetErrorDetails(NetErrorDetails* details) {
  details->connection_info =
      ConnectionInfoFromQuicVersion(quic_session_->GetQuicVersion());
  quic_session_->PopulateNetErrorDetails(details);
  if (quic_session_->IsCryptoHandshakeConfirmed() && stream_)
    details->quic_connection_error = stream_->connection_error();
}

Does it make sense? If yes, I can give it a try.

Comment 5 by lassey@chromium.org, Jul 10 2017

Owner: lassey@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 19 2017

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

commit 5f488db43ebc9b5dc7d51aa8a5c604082c8c9242
Author: Brad Lassey <lassey@chromium.org>
Date: Wed Jul 19 00:30:46 2017

Add support for NetErrorDetails to Cronet's BidirectionalStream

This plumbs the quic error information through to Cronet's
BidirectionalStream::OnError

Bug:  624942 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: I7debf4c530cc9fba4a8fe5c62808afa582e6428e
Reviewed-on: https://chromium-review.googlesource.com/567282
Reviewed-by: Misha Efimov <mef@chromium.org>
Commit-Queue: Brad Lassey <lassey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487690}
[modify] https://crrev.com/5f488db43ebc9b5dc7d51aa8a5c604082c8c9242/components/cronet/android/cronet_bidirectional_stream_adapter.cc
[modify] https://crrev.com/5f488db43ebc9b5dc7d51aa8a5c604082c8c9242/components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamQuicTest.java
[modify] https://crrev.com/5f488db43ebc9b5dc7d51aa8a5c604082c8c9242/net/http/bidirectional_stream.cc
[modify] https://crrev.com/5f488db43ebc9b5dc7d51aa8a5c604082c8c9242/net/http/bidirectional_stream.h
[modify] https://crrev.com/5f488db43ebc9b5dc7d51aa8a5c604082c8c9242/net/http/bidirectional_stream_impl.h
[modify] https://crrev.com/5f488db43ebc9b5dc7d51aa8a5c604082c8c9242/net/quic/chromium/bidirectional_stream_quic_impl.cc
[modify] https://crrev.com/5f488db43ebc9b5dc7d51aa8a5c604082c8c9242/net/quic/chromium/bidirectional_stream_quic_impl.h
[modify] https://crrev.com/5f488db43ebc9b5dc7d51aa8a5c604082c8c9242/net/spdy/chromium/bidirectional_stream_spdy_impl.cc
[modify] https://crrev.com/5f488db43ebc9b5dc7d51aa8a5c604082c8c9242/net/spdy/chromium/bidirectional_stream_spdy_impl.h

Comment 7 by lassey@chromium.org, Aug 21 2017

Status: Fixed (was: Started)

Sign in to add a comment