Skip to content
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

[Bug] Respect client timeout even after response headers are returned #9340

Closed
JoshLove-msft opened this issue Jan 7, 2020 · 4 comments
Closed
Assignees
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library.
Milestone

Comments

@JoshLove-msft
Copy link
Member

JoshLove-msft commented Jan 7, 2020

Since we use the HttpCompletionOption.ResponseHeadersRead option in HttpClientTransport, the HttpClient timeout will not apply after the response headers are returned.
This means that if the body of the response never returns for some reason, the client will block indefinitely.

Describe the solution you'd like
Issue #9289 requested a ClientTimeout property in ClientOptions. Once we add this, one idea would be to respect the property even after the header is returned. Specifically, RetriableStream could respect this timeout and throw a TimeoutException if we pass the timeout. A problem with this approach would be that large files would likely timeout, so a better approach would be to either use the same timeout (or a different timeout value) to mean how long to wait before throwing when data has stopped returning, or go by the rate that the data is being read and throw if we fall under a certain threshold. The service has a 2MB/minute minimum threshold before throwing: https://docs.microsoft.com/en-us/rest/api/storageservices/setting-timeouts-for-blob-service-operations

I'm not sure if there are other scenarios not involving RetriableStream where this indefinite blocking can occur, but if there are we should address those to.

@JoshLove-msft JoshLove-msft added Storage Storage Service (Queues, Blobs, Files) Client This issue points to a problem in the data-plane of the library. Azure.Core and removed Storage Storage Service (Queues, Blobs, Files) labels Jan 7, 2020
@JoshLove-msft JoshLove-msft changed the title [FEATURE REQ] Respect client timeout even after response headers are returned [Bug] Respect client timeout even after response headers are returned Jan 27, 2020
@mikeharder
Copy link
Member

@JoshLove-msft: Should this be fixed by #10128? If so, can we try to re-enable the tests?

@JoshLove-msft
Copy link
Member Author

I think so, we should give it a try.

@AlexGhiondea AlexGhiondea assigned pakrym and JoshLove-msft and unassigned pakrym Mar 2, 2020
@AlexGhiondea AlexGhiondea added this to the [2020] March milestone Mar 2, 2020
@AlexGhiondea
Copy link
Contributor

@JoshLove-msft is this still applicable? If not, please close this.

@JoshLove-msft
Copy link
Member Author

This was fixed in #10169
Closing

@github-actions github-actions bot locked and limited conversation to collaborators Mar 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Development

No branches or pull requests

4 participants