google / cronet-transport-for-okhttp

This package allows OkHttp and Retrofit users to use Cronet as their transport layer, benefiting from features like QUIC/HTTP3 support or connection migration.
Apache License 2.0
427 stars 31 forks source link

Update CronetCallFactory.java #18

Closed limuyang2 closed 1 year ago

Danstahrg commented 1 year ago

Hi,

Thank you for your contribution to the library!

The change the PR proposes is incorrect. Note that we don't want to stop the timeout before the entire response body is read, and that moment falls outside of the execute() method.

If the request is handled successfully, the timeout is being correctly stopped at line 261: https://github.com/google/cronet-transport-for-okhttp/pull/18/files#diff-c75fce2a533bf50354d3eca8b3cd62cc1234f8c7977cb90f665c6ae5fb3cd07cL261

I'll add a comment to make sure this doesn't confuse future readers.

Danstahrg commented 1 year ago

Comments updated in 30f19bc.