From 011be49a125887dff3cdf120a49dde0e2d8d6dfc Mon Sep 17 00:00:00 2001 From: Steffi Grewenig Date: Tue, 1 Jun 2021 09:20:53 +0200 Subject: [PATCH 1/4] Add httpMock dep to mock http error responses with body --- tests/StackExchange.Utils.Tests/StackExchange.Utils.Tests.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/StackExchange.Utils.Tests/StackExchange.Utils.Tests.csproj b/tests/StackExchange.Utils.Tests/StackExchange.Utils.Tests.csproj index 90d7c5b..a8dbec3 100644 --- a/tests/StackExchange.Utils.Tests/StackExchange.Utils.Tests.csproj +++ b/tests/StackExchange.Utils.Tests/StackExchange.Utils.Tests.csproj @@ -3,6 +3,7 @@ netcoreapp3.1 + From 58e7f7493f1caca99b91993ee7c8161314266ef9 Mon Sep 17 00:00:00 2001 From: Steffi Grewenig Date: Tue, 1 Jun 2021 09:23:17 +0200 Subject: [PATCH 2/4] Add modifier extension to allow logging error response body for given http status codes --- src/StackExchange.Utils.Http/Extensions.Modifier.cs | 12 ++++++++++++ src/StackExchange.Utils.Http/Http.cs | 9 +++++++++ src/StackExchange.Utils.Http/HttpBuilder.cs | 1 + src/StackExchange.Utils.Http/IRequestBuilder.cs | 6 ++++++ 4 files changed, 28 insertions(+) diff --git a/src/StackExchange.Utils.Http/Extensions.Modifier.cs b/src/StackExchange.Utils.Http/Extensions.Modifier.cs index aa70c4f..2a8119c 100644 --- a/src/StackExchange.Utils.Http/Extensions.Modifier.cs +++ b/src/StackExchange.Utils.Http/Extensions.Modifier.cs @@ -91,6 +91,18 @@ public static IRequestBuilder WithoutLogging(this IRequestBuilder builder, HttpS return builder; } + /// + /// Logs error response body as part of the httpClientException data when the response's HTTP status code is any of the . + /// + /// The builder we're working on. + /// HTTP error status codes to log for. + /// The request builder for chaining. + public static IRequestBuilder WithErrorResponseBodyLogging(this IRequestBuilder builder, params HttpStatusCode[] statusCodes) + { + builder.LogErrorResponseBodyStatuses = statusCodes; + return builder; + } + /// /// Adds an event handler for this request, for appending additional information to the logged exception for example. /// diff --git a/src/StackExchange.Utils.Http/Http.cs b/src/StackExchange.Utils.Http/Http.cs index 0aec09e..37c8974 100644 --- a/src/StackExchange.Utils.Http/Http.cs +++ b/src/StackExchange.Utils.Http/Http.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Immutable; using System.Diagnostics; +using System.IO; using System.Linq; using System.Net.Http; using System.Reflection; @@ -68,6 +69,14 @@ internal static async Task> SendAsync(IRequestBuilder if (!response.IsSuccessStatusCode && !builder.Inner.IgnoredResponseStatuses.Contains(response.StatusCode)) { exception = new HttpClientException($"Response code was {(int)response.StatusCode} ({response.StatusCode}) from {response.RequestMessage.RequestUri}: {response.ReasonPhrase}", response.StatusCode, response.RequestMessage.RequestUri); + + if (builder.Inner.LogErrorResponseBodyStatuses.Contains(response.StatusCode)) + { + using var responseStream = await response.Content.ReadAsStreamAsync(); + using var streamReader = new StreamReader(responseStream); + exception.AddLoggedData("Response.Body", await streamReader.ReadToEndAsync()); + } + stackTraceString.SetValue(exception, new StackTrace(true).ToString()); } else diff --git a/src/StackExchange.Utils.Http/HttpBuilder.cs b/src/StackExchange.Utils.Http/HttpBuilder.cs index b0cccd2..cae0ea1 100644 --- a/src/StackExchange.Utils.Http/HttpBuilder.cs +++ b/src/StackExchange.Utils.Http/HttpBuilder.cs @@ -13,6 +13,7 @@ internal class HttpBuilder : IRequestBuilder public HttpRequestMessage Message { get; } public bool LogErrors { get; set; } = true; public IEnumerable IgnoredResponseStatuses { get; set; } = Enumerable.Empty(); + public IEnumerable LogErrorResponseBodyStatuses { get; set; } = Enumerable.Empty(); public TimeSpan Timeout { get; set; } public IWebProxy Proxy { get; set; } public bool BufferResponse { get; set; } = true; diff --git a/src/StackExchange.Utils.Http/IRequestBuilder.cs b/src/StackExchange.Utils.Http/IRequestBuilder.cs index cff49f1..d639306 100644 --- a/src/StackExchange.Utils.Http/IRequestBuilder.cs +++ b/src/StackExchange.Utils.Http/IRequestBuilder.cs @@ -29,6 +29,12 @@ public interface IRequestBuilder [EditorBrowsable(EditorBrowsableState.Never)] bool LogErrors { get; set; } + /// + /// Whether to log error response body on a given http status code. + /// + [EditorBrowsable(EditorBrowsableState.Never)] + IEnumerable LogErrorResponseBodyStatuses { get; set; } + /// /// Which s to ignore as errors on responses. /// From 27b932e008e8f5b21a2cb95f273d2f586569f08e Mon Sep 17 00:00:00 2001 From: Steffi Grewenig Date: Tue, 1 Jun 2021 09:23:49 +0200 Subject: [PATCH 3/4] Unit tests for modifier extension using httpmock --- .../HttpLogErrorResponseBodyTest.cs | 111 ++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 tests/StackExchange.Utils.Tests/HttpLogErrorResponseBodyTest.cs diff --git a/tests/StackExchange.Utils.Tests/HttpLogErrorResponseBodyTest.cs b/tests/StackExchange.Utils.Tests/HttpLogErrorResponseBodyTest.cs new file mode 100644 index 0000000..40694c7 --- /dev/null +++ b/tests/StackExchange.Utils.Tests/HttpLogErrorResponseBodyTest.cs @@ -0,0 +1,111 @@ +using System.Net; +using System.Threading.Tasks; +using HttpMock; +using Xunit; + +namespace StackExchange.Utils.Tests +{ + public class HttpLogErrorResponseBodyTest + { + private readonly IHttpServer _stubHttp = HttpMockRepository.At("http://localhost:9191"); + + [Fact] + public async Task WithErrorResponseBodyLogging_IfStatusCodeMatchesTheGivenOne_IncludeResponseBodyInException() + { + const string errorResponseBody = "{'Foo': 'Bar'}"; + _stubHttp.Stub(x => x.Get("/some-endpoint")) + .Return(errorResponseBody) + .WithStatus(HttpStatusCode.UnprocessableEntity); + + var response = await Http + .Request("http://localhost:9191/some-endpoint") + .WithErrorResponseBodyLogging(HttpStatusCode.UnprocessableEntity) + .ExpectString() + .GetAsync(); + + Assert.Equal(HttpStatusCode.UnprocessableEntity,response.StatusCode); + + var httpCallResponses = Assert.IsType>(response); + Assert.Equal(errorResponseBody, httpCallResponses.Error.Data[Http.DefaultSettings.ErrorDataPrefix + "Response.Body"]); + } + + [Fact] + public async Task WithErrorResponseBodyLogging_IfStatusCodeMatchesOneOfTheGiven_IncludeResponseBodyInException() + { + const string errorResponseBody = "{'Foo': 'Bar'}"; + _stubHttp.Stub(x => x.Get("/some-endpoint")) + .Return(errorResponseBody) + .WithStatus(HttpStatusCode.UnprocessableEntity); + + var response = await Http + .Request("http://localhost:9191/some-endpoint") + .WithErrorResponseBodyLogging(HttpStatusCode.NotAcceptable, HttpStatusCode.UnprocessableEntity) + .ExpectString() + .GetAsync(); + + Assert.Equal(HttpStatusCode.UnprocessableEntity,response.StatusCode); + + var httpCallResponses = Assert.IsType>(response); + Assert.Equal(errorResponseBody, httpCallResponses.Error.Data[Http.DefaultSettings.ErrorDataPrefix + "Response.Body"]); + } + + [Fact] + public async Task WithErrorResponseBodyLogging_IfStatusCodeDoesNotMatchAnyOfTheGiven_DoesNotIncludeResponseBodyInException() + { + const string errorResponseBody = "{'Foo': 'Bar'}"; + _stubHttp.Stub(x => x.Get("/some-endpoint")) + .Return(errorResponseBody) + .WithStatus(HttpStatusCode.UnprocessableEntity); + + var response = await Http + .Request("http://localhost:9191/some-endpoint") + .WithErrorResponseBodyLogging(HttpStatusCode.NotAcceptable, HttpStatusCode.BadRequest) + .ExpectString() + .GetAsync(); + + Assert.Equal(HttpStatusCode.UnprocessableEntity,response.StatusCode); + + var httpCallResponses = Assert.IsType>(response); + Assert.Null(httpCallResponses.Error.Data[Http.DefaultSettings.ErrorDataPrefix + "Response.Body"]); + } + + [Fact] + public async Task WithErrorResponseBodyLogging_IfNoStatusCodesGiven_DoesNotIncludeResponseBodyInException() + { + const string errorResponseBody = "{'Foo': 'Bar'}"; + _stubHttp.Stub(x => x.Get("/some-endpoint")) + .Return(errorResponseBody) + .WithStatus(HttpStatusCode.UnprocessableEntity); + + var response = await Http + .Request("http://localhost:9191/some-endpoint") + .WithErrorResponseBodyLogging() + .ExpectString() + .GetAsync(); + + Assert.Equal(HttpStatusCode.UnprocessableEntity,response.StatusCode); + + var httpCallResponses = Assert.IsType>(response); + Assert.Null(httpCallResponses.Error.Data[Http.DefaultSettings.ErrorDataPrefix + "Response.Body"]); + } + + [Fact] + public async Task WithErrorResponseBodyLogging_WithoutCallingWithErrorResponseBodyLogging_DoesNotIncludeResponseBodyInException() + { + const string errorResponseBody = "{'Foo': 'Bar'}"; + _stubHttp.Stub(x => x.Get("/some-endpoint")) + .Return(errorResponseBody) + .WithStatus(HttpStatusCode.UnprocessableEntity); + + var response = await Http + .Request("http://localhost:9191/some-endpoint") + .ExpectString() + .GetAsync(); + + Assert.Equal(HttpStatusCode.UnprocessableEntity,response.StatusCode); + + var httpCallResponses = Assert.IsType>(response); + Assert.Null(httpCallResponses.Error.Data[Http.DefaultSettings.ErrorDataPrefix + "Response.Body"]); + } + } +} From 25a1d0f32eaf13d27c926f9fe61e7100fbd7f703 Mon Sep 17 00:00:00 2001 From: Steffi Grewenig Date: Tue, 1 Jun 2021 10:03:38 +0200 Subject: [PATCH 4/4] Add unit test to ensure correct deserialization is not affected by the change --- .../HttpLogErrorResponseBodyTest.cs | 46 +++++++++++++++---- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/tests/StackExchange.Utils.Tests/HttpLogErrorResponseBodyTest.cs b/tests/StackExchange.Utils.Tests/HttpLogErrorResponseBodyTest.cs index 40694c7..de5ede3 100644 --- a/tests/StackExchange.Utils.Tests/HttpLogErrorResponseBodyTest.cs +++ b/tests/StackExchange.Utils.Tests/HttpLogErrorResponseBodyTest.cs @@ -20,12 +20,12 @@ public async Task WithErrorResponseBodyLogging_IfStatusCodeMatchesTheGivenOne_In var response = await Http .Request("http://localhost:9191/some-endpoint") .WithErrorResponseBodyLogging(HttpStatusCode.UnprocessableEntity) - .ExpectString() + .ExpectJson() .GetAsync(); Assert.Equal(HttpStatusCode.UnprocessableEntity,response.StatusCode); - var httpCallResponses = Assert.IsType>(response); + var httpCallResponses = Assert.IsType>(response); Assert.Equal(errorResponseBody, httpCallResponses.Error.Data[Http.DefaultSettings.ErrorDataPrefix + "Response.Body"]); } @@ -40,12 +40,12 @@ public async Task WithErrorResponseBodyLogging_IfStatusCodeMatchesOneOfTheGiven_ var response = await Http .Request("http://localhost:9191/some-endpoint") .WithErrorResponseBodyLogging(HttpStatusCode.NotAcceptable, HttpStatusCode.UnprocessableEntity) - .ExpectString() + .ExpectJson() .GetAsync(); Assert.Equal(HttpStatusCode.UnprocessableEntity,response.StatusCode); - var httpCallResponses = Assert.IsType>(response); + var httpCallResponses = Assert.IsType>(response); Assert.Equal(errorResponseBody, httpCallResponses.Error.Data[Http.DefaultSettings.ErrorDataPrefix + "Response.Body"]); } @@ -60,12 +60,12 @@ public async Task WithErrorResponseBodyLogging_IfStatusCodeDoesNotMatchAnyOfTheG var response = await Http .Request("http://localhost:9191/some-endpoint") .WithErrorResponseBodyLogging(HttpStatusCode.NotAcceptable, HttpStatusCode.BadRequest) - .ExpectString() + .ExpectJson() .GetAsync(); Assert.Equal(HttpStatusCode.UnprocessableEntity,response.StatusCode); - var httpCallResponses = Assert.IsType>(response); + var httpCallResponses = Assert.IsType>(response); Assert.Null(httpCallResponses.Error.Data[Http.DefaultSettings.ErrorDataPrefix + "Response.Body"]); } @@ -80,12 +80,12 @@ public async Task WithErrorResponseBodyLogging_IfNoStatusCodesGiven_DoesNotInclu var response = await Http .Request("http://localhost:9191/some-endpoint") .WithErrorResponseBodyLogging() - .ExpectString() + .ExpectJson() .GetAsync(); Assert.Equal(HttpStatusCode.UnprocessableEntity,response.StatusCode); - var httpCallResponses = Assert.IsType>(response); + var httpCallResponses = Assert.IsType>(response); Assert.Null(httpCallResponses.Error.Data[Http.DefaultSettings.ErrorDataPrefix + "Response.Body"]); } @@ -99,13 +99,39 @@ public async Task WithErrorResponseBodyLogging_WithoutCallingWithErrorResponseBo var response = await Http .Request("http://localhost:9191/some-endpoint") - .ExpectString() + .ExpectJson() .GetAsync(); Assert.Equal(HttpStatusCode.UnprocessableEntity,response.StatusCode); - var httpCallResponses = Assert.IsType>(response); + var httpCallResponses = Assert.IsType>(response); Assert.Null(httpCallResponses.Error.Data[Http.DefaultSettings.ErrorDataPrefix + "Response.Body"]); } + + [Fact] + public async Task WithErrorResponseBodyLogging_IfResponseSuccess_DoesNotIncludeResponseBodyInExceptionAndDeserializesCorrectly() + { + const string successResponseBody = @"{""SomeAttribute"": ""some value""}"; + _stubHttp.Stub(x => x.Get("/some-endpoint")) + .Return(successResponseBody) + .WithStatus(HttpStatusCode.OK); + + var response = await Http + .Request("http://localhost:9191/some-endpoint") + .WithErrorResponseBodyLogging(HttpStatusCode.UnprocessableEntity) + .ExpectJson() + .GetAsync(); + + Assert.Equal(HttpStatusCode.OK,response.StatusCode); + + var httpCallResponses = Assert.IsType>(response); + Assert.Null(httpCallResponses.Error); + Assert.Equal("some value", httpCallResponses.Data.SomeAttribute); + } + } + + public class SomeResponseObject + { + public string SomeAttribute { get; set; } } }