Merge branch 'jlukawska-feature/492-log-500-status-code-as-error' into develop

Ocelot.sln

@@ -9,23 +9,13 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
 	ProjectSection(SolutionItems) = preProject
 		.dockerignore = .dockerignore
 		.gitignore = .gitignore
-		build-and-release-unstable.ps1 = build-and-release-unstable.ps1
-		build-and-run-tests.ps1 = build-and-run-tests.ps1
 		build.cake = build.cake
 		build.ps1 = build.ps1
 		codeanalysis.ruleset = codeanalysis.ruleset
-		docker-compose.yaml = docker-compose.yaml
-		Dockerfile = Dockerfile
 		GitVersion.yml = GitVersion.yml
-		global.json = global.json
 		LICENSE.md = LICENSE.md
 		README.md = README.md
-		release.ps1 = release.ps1
 		ReleaseNotes.md = ReleaseNotes.md
-		run-acceptance-tests.ps1 = run-acceptance-tests.ps1
-		run-benchmarks.ps1 = run-benchmarks.ps1
-		run-unit-tests.ps1 = run-unit-tests.ps1
-		version.ps1 = version.ps1
 	EndProjectSection
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "test", "test", "{5B401523-36DA-4491-B73A-7590A26E420B}"

samples/OcelotBasic/Properties/launchSettings.json

@@ -0,0 +1,27 @@
+{
+  "iisSettings": {
+    "windowsAuthentication": false,
+    "anonymousAuthentication": true,
+    "iisExpress": {
+      "applicationUrl": "http://localhost:55029/",
+      "sslPort": 44390
+    }
+  },
+  "profiles": {
+    "IIS Express": {
+      "commandName": "IISExpress",
+      "launchBrowser": true,
+      "environmentVariables": {
+        "ASPNETCORE_ENVIRONMENT": "Development"
+      }
+    },
+    "OcelotBasic": {
+      "commandName": "Project",
+      "launchBrowser": true,
+      "environmentVariables": {
+        "ASPNETCORE_ENVIRONMENT": "Development"
+      },
+      "applicationUrl": "https://localhost:5001;http://localhost:5000"
+    }
+  }
+}

src/Ocelot/Requester/Middleware/HttpRequesterMiddleware.cs

@@ -1,6 +1,9 @@
+using System.Net;
+using System.Net.Http;
 using Ocelot.Logging;
 using Ocelot.Middleware;
 using System.Threading.Tasks;
+using Ocelot.Responses;
 
 namespace Ocelot.Requester.Middleware
 {
@@ -22,6 +25,8 @@ namespace Ocelot.Requester.Middleware
         {
             var response = await _requester.GetResponse(context);
 
+            CreateLogBasedOnResponse(response);
+
             if (response.IsError)
             {
                 Logger.LogDebug("IHttpRequester returned an error, setting pipeline error");
@@ -36,5 +41,19 @@ namespace Ocelot.Requester.Middleware
 
             await _next.Invoke(context);
         }
+
+        private void CreateLogBasedOnResponse(Response<HttpResponseMessage> response)
+        {
+            if (response.Data?.StatusCode <= HttpStatusCode.BadRequest)
+            {
+                Logger.LogInformation(
+                    $"{(int)response.Data.StatusCode} ({response.Data.ReasonPhrase}) status code, request uri: {response.Data.RequestMessage?.RequestUri}");
+            } 
+            else if (response.Data?.StatusCode >= HttpStatusCode.BadRequest)
+            {
+                Logger.LogWarning(
+                    $"{(int) response.Data.StatusCode} ({response.Data.ReasonPhrase}) status code, request uri: {response.Data.RequestMessage?.RequestUri}");
+            }
+        }
     }
 }

test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <VersionPrefix>0.0.0-dev</VersionPrefix>
     <TargetFramework>netcoreapp3.1</TargetFramework>
@@ -40,6 +40,7 @@
   <ItemGroup>
     <PackageReference Include="Microsoft.AspNetCore.TestHost" Version="3.0.0" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.3.0" />
+    <PackageReference Include="Moq" Version="4.13.0" />
     <PackageReference Include="StyleCop.Analyzers" Version="1.2.0-beta.66">
       <PrivateAssets>all</PrivateAssets>
     </PackageReference>

test/Ocelot.AcceptanceTests/ResponseCodeTests.cs

@@ -34,7 +34,7 @@ namespace Ocelot.AcceptanceTests
                                 new FileHostAndPort
                                 {
                                     Host = "localhost",
-                                    Port = 50092,
+                                    Port = 51092,
                                 }
                             },
                             UpstreamPathTemplate = "/{everything}",
@@ -43,7 +43,7 @@ namespace Ocelot.AcceptanceTests
                     }
             };
 
-            this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:50092", "/inline.132.bundle.js", 304))
+            this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:51092", "/inline.132.bundle.js", 304))
                 .And(x => _steps.GivenThereIsAConfiguration(configuration))
                 .And(x => _steps.GivenOcelotIsRunning())
                 .When(x => _steps.WhenIGetUrlOnTheApiGateway("/inline.132.bundle.js"))

test/Ocelot.AcceptanceTests/ReturnsErrorTests.cs

@@ -21,6 +21,7 @@
         [Fact]
         public void should_return_internal_server_error_if_downstream_service_returns_internal_server_error()
         {
+
             var configuration = new FileConfiguration
             {
                 ReRoutes = new List<FileReRoute>
@@ -51,6 +52,39 @@
                 .BDDfy();
         }
 
+        [Fact]
+        public void should_log_warning_if_downstream_service_returns_internal_server_error()
+        {
+            var configuration = new FileConfiguration
+            {
+                ReRoutes = new List<FileReRoute>
+                    {
+                        new FileReRoute
+                        {
+                            DownstreamPathTemplate = "/",
+                            UpstreamPathTemplate = "/",
+                            UpstreamHttpMethod = new List<string> { "Get" },
+                            DownstreamHostAndPorts = new List<FileHostAndPort>
+                            {
+                                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.ThenWarningShouldBeLogged())
+                .BDDfy();
+        }
+
         private void GivenThereIsAServiceRunningOn(string url)
         {
             _serviceHandler.GivenThereIsAServiceRunningOn(url, context => throw new Exception("BLAMMMM"));

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<IOcelotLoggerFactory, MockLoggerFactory>();
+                })
+                .Configure(app =>
+                {
+                    app.UseOcelot().Wait();
+                });
+
+            _ocelotServer = new TestServer(_webHostBuilder);
+
+            _ocelotClient = _ocelotServer.CreateClient();
+        }
+
+        public void ThenWarningShouldBeLogged()
+        {
+            MockLoggerFactory loggerFactory = (MockLoggerFactory)_ocelotServer.Host.Services.GetService<IOcelotLoggerFactory>();
+            loggerFactory.Verify();
+        }
+
+        internal class MockLoggerFactory : IOcelotLoggerFactory
+        {
+            private Mock<IOcelotLogger> _logger;
+
+            public IOcelotLogger CreateLogger<T>()
+            {
+                if (_logger == null)
+                {
+                    _logger = new Mock<IOcelotLogger>();
+                    _logger.Setup(x => x.LogWarning(It.IsAny<string>())).Verifiable();
+                }
+                return _logger.Object;
+            }
+
+            public void Verify()
+            {
+                _logger.Verify(x => x.LogWarning(It.IsAny<string>()), Times.Once);
+            }
+        }
     }
 }

test/Ocelot.UnitTests/Requester/HttpRequesterMiddlewareTests.cs

@@ -41,9 +41,10 @@ namespace Ocelot.UnitTests.Requester
         public void should_call_services_correctly()
         {
             this.Given(x => x.GivenTheRequestIs())
-                .And(x => x.GivenTheRequesterReturns(new OkResponse<HttpResponseMessage>(new HttpResponseMessage())))
+                .And(x => x.GivenTheRequesterReturns(new OkResponse<HttpResponseMessage>(new HttpResponseMessage(System.Net.HttpStatusCode.OK))))
                 .When(x => x.WhenICallTheMiddleware())
                 .Then(x => x.ThenTheDownstreamResponseIsSet())
+                .Then(x => InformationIsLogged())
                 .BDDfy();
         }
 
@@ -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<HttpResponseMessage>(new HttpResponseMessage(System.Net.HttpStatusCode.InternalServerError))))
+                .When(x => x.WhenICallTheMiddleware())
+                .Then(x => x.WarningIsLogged())
+                .BDDfy();
+        }
+
         private void ThenTheErrorIsSet()
         {
             _downstreamContext.IsError.ShouldBeTrue();
@@ -98,5 +110,23 @@ namespace Ocelot.UnitTests.Requester
             _downstreamContext.DownstreamResponse.Content.ShouldBe(_response.Data.Content);
             _downstreamContext.DownstreamResponse.StatusCode.ShouldBe(_response.Data.StatusCode);
         }
+
+        private void WarningIsLogged()
+        {
+            _logger.Verify(
+                x => x.LogWarning(                 
+                    It.IsAny<string>()
+                   ),
+                Times.Once);
+        }
+
+        private void InformationIsLogged()
+        {
+            _logger.Verify(
+                x => x.LogInformation(
+                    It.IsAny<string>()
+                ),
+                Times.Once);
+        }
     }
 }