From f35a07a3daceeb5037c5e0607fb6812c74d3f949 Mon Sep 17 00:00:00 2001 From: jlukawska Date: Fri, 22 Nov 2019 08:01:02 +0100 Subject: [PATCH 1/4] #603 return 502 when request cannot be completed --- .../Responder/ErrorsToHttpStatusCodeMapper.cs | 2 +- .../ReturnsErrorTests.cs | 32 +++++++++++++++++++ test/Ocelot.AcceptanceTests/SslTests.cs | 2 +- .../ErrorsToHttpStatusCodeMapperTests.cs | 4 +-- 4 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs b/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs index c50daa27..718f4e3b 100644 --- a/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs +++ b/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs @@ -42,7 +42,7 @@ namespace Ocelot.Responder if (errors.Any(e => e.Code == OcelotErrorCode.UnableToCompleteRequestError)) { - return 500; + return 502; } return 404; diff --git a/test/Ocelot.AcceptanceTests/ReturnsErrorTests.cs b/test/Ocelot.AcceptanceTests/ReturnsErrorTests.cs index f72efff8..f18b5cad 100644 --- a/test/Ocelot.AcceptanceTests/ReturnsErrorTests.cs +++ b/test/Ocelot.AcceptanceTests/ReturnsErrorTests.cs @@ -16,6 +16,38 @@ { _serviceHandler = new ServiceHandler(); _steps = new Steps(); + } + + [Fact] + public void should_return_bad_gateway_error_if_downstream_service_doesnt_respond() + { + 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 => _steps.GivenThereIsAConfiguration(configuration)) + .And(x => _steps.GivenOcelotIsRunning()) + .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) + .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.BadGateway)) + .BDDfy(); } [Fact] diff --git a/test/Ocelot.AcceptanceTests/SslTests.cs b/test/Ocelot.AcceptanceTests/SslTests.cs index a0ea17a4..800f1e43 100644 --- a/test/Ocelot.AcceptanceTests/SslTests.cs +++ b/test/Ocelot.AcceptanceTests/SslTests.cs @@ -89,7 +89,7 @@ namespace Ocelot.AcceptanceTests .And(x => _steps.GivenThereIsAConfiguration(configuration)) .And(x => _steps.GivenOcelotIsRunning()) .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) - .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.InternalServerError)) + .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.BadGateway)) .BDDfy(); } diff --git a/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs b/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs index 8b550a05..af104aa6 100644 --- a/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs +++ b/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs @@ -47,9 +47,9 @@ namespace Ocelot.UnitTests.Responder [Theory] [InlineData(OcelotErrorCode.UnableToCompleteRequestError)] - public void should_return_internal_server_error(OcelotErrorCode errorCode) + public void should_return_bad_gateway_error(OcelotErrorCode errorCode) { - ShouldMapErrorToStatusCode(errorCode, HttpStatusCode.InternalServerError); + ShouldMapErrorToStatusCode(errorCode, HttpStatusCode.BadGateway); } [Theory] From 68e3ae8cdd61a829bd3dbd6b241bfff540e9b52c Mon Sep 17 00:00:00 2001 From: jlukawska Date: Wed, 22 Jan 2020 14:55:41 +0100 Subject: [PATCH 2/4] #603 return 502 on HttpRequestException --- src/Ocelot/Errors/OcelotErrorCode.cs | 3 ++- .../Requester/ConnectionToDownstreamServiceError.cs | 13 +++++++++++++ src/Ocelot/Requester/HttpExeptionToErrorMapper.cs | 7 +++++++ .../Responder/ErrorsToHttpStatusCodeMapper.cs | 7 ++++++- test/Ocelot.AcceptanceTests/SslTests.cs | 2 +- .../Responder/ErrorsToHttpStatusCodeMapperTests.cs | 9 ++++++++- 6 files changed, 37 insertions(+), 4 deletions(-) create mode 100644 src/Ocelot/Requester/ConnectionToDownstreamServiceError.cs diff --git a/src/Ocelot/Errors/OcelotErrorCode.cs b/src/Ocelot/Errors/OcelotErrorCode.cs index d8b6f333..6ca67e7d 100644 --- a/src/Ocelot/Errors/OcelotErrorCode.cs +++ b/src/Ocelot/Errors/OcelotErrorCode.cs @@ -39,6 +39,7 @@ CannotAddPlaceholderError = 34, CannotRemovePlaceholderError = 35, QuotaExceededError = 36, - RequestCanceled = 37, + RequestCanceled = 37, + ConnectionToDownstreamServiceError = 38, } } diff --git a/src/Ocelot/Requester/ConnectionToDownstreamServiceError.cs b/src/Ocelot/Requester/ConnectionToDownstreamServiceError.cs new file mode 100644 index 00000000..d2087983 --- /dev/null +++ b/src/Ocelot/Requester/ConnectionToDownstreamServiceError.cs @@ -0,0 +1,13 @@ +using Ocelot.Errors; +using System; + +namespace Ocelot.Requester +{ + class ConnectionToDownstreamServiceError : Error + { + public ConnectionToDownstreamServiceError(Exception exception) + : base($"Error connecting to downstream service, exception: {exception}", OcelotErrorCode.ConnectionToDownstreamServiceError) + { + } + } +} diff --git a/src/Ocelot/Requester/HttpExeptionToErrorMapper.cs b/src/Ocelot/Requester/HttpExeptionToErrorMapper.cs index 100d01f4..80cec0ea 100644 --- a/src/Ocelot/Requester/HttpExeptionToErrorMapper.cs +++ b/src/Ocelot/Requester/HttpExeptionToErrorMapper.cs @@ -4,6 +4,8 @@ namespace Ocelot.Requester using Microsoft.Extensions.DependencyInjection; using System; using System.Collections.Generic; + using System.Net.Http; + using System.Net.Sockets; public class HttpExeptionToErrorMapper : IExceptionToErrorMapper { @@ -28,6 +30,11 @@ namespace Ocelot.Requester return new RequestCanceledError(exception.Message); } + if (type == typeof(HttpRequestException)) + { + return new ConnectionToDownstreamServiceError(exception); + } + return new UnableToCompleteRequestError(exception); } } diff --git a/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs b/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs index 718f4e3b..6b0ee6cc 100644 --- a/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs +++ b/src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs @@ -38,11 +38,16 @@ namespace Ocelot.Responder if (errors.Any(e => e.Code == OcelotErrorCode.UnableToFindDownstreamRouteError)) { return 404; + } + + if (errors.Any(e => e.Code == OcelotErrorCode.ConnectionToDownstreamServiceError)) + { + return 502; } if (errors.Any(e => e.Code == OcelotErrorCode.UnableToCompleteRequestError)) { - return 502; + return 500; } return 404; diff --git a/test/Ocelot.AcceptanceTests/SslTests.cs b/test/Ocelot.AcceptanceTests/SslTests.cs index 800f1e43..a0ea17a4 100644 --- a/test/Ocelot.AcceptanceTests/SslTests.cs +++ b/test/Ocelot.AcceptanceTests/SslTests.cs @@ -89,7 +89,7 @@ namespace Ocelot.AcceptanceTests .And(x => _steps.GivenThereIsAConfiguration(configuration)) .And(x => _steps.GivenOcelotIsRunning()) .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) - .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.BadGateway)) + .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.InternalServerError)) .BDDfy(); } diff --git a/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs b/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs index af104aa6..d662f86f 100644 --- a/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs +++ b/test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs @@ -47,6 +47,13 @@ namespace Ocelot.UnitTests.Responder [Theory] [InlineData(OcelotErrorCode.UnableToCompleteRequestError)] + public void should_return_internal_server_error(OcelotErrorCode errorCode) + { + ShouldMapErrorToStatusCode(errorCode, HttpStatusCode.InternalServerError); + } + + [Theory] + [InlineData(OcelotErrorCode.ConnectionToDownstreamServiceError)] public void should_return_bad_gateway_error(OcelotErrorCode errorCode) { ShouldMapErrorToStatusCode(errorCode, HttpStatusCode.BadGateway); @@ -125,7 +132,7 @@ namespace Ocelot.UnitTests.Responder // If this test fails then it's because the number of error codes has changed. // You should make the appropriate changes to the test cases here to ensure // they cover all the error codes, and then modify this assertion. - Enum.GetNames(typeof(OcelotErrorCode)).Length.ShouldBe(38, "Looks like the number of error codes has changed. Do you need to modify ErrorsToHttpStatusCodeMapper?"); + Enum.GetNames(typeof(OcelotErrorCode)).Length.ShouldBe(39, "Looks like the number of error codes has changed. Do you need to modify ErrorsToHttpStatusCodeMapper?"); } private void ShouldMapErrorToStatusCode(OcelotErrorCode errorCode, HttpStatusCode expectedHttpStatusCode) From cbb21a13b307c2838e95387d69d1606cd89eb180 Mon Sep 17 00:00:00 2001 From: jlukawska Date: Wed, 22 Jan 2020 16:14:20 +0100 Subject: [PATCH 3/4] #603 return 502 on HttpRequestException --- test/Ocelot.AcceptanceTests/ReturnsErrorTests.cs | 2 +- test/Ocelot.AcceptanceTests/SslTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Ocelot.AcceptanceTests/ReturnsErrorTests.cs b/test/Ocelot.AcceptanceTests/ReturnsErrorTests.cs index ef44acad..293367f0 100644 --- a/test/Ocelot.AcceptanceTests/ReturnsErrorTests.cs +++ b/test/Ocelot.AcceptanceTests/ReturnsErrorTests.cs @@ -35,7 +35,7 @@ new FileHostAndPort { Host = "localhost", - Port = 53876, + Port = 53877, }, }, DownstreamScheme = "http", diff --git a/test/Ocelot.AcceptanceTests/SslTests.cs b/test/Ocelot.AcceptanceTests/SslTests.cs index a0ea17a4..800f1e43 100644 --- a/test/Ocelot.AcceptanceTests/SslTests.cs +++ b/test/Ocelot.AcceptanceTests/SslTests.cs @@ -89,7 +89,7 @@ namespace Ocelot.AcceptanceTests .And(x => _steps.GivenThereIsAConfiguration(configuration)) .And(x => _steps.GivenOcelotIsRunning()) .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) - .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.InternalServerError)) + .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.BadGateway)) .BDDfy(); } From 8ead5114c3bcd1ca61fe814c47afd475cb666736 Mon Sep 17 00:00:00 2001 From: jlukawska Date: Wed, 11 Mar 2020 22:17:47 +0100 Subject: [PATCH 4/4] update acceptance tests --- test/Ocelot.AcceptanceTests/HttpTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Ocelot.AcceptanceTests/HttpTests.cs b/test/Ocelot.AcceptanceTests/HttpTests.cs index 57947d07..da59d000 100644 --- a/test/Ocelot.AcceptanceTests/HttpTests.cs +++ b/test/Ocelot.AcceptanceTests/HttpTests.cs @@ -141,7 +141,7 @@ namespace Ocelot.AcceptanceTests } [Fact] - public void should_return_response_500_when_using_http_one_to_talk_to_server_running_http_two() + public void should_return_response_502_when_using_http_one_to_talk_to_server_running_http_two() { var port = RandomPortFinder.GetRandomPort(); @@ -177,7 +177,7 @@ namespace Ocelot.AcceptanceTests .And(x => _steps.GivenThereIsAConfiguration(configuration)) .And(x => _steps.GivenOcelotIsRunning()) .When(x => _steps.WhenIGetUrlOnTheApiGateway("/", httpContent)) - .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.InternalServerError)) + .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.BadGateway)) .BDDfy(); }