From 1e62b20fe695313d4084b81d498c53c223115164 Mon Sep 17 00:00:00 2001 From: Claire Novotny Date: Wed, 9 Dec 2020 10:19:43 -0500 Subject: [PATCH] Wrap Deserialization exceptions as an ApiException --- Refit.Tests/ResponseTests.cs | 51 ++++++++++++++++++++++ Refit/ApiException.cs | 16 +++---- Refit/RequestBuilderImplementation.cs | 61 +++++++++++++++------------ 3 files changed, 94 insertions(+), 34 deletions(-) diff --git a/Refit.Tests/ResponseTests.cs b/Refit.Tests/ResponseTests.cs index 57cec7f09..cdff2abd4 100644 --- a/Refit.Tests/ResponseTests.cs +++ b/Refit.Tests/ResponseTests.cs @@ -248,6 +248,57 @@ public async Task ValidationApiException_HydratesBaseContent() var actualBaseException = actualException as ApiException; Assert.Equal(expectedContent, actualBaseException.Content); } + + + [Fact] + public async Task WithHtmlResponse_ShouldReturnApiException() + { + const string htmlResponse = "Hello world"; + var expectedResponse = new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(htmlResponse) + }; + expectedResponse.Content.Headers.Clear(); + + mockHandler.Expect(HttpMethod.Get, "http://api/aliasTest") + .Respond(req => expectedResponse); + + var actualException = await Assert.ThrowsAsync(() => fixture.GetTestObject()); + + Assert.IsType(actualException.InnerException); + Assert.NotNull(actualException.Content); + Assert.Equal(htmlResponse, actualException.Content); + } + + [Fact] + public async Task WithNonJsonResponseUsingNewtonsoftJsonContentSerializer_ShouldReturnApiException() + { + var settings = new RefitSettings + { + HttpMessageHandlerFactory = () => mockHandler, + ContentSerializer = new NewtonsoftJsonContentSerializer() + }; + + var newtonSoftFixture = RestService.For("http://api", settings); + + const string nonJsonResponse = "bad response"; + var expectedResponse = new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(nonJsonResponse) + }; + expectedResponse.Content.Headers.Clear(); + + mockHandler.Expect(HttpMethod.Get, "http://api/aliasTest") + .Respond(req => expectedResponse); + + var actualException = await Assert.ThrowsAsync(() => newtonSoftFixture.GetTestObject()); + + Assert.IsType(actualException.InnerException); + Assert.NotNull(actualException.Content); + Assert.Equal(nonJsonResponse, actualException.Content); + } + + } public sealed class ThrowOnGetLengthMemoryStream : MemoryStream diff --git a/Refit/ApiException.cs b/Refit/ApiException.cs index 13e11ce61..6da5eb8d1 100644 --- a/Refit/ApiException.cs +++ b/Refit/ApiException.cs @@ -21,13 +21,13 @@ public class ApiException : Exception public bool HasContent => !string.IsNullOrWhiteSpace(Content); public RefitSettings RefitSettings { get; } - protected ApiException(HttpRequestMessage message, HttpMethod httpMethod, string? content, HttpStatusCode statusCode, string? reasonPhrase, HttpResponseHeaders headers, RefitSettings refitSettings) : - this(CreateMessage(statusCode, reasonPhrase), message, httpMethod, content, statusCode, reasonPhrase, headers, refitSettings) + protected ApiException(HttpRequestMessage message, HttpMethod httpMethod, string? content, HttpStatusCode statusCode, string? reasonPhrase, HttpResponseHeaders headers, RefitSettings refitSettings, Exception? innerException = null) : + this(CreateMessage(statusCode, reasonPhrase), message, httpMethod, content, statusCode, reasonPhrase, headers, refitSettings, innerException) { } - protected ApiException(string exceptionMessage, HttpRequestMessage message, HttpMethod httpMethod, string? content, HttpStatusCode statusCode, string? reasonPhrase, HttpResponseHeaders headers, RefitSettings refitSettings) : - base(exceptionMessage) + protected ApiException(string exceptionMessage, HttpRequestMessage message, HttpMethod httpMethod, string? content, HttpStatusCode statusCode, string? reasonPhrase, HttpResponseHeaders headers, RefitSettings refitSettings, Exception? innerException = null) : + base(exceptionMessage, innerException) { RequestMessage = message; HttpMethod = httpMethod; @@ -43,18 +43,18 @@ await RefitSettings.ContentSerializer.DeserializeAsync(new StringContent(Cont default; #pragma warning disable VSTHRD200 // Use "Async" suffix for async methods - public static Task Create(HttpRequestMessage message, HttpMethod httpMethod, HttpResponseMessage response, RefitSettings refitSettings) + public static Task Create(HttpRequestMessage message, HttpMethod httpMethod, HttpResponseMessage response, RefitSettings refitSettings, Exception? innerException = null) #pragma warning restore VSTHRD200 // Use "Async" suffix for async methods { var exceptionMessage = CreateMessage(response.StatusCode, response.ReasonPhrase); - return Create(exceptionMessage, message, httpMethod, response, refitSettings); + return Create(exceptionMessage, message, httpMethod, response, refitSettings, innerException); } #pragma warning disable VSTHRD200 // Use "Async" suffix for async methods - public static async Task Create(string exceptionMessage, HttpRequestMessage message, HttpMethod httpMethod, HttpResponseMessage response, RefitSettings refitSettings) + public static async Task Create(string exceptionMessage, HttpRequestMessage message, HttpMethod httpMethod, HttpResponseMessage response, RefitSettings refitSettings, Exception? innerException = null) #pragma warning restore VSTHRD200 // Use "Async" suffix for async methods { - var exception = new ApiException(exceptionMessage, message, httpMethod, null, response.StatusCode, response.ReasonPhrase, response.Headers, refitSettings); + var exception = new ApiException(exceptionMessage, message, httpMethod, null, response.StatusCode, response.ReasonPhrase, response.Headers, refitSettings, innerException); if (response.Content == null) { diff --git a/Refit/RequestBuilderImplementation.cs b/Refit/RequestBuilderImplementation.cs index 1d2e0f8d1..b276058f1 100644 --- a/Refit/RequestBuilderImplementation.cs +++ b/Refit/RequestBuilderImplementation.cs @@ -253,6 +253,7 @@ async Task AddMultipartItemAsync(MultipartFormDataContent multiPartContent, stri e = await settings.ExceptionFactory(resp).ConfigureAwait(false); } + if (restMethod.IsApiResponse) { // Only attempt to deserialize content if no error present for backward-compatibility @@ -269,6 +270,7 @@ async Task AddMultipartItemAsync(MultipartFormDataContent multiPartContent, stri } else return await DeserializeContentAsync(resp, content, ct).ConfigureAwait(false); + } finally { @@ -286,35 +288,42 @@ async Task AddMultipartItemAsync(MultipartFormDataContent multiPartContent, stri async Task DeserializeContentAsync(HttpResponseMessage resp, HttpContent content, CancellationToken cancellationToken) { - T? result; - if (typeof(T) == typeof(HttpResponseMessage)) - { - // NB: This double-casting manual-boxing hate crime is the only way to make - // this work without a 'class' generic constraint. It could blow up at runtime - // and would be A Bad Idea if we hadn't already vetted the return type. - result = (T)(object)resp; - } - else if (typeof(T) == typeof(HttpContent)) - { - result = (T)(object)content; - } - else if (typeof(T) == typeof(Stream)) - { - var stream = (object)await content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); - result = (T)stream; - } - else if (typeof(T) == typeof(string)) + try { - using var stream = await content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); - using var reader = new StreamReader(stream); - var str = (object)await reader.ReadToEndAsync().ConfigureAwait(false); - result = (T)str; + T? result; + if (typeof(T) == typeof(HttpResponseMessage)) + { + // NB: This double-casting manual-boxing hate crime is the only way to make + // this work without a 'class' generic constraint. It could blow up at runtime + // and would be A Bad Idea if we hadn't already vetted the return type. + result = (T)(object)resp; + } + else if (typeof(T) == typeof(HttpContent)) + { + result = (T)(object)content; + } + else if (typeof(T) == typeof(Stream)) + { + var stream = (object)await content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); + result = (T)stream; + } + else if (typeof(T) == typeof(string)) + { + using var stream = await content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); + using var reader = new StreamReader(stream); + var str = (object)await reader.ReadToEndAsync().ConfigureAwait(false); + result = (T)str; + } + else + { + result = await serializer.DeserializeAsync(content, cancellationToken).ConfigureAwait(false); + } + return result; } - else + catch(Exception ex) // wrap the exception as an ApiException { - result = await serializer.DeserializeAsync(content, cancellationToken).ConfigureAwait(false); - } - return result; + throw await ApiException.Create("An error occured deserializing the response.", resp.RequestMessage!, resp.RequestMessage!.Method, resp, settings, ex); + } } List> BuildQueryMap(object? @object, string? delimiter = null, RestMethodParameterInfo? parameterInfo = null)