canton7 / RestEase

Easy-to-use typesafe REST API client library for .NET Standard 1.1 and .NET Framework 4.5 and higher, which is simple and customisable. Inspired by Refit
MIT License
1.08k stars 109 forks source link

HttpRequestException fires instead of ApiException #180

Closed mykyta-bondarenko closed 3 years ago

mykyta-bondarenko commented 3 years ago

I have .net core 5 application and integration tests for it. I am using middleware to handle exceptions from the whole application and return needed responses (with UseExceptionHandler helps). I have a custom exception class (let it be TestException) and in case of this exception, I return NotFound result. And in this case, ApiRequester doesn't handle it as ApiException but as a HttpRequestException. So, I have

var apiException = await Assert.ThrowsAsync<ApiException>(async () => await Client.RunRequestAsync(id));

And it's what I get:

Assert.Throws() Failure
    Expected: typeof(RestEase.ApiException)
    Actual:   typeof(System.Net.Http.HttpRequestException): Error while copying content to a stream.
    ---- System.Net.Http.HttpRequestException : Error while copying content to a stream.
    -------- System.IO.IOException : TestException [text of exception]

If I change the returned value to ServerInternalException- everything works properly and it's handled as APiException, but not in case I am using NotFound

canton7 commented 3 years ago

I'm afraid I'm having trouble understanding your setup:

  1. What is ApiRequester?
  2. What is Client.RunRequestAsync?
  3. ServerInternalException seems to be specific to the AWS SDK?
  4. Presumably the middleware you're referring to is ASP.NET middleware? You don't mention that you're using ASP.NET, but I assume you are from UseExceptionHandler.

ApiException is thrown purely based on the response code of the HttpResponseMessage, see here. From your InnerException, it looks like the exception is being raised when trying to read the content of the response stream, perhaps?

Please post the stack trace of the HttpRequestException, so we can see where it's being thrown from.

mykyta-bondarenko commented 3 years ago

Hi @canton7. Sorry, I mixed some types up, so, here are my explanations.

  1. we use public class ApiRequester : Requester to handle and log additional issues. We override SendRequestAsyncand using base.SendRequestAsync inside of it.
  2. Client.RunRequestAsync is a request to an API that is declared like
    [BasePath("api/v1.0/tests")]
    public interface ITestsClient
  3. I meant 500 Internal Server Error, just an example of any kind of error except 404 Not Found
  4. Yeah, you are right, it's asp net core 5 application

The full stacktrace

Message: 
    Assert.Throws() Failure
    Expected: typeof(RestEase.ApiException)
    Actual:   typeof(System.Net.Http.HttpRequestException): Error while copying content to a stream.
    ---- System.Net.Http.HttpRequestException : Error while copying content to a stream.
    -------- System.IO.IOException : 
    ------------ [namespace].TestException : [exceptions custom message]
  Stack Trace: 
    HttpContent.LoadIntoBufferAsyncCore(Task serializeToStreamTask, MemoryStream tempBuffer)
    HttpClient.SendAsyncCore(HttpRequestMessage request, HttpCompletionOption completionOption, Boolean async, Boolean emitTelemetryStartStop, CancellationToken cancellationToken)
    Requester.SendRequestAsync(IRequestInfo requestInfo, Boolean readBody)
    ApiRequester.SendRequestAsync(IRequestInfo requestInfo, Boolean readBody) line 36
    Requester.RequestAsync[T](IRequestInfo requestInfo)
    <<[Test-Name]>b__0>d.MoveNext() line 57
    --- End of stack trace from previous location ---
    ----- Inner Stack Trace -----
    HttpContent.LoadIntoBufferAsyncCore(Task serializeToStreamTask, MemoryStream tempBuffer)
    HttpClient.SendAsyncCore(HttpRequestMessage request, HttpCompletionOption completionOption, Boolean async, Boolean emitTelemetryStartStop, CancellationToken cancellationToken)
    Requester.SendRequestAsync(IRequestInfo requestInfo, Boolean readBody)
    ApiRequester.SendRequestAsync(IRequestInfo requestInfo, Boolean readBody) line 36
    Requester.RequestAsync[T](IRequestInfo requestInfo)
    <<[Test-Name]>b__0>d.MoveNext() line 57
    --- End of stack trace from previous location ---
    ----- Inner Stack Trace -----
    ResponseBodyReaderStream.CheckAborted()
    ResponseBodyReaderStream.ReadAsync(Byte[] buffer, Int32 offset, Int32 count, CancellationToken cancellationToken)
    Stream.CopyToAsyncInternal(Stream destination, Int32 bufferSize, CancellationToken cancellationToken)
    HttpContent.LoadIntoBufferAsyncCore(Task serializeToStreamTask, MemoryStream tempBuffer)
    ----- Inner Stack Trace -----
   [application-related-stacktrace]
canton7 commented 3 years ago

So, it looks like HttpClient hit that exception as it was trying to write the request. Presumably you're using some sort of synchronous HttpMessageHandler which isn't actually making network requests? There's no way that your TestException from your server could be transferred back to the client otherwise.

Either way, ApiException only covers non-success status codes in the HTTP response. It does not cover unexpected errors which occur when trying to write the request, or read the response. There's something about your test setup which means that HttpClient.SendAsync is failing with a TestException, and RestEase won't try and turn that into an ApiException (and how could it? There's no status code to assign to the StatusCode property, or response to assign to Content).

So I'm afraid that whatever is going on, it's in your test setup: it sounds like you want to get to a point where your HttpMessageHandler returns a response with a non-success status code, rather than throwing when we try to send the request itself. If you get that fixed, RestEase will turn that non-success response message info an ApiException for you.

canton7 commented 3 years ago

I'd also encourage you to use a custom HttpMessageHandler instead of subclassing Requester, if you can. I can't realistically keep the interface of Requester non-breaking while also introducing new functionality (and the doc comments now explain this), and if you're just overriding SendRequestAsync, you should be able to do the same from a custom HttpMessageHandler. The next version of RestEase will include RestClient overloads to make it easier to specify one.

mykyta-bondarenko commented 3 years ago

@canton7 I've found that there is an inner exception and it looks like smth wrong with an HTTP client that I got from asp testing nuget. Anyway, it's not this code issue, sorry. The inner exception is:

   at Microsoft.AspNetCore.TestHost.ResponseBodyReaderStream.CheckAborted()
   at Microsoft.AspNetCore.TestHost.ResponseBodyReaderStream.<ReadAsync>d__26.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.IO.Stream.<CopyToAsyncInternal>d__29.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable.ConfiguredTaskAwaiter.GetResult()
   at System.Net.Http.HttpContent.<LoadIntoBufferAsyncCore>d__63.MoveNext()

And it has its inner exception with is a custom one (TestException)

mykyta-bondarenko commented 3 years ago

@canton7 so, I found the reason for it, if you are interested it's here https://github.com/aspnet/Announcements/issues/434

canton7 commented 3 years ago

I'd be surprised if that linked announcement was your issue: that's talking about returning 404 vs 500, whereas what you're seeing is that an exception is being fed back directly through the test HttpClient, rather than being turned into a proper HTTP response.

However I don't think I can help you further I'm afraid, and I don't think this is a RestEase issue. Good luck!