dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.35k stars 9.99k forks source link

TestServer throws IOException when client timeout occurs #18258

Closed lukasz-pyrzyk closed 4 years ago

lukasz-pyrzyk commented 4 years ago

Describe the bug

I have been writing tests with WebApplicationFactory for my wrapper around HttpClient shared between many projects. I've spotted a quite surprising exception when I tried to validate my Polly retry logic for request timeout - OException : The client aborted the request. The exception does not occur when I change my test to use remote instance instead of one from WebApplicationFactory.

To Reproduce

Repro is available at https://github.com/lukasz-pyrzyk/testServerIssue

System.Net.Http.HttpRequestException : Error while copying content to a stream.
---- System.IO.IOException :
-------- System.IO.IOException : The client aborted the request.
   at System.Net.Http.HttpContent.LoadIntoBufferAsyncCore(Task serializeToStreamTask, MemoryStream tempBuffer)
   at System.Net.Http.HttpClient.FinishSendAsyncBuffered(Task`1 sendTask, HttpRequestMessage request, CancellationTokenSource cts, Boolean disposeCts)
   at Tests.CheckRetry.ShouldRetry_OnInMemoryInstance() in C:\dev\testserverIssue\src\Tests\CheckRetry.cs:line 23
--- End of stack trace from previous location where exception was thrown ---
----- Inner Stack Trace -----
   at Microsoft.AspNetCore.TestHost.ResponseBodyReaderStream.CheckAborted()
   at Microsoft.AspNetCore.TestHost.ResponseBodyReaderStream.ReadAsync(Byte[] buffer, Int32 offset, Int32 count, CancellationToken cancellationToken)
   at System.IO.Stream.CopyToAsyncInternal(Stream destination, Int32 bufferSize, CancellationToken cancellationToken)
   at System.Net.Http.HttpContent.LoadIntoBufferAsyncCore(Task serializeToStreamTask, MemoryStream tempBuffer)
----- Inner Stack Trace -----

Further technical details

Runtime Environment: OS Name: Windows OS Version: 10.0.18363 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\3.1.100

Tratcher commented 4 years ago

What exception was polly expecting to handle when the first request timed out? An OperationCancelledException or similar?

TestServer is optimized to unit test server behaviors and to surface any server related errors to the client with as high a fidelity as possible. Sometimes this involves deviating from errors that HttpClient would normally throw. As such, I don't recommend using TestServer to test client side functionality. A custom HttpMessageHandler would be more appropriate for unit testing the client's behavior. For a full integration test using a remote instance is recommended.

lukasz-pyrzyk commented 4 years ago

My assumption was that the logic of testing the custom http client should work because it was working before migration from .NET Core 2.1 to 3.0/3.1.

Answering to your questions,

What exception was polly expecting to handle when the first request timed out? An OperationCancelledException or similar?

Polly should handle any HttpRequestException or response response => response.StatusCode >= HttpStatusCode.InternalServerError || response.StatusCode == HttpStatusCode.RequestTimeout

The in-memory test throws HttpRequestException with AllowRetry=false and inner ex.InnerException.InnerException.Message saying The client aborted the request..

I'm wondering why a retry predicate was not even called to check if the exception should be retried. It looks like the exception is thrown somehow out of the scope of Polly,

I can close this issue and migrate to HttpMessageHandler if this is your recommendation

Tratcher commented 4 years ago

I'm wondering why a retry predicate was not even called to check if the exception should be retried. It looks like the exception is thrown somehow out of the scope of Polly,

It looks like it got a response and then failed while buffering the body. It seems like Polly disengages after the headers are received.

Yes, a custom HttpMessageHandler is the best option to unit test client modules.

ghost commented 4 years ago

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.