From f0667471dd752ab0893e7ed024c40aeab3f5276c Mon Sep 17 00:00:00 2001 From: jlukawska Date: Thu, 7 Nov 2019 09:18:05 +0100 Subject: [PATCH] #492 log 500 with error log level, acceptance test, unit test --- .../Middleware/HttpRequesterMiddleware.cs | 7 ++- .../Ocelot.AcceptanceTests.csproj | 1 + .../ReturnsErrorTests.cs | 36 +++++++++++- test/Ocelot.AcceptanceTests/Steps.cs | 57 +++++++++++++++++++ .../Requester/HttpRequesterMiddlewareTests.cs | 21 +++++++ 5 files changed, 120 insertions(+), 2 deletions(-) diff --git a/src/Ocelot/Requester/Middleware/HttpRequesterMiddleware.cs b/src/Ocelot/Requester/Middleware/HttpRequesterMiddleware.cs index 92d3128e..22e4d8f8 100644 --- a/src/Ocelot/Requester/Middleware/HttpRequesterMiddleware.cs +++ b/src/Ocelot/Requester/Middleware/HttpRequesterMiddleware.cs @@ -20,7 +20,12 @@ namespace Ocelot.Requester.Middleware public async Task Invoke(DownstreamContext context) { - var response = await _requester.GetResponse(context); + var response = await _requester.GetResponse(context); + + if (response.Data != null && response.Data.StatusCode == System.Net.HttpStatusCode.InternalServerError) + { + Logger.LogError("500 (Internal Server Error) status code, request uri: " + response.Data.RequestMessage?.RequestUri, null); + } if (response.IsError) { diff --git a/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj b/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj index 762448e7..38f2ecca 100644 --- a/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj +++ b/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj @@ -40,6 +40,7 @@ + all diff --git a/test/Ocelot.AcceptanceTests/ReturnsErrorTests.cs b/test/Ocelot.AcceptanceTests/ReturnsErrorTests.cs index f72efff8..8f9a6455 100644 --- a/test/Ocelot.AcceptanceTests/ReturnsErrorTests.cs +++ b/test/Ocelot.AcceptanceTests/ReturnsErrorTests.cs @@ -20,7 +20,8 @@ [Fact] public void should_return_internal_server_error_if_downstream_service_returns_internal_server_error() - { + { + var configuration = new FileConfiguration { ReRoutes = new List @@ -49,6 +50,39 @@ .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.InternalServerError)) .BDDfy(); + } + + [Fact] + public void should_log_error_if_downstream_service_returns_internal_server_error() + { + var configuration = new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/", + UpstreamPathTemplate = "/", + UpstreamHttpMethod = new List { "Get" }, + DownstreamHostAndPorts = new List + { + new FileHostAndPort + { + Host = "localhost", + Port = 53876, + }, + }, + DownstreamScheme = "http", + }, + }, + }; + + this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:53876")) + .And(x => _steps.GivenThereIsAConfiguration(configuration)) + .And(x => _steps.GivenOcelotIsRunningWithLogger()) + .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) + .Then(x => _steps.ThenErrorShouldBeLogged()) + .BDDfy(); } private void GivenThereIsAServiceRunningOn(string url) diff --git a/test/Ocelot.AcceptanceTests/Steps.cs b/test/Ocelot.AcceptanceTests/Steps.cs index 6b9870bd..0fbb0105 100644 --- a/test/Ocelot.AcceptanceTests/Steps.cs +++ b/test/Ocelot.AcceptanceTests/Steps.cs @@ -10,12 +10,14 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; + using Moq; using Newtonsoft.Json; using Ocelot.Cache.CacheManager; using Ocelot.Configuration.Creator; using Ocelot.Configuration.File; using Ocelot.DependencyInjection; using Ocelot.Infrastructure; + using Ocelot.Logging; using Ocelot.Middleware; using Ocelot.Middleware.Multiplexer; using Ocelot.Provider.Consul; @@ -1120,5 +1122,60 @@ _ocelotClient = _ocelotServer.CreateClient(); } + + public void GivenOcelotIsRunningWithLogger() + { + _webHostBuilder = new WebHostBuilder(); + + _webHostBuilder + .ConfigureAppConfiguration((hostingContext, config) => + { + config.SetBasePath(hostingContext.HostingEnvironment.ContentRootPath); + var env = hostingContext.HostingEnvironment; + config.AddJsonFile("appsettings.json", optional: true, reloadOnChange: false) + .AddJsonFile($"appsettings.{env.EnvironmentName}.json", optional: true, reloadOnChange: false); + config.AddJsonFile("ocelot.json", false, false); + config.AddEnvironmentVariables(); + }) + .ConfigureServices(s => + { + s.AddOcelot(); + s.AddSingleton(); + }) + .Configure(app => + { + app.UseOcelot().Wait(); + }); + + _ocelotServer = new TestServer(_webHostBuilder); + + _ocelotClient = _ocelotServer.CreateClient(); + } + + public void ThenErrorShouldBeLogged() + { + MockLoggerFactory loggerFactory = (MockLoggerFactory)_ocelotServer.Host.Services.GetService(); + loggerFactory.Verify(); + } + + internal class MockLoggerFactory : IOcelotLoggerFactory + { + private Mock _logger; + + public IOcelotLogger CreateLogger() + { + if (_logger == null) + { + _logger = new Mock(); + _logger.Setup(x => x.LogError(It.IsAny(), It.IsAny())).Verifiable(); + } + return _logger.Object; + } + + public void Verify() + { + _logger.Verify(x => x.LogError(It.IsAny(), It.IsAny()), Times.Once); + } + } } } diff --git a/test/Ocelot.UnitTests/Requester/HttpRequesterMiddlewareTests.cs b/test/Ocelot.UnitTests/Requester/HttpRequesterMiddlewareTests.cs index a597df6b..8b4c53c5 100644 --- a/test/Ocelot.UnitTests/Requester/HttpRequesterMiddlewareTests.cs +++ b/test/Ocelot.UnitTests/Requester/HttpRequesterMiddlewareTests.cs @@ -1,6 +1,7 @@ namespace Ocelot.UnitTests.Requester { using Microsoft.AspNetCore.Http; + using Microsoft.Extensions.Logging; using Moq; using Ocelot.Configuration.Builder; using Ocelot.Logging; @@ -57,6 +58,17 @@ namespace Ocelot.UnitTests.Requester .BDDfy(); } + [Fact] + public void should_log_downstream_internal_server_error() + { + this.Given(x => x.GivenTheRequestIs()) + .And(x => x.GivenTheRequesterReturns( + new OkResponse(new HttpResponseMessage(System.Net.HttpStatusCode.InternalServerError)))) + .When(x => x.WhenICallTheMiddleware()) + .Then(x => x.ErrorIsLogged()) + .BDDfy(); + } + private void ThenTheErrorIsSet() { _downstreamContext.IsError.ShouldBeTrue(); @@ -98,5 +110,14 @@ namespace Ocelot.UnitTests.Requester _downstreamContext.DownstreamResponse.Content.ShouldBe(_response.Data.Content); _downstreamContext.DownstreamResponse.StatusCode.ShouldBe(_response.Data.StatusCode); } + + private void ErrorIsLogged() + { + _logger.Verify( + x => x.LogError( + It.IsAny(), + It.IsAny() + )); + } } }