Add response read timeouts #10128
Add response read timeouts #10128
Conversation
| /// <summary> | ||
| /// The timeout for a single operation. | ||
| /// </summary> | ||
| public TimeSpan TryTimeout { get; set; } = TimeSpan.FromSeconds(100); |
There was a problem hiding this comment.
cc @KrzysztofCwalina
I took the default HttpClient timeout.
There was a problem hiding this comment.
Should this be part of and/or documented in the General Guidelines?
There was a problem hiding this comment.
I find the name a little hard to wrap my head around at first glance. PerTryTimeout is kind of awkward but matches the pipeline position naming with PerCall, etc. I don't feel strongly here.
There was a problem hiding this comment.
Changed to NetworkTimeout
| } | ||
|
|
||
| #if NETCOREAPP |
There was a problem hiding this comment.
.NET Framework doesn't react to cancellation token on response reads.
sdk/core/Azure.Core/src/Pipeline/Internal/ResponseBodyPolicy.cs
Outdated
Show resolved
Hide resolved
| /// <summary> | ||
| /// The timeout for a single operation. | ||
| /// </summary> | ||
| public TimeSpan TryTimeout { get; set; } = TimeSpan.FromSeconds(100); |
There was a problem hiding this comment.
Should this be part of and/or documented in the General Guidelines?
| /// <summary> | ||
| /// The timeout for a single operation. | ||
| /// </summary> | ||
| public TimeSpan TryTimeout { get; set; } = TimeSpan.FromSeconds(100); |
There was a problem hiding this comment.
I find the name a little hard to wrap my head around at first glance. PerTryTimeout is kind of awkward but matches the pipeline position naming with PerCall, etc. I don't feel strongly here.
| @@ -73,7 +73,7 @@ public static HttpPipeline Build(ClientOptions options, HttpPipelinePolicy[] per | |||
| diagnostics.LoggedHeaderNames.ToArray(), diagnostics.LoggedQueryParameters.ToArray())); | |||
| } | |||
|
|
|||
| policies.Add(BufferResponsePolicy.Shared); | |||
| policies.Add(new ResponseBodyPolicy(options.Retry.NetworkTimeout)); | |||
There was a problem hiding this comment.
Should we check if the timeout was changed, and if not, use the shared policy
Limitations: