more error handling and docs

docs/features/loadbalancer.rst

@@ -214,6 +214,6 @@ There are numerous extension methods to add a custom load balancer and the inter
             Func<IServiceProvider, DownstreamReRoute, IServiceDiscoveryProvider, T> loadBalancerFactoryFunc)
             where T : ILoadBalancer;
 
-When you enable custom load balancers Ocelot looks up your load balancer by its class name when it decides if it should do load balancing. If it finds a match it will load balance your request. If Ocelot cannot match the load balancer type in your configuration with the name of registered load balancer class then you will receive a HTTP 500 internal server error.
+When you enable custom load balancers Ocelot looks up your load balancer by its class name when it decides if it should do load balancing. If it finds a match it will use your load balaner to load balance. If Ocelot cannot match the load balancer type in your configuration with the name of registered load balancer class then you will receive a HTTP 500 internal server error. If your load balancer factory throw an exception when Ocelot calls it you will receive a HTTP 500 internal server error.
 
 Remember if you specify no load balancer in your config Ocelot will not try and load balance.

src/Ocelot/Errors/OcelotErrorCode.cs

@@ -42,5 +42,6 @@
         RequestCanceled = 37,
         ConnectionToDownstreamServiceError = 38,
         CouldNotFindLoadBalancerCreator = 39,
+        ErrorInvokingLoadBalancerCreator = 40,
     }
 }

src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessionsCreator.cs

@@ -4,15 +4,16 @@
     using Ocelot.Configuration;
     using Ocelot.Infrastructure;
     using Ocelot.ServiceDiscovery.Providers;
+    using Ocelot.Responses;
 
     public class CookieStickySessionsCreator : ILoadBalancerCreator
     {
-        public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider)
+        public Response<ILoadBalancer> Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider)
         {
             var loadBalancer = new RoundRobin(async () => await serviceProvider.Get());
             var bus = new InMemoryBus<StickySession>();
-            return new CookieStickySessions(loadBalancer, reRoute.LoadBalancerOptions.Key,
+            return new OkResponse<ILoadBalancer>(new CookieStickySessions(loadBalancer, reRoute.LoadBalancerOptions.Key,
-                reRoute.LoadBalancerOptions.ExpiryInMs, bus);
+                reRoute.LoadBalancerOptions.ExpiryInMs, bus));
         }
 
         public string Type => nameof(CookieStickySessions);

src/Ocelot/LoadBalancer/LoadBalancers/DelegateInvokingLoadBalancerCreator.cs

@@ -3,6 +3,7 @@
     using System;
     using Ocelot.Configuration;
     using Ocelot.ServiceDiscovery.Providers;
+    using Ocelot.Responses;
 
     public class DelegateInvokingLoadBalancerCreator<T> : ILoadBalancerCreator
         where T : ILoadBalancer
@@ -15,9 +16,17 @@
             _creatorFunc = creatorFunc;
         }
 
-        public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider)
+        public Response<ILoadBalancer> Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider)
         {
-            return _creatorFunc(reRoute, serviceProvider);
+            try
+            {
+                return new OkResponse<ILoadBalancer>(_creatorFunc(reRoute, serviceProvider));
+
+            }
+            catch (Exception e)
+            {
+                return new ErrorResponse<ILoadBalancer>(new ErrorInvokingLoadBalancerCreator(e));
+            }
         }
 
         public string Type => typeof(T).Name;

src/Ocelot/LoadBalancer/LoadBalancers/ErrorInvokingLoadBalancerCreator.cs

@@ -0,0 +1,12 @@
+namespace Ocelot.LoadBalancer.LoadBalancers
+{
+    using System;
+    using Errors;
+
+    public class ErrorInvokingLoadBalancerCreator : Error
+    {
+        public ErrorInvokingLoadBalancerCreator(Exception e) : base($"Error when invoking user provided load balancer creator function, Message: {e.Message}, StackTrace: {e.StackTrace}", OcelotErrorCode.ErrorInvokingLoadBalancerCreator)
+        {
+        }
+    }
+}

src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancerCreator.cs

@@ -1,11 +1,12 @@
-using Ocelot.Configuration;
+namespace Ocelot.LoadBalancer.LoadBalancers
+{
+    using Ocelot.Responses;
+    using Ocelot.Configuration;
     using Ocelot.ServiceDiscovery.Providers;
 
-namespace Ocelot.LoadBalancer.LoadBalancers
-{
     public interface ILoadBalancerCreator
     {
-        ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider);
+        Response<ILoadBalancer> Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider);
         string Type { get; }
     }
 }

src/Ocelot/LoadBalancer/LoadBalancers/LeastConnectionCreator.cs

@@ -2,12 +2,13 @@
 {
     using Ocelot.Configuration;
     using Ocelot.ServiceDiscovery.Providers;
+    using Ocelot.Responses;
 
     public class LeastConnectionCreator : ILoadBalancerCreator
     {
-        public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider)
+        public Response<ILoadBalancer> Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider)
         {
-            return new LeastConnection(async () => await serviceProvider.Get(), reRoute.ServiceName);
+            return new OkResponse<ILoadBalancer>(new LeastConnection(async () => await serviceProvider.Get(), reRoute.ServiceName));
         }
 
         public string Type => nameof(LeastConnection);

src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerFactory.cs

@@ -35,8 +35,14 @@
                 return new ErrorResponse<ILoadBalancer>(new CouldNotFindLoadBalancerCreator($"Could not find load balancer creator for Type: {requestedType}, please check your config specified the correct load balancer and that you have registered a class with the same name."));
             }
 
-            var createdLoadBalancer = applicableCreator.Create(reRoute, serviceProvider);
+            var createdLoadBalancerResponse = applicableCreator.Create(reRoute, serviceProvider);
-            return new OkResponse<ILoadBalancer>(createdLoadBalancer);
+
+            if (createdLoadBalancerResponse.IsError)
+            {
+                return new ErrorResponse<ILoadBalancer>(createdLoadBalancerResponse.Errors);
+            }
+
+            return new OkResponse<ILoadBalancer>(createdLoadBalancerResponse.Data);
         }
     }
 }

src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancerCreator.cs

@@ -2,12 +2,13 @@
 { 
     using Ocelot.Configuration;
     using Ocelot.ServiceDiscovery.Providers;
+    using Ocelot.Responses;
 
     public class NoLoadBalancerCreator : ILoadBalancerCreator
     {
-        public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider)
+        public Response<ILoadBalancer> Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider)
         {
-            return new NoLoadBalancer(async () => await serviceProvider.Get());
+            return new OkResponse<ILoadBalancer>(new NoLoadBalancer(async () => await serviceProvider.Get()));
         }
 
         public string Type => nameof(NoLoadBalancer);

src/Ocelot/LoadBalancer/LoadBalancers/RoundRobinCreator.cs

@@ -2,12 +2,13 @@
 {
     using Ocelot.Configuration;
     using Ocelot.ServiceDiscovery.Providers;
+    using Ocelot.Responses;
 
     public class RoundRobinCreator : ILoadBalancerCreator
     {
-        public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider)
+        public Response<ILoadBalancer> Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider)
         {
-            return new RoundRobin(async () => await serviceProvider.Get());
+            return new OkResponse<ILoadBalancer>(new RoundRobin(async () => await serviceProvider.Get()));
         }
 
         public string Type => nameof(RoundRobin);

src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs

@@ -46,7 +46,8 @@ namespace Ocelot.Responder
             }
 
             if (errors.Any(e => e.Code == OcelotErrorCode.UnableToCompleteRequestError
-                || e.Code == OcelotErrorCode.CouldNotFindLoadBalancerCreator))
+                || e.Code == OcelotErrorCode.CouldNotFindLoadBalancerCreator
+                || e.Code == OcelotErrorCode.ErrorInvokingLoadBalancerCreator))
             {
                 return 500;
             }

test/Ocelot.UnitTests/LoadBalancer/CookieStickySessionsCreatorTests.cs

@@ -1,20 +1,21 @@
-using Moq;
+namespace Ocelot.UnitTests.LoadBalancer
+{
+    using Moq;
     using Ocelot.Configuration;
     using Ocelot.Configuration.Builder;
     using Ocelot.LoadBalancer.LoadBalancers;
     using Ocelot.ServiceDiscovery.Providers;
+    using Ocelot.Responses;
     using Shouldly;
     using TestStack.BDDfy;
     using Xunit;
 
-namespace Ocelot.UnitTests.LoadBalancer
-{
     public class CookieStickySessionsCreatorTests
     {
         private readonly CookieStickySessionsCreator _creator;
         private readonly Mock<IServiceDiscoveryProvider> _serviceProvider;
         private DownstreamReRoute _reRoute;
-        private ILoadBalancer _loadBalancer;
+        private Response<ILoadBalancer> _loadBalancer;
         private string _typeName;
 
         public CookieStickySessionsCreatorTests()
@@ -62,7 +63,7 @@ namespace Ocelot.UnitTests.LoadBalancer
         private void ThenTheLoadBalancerIsReturned<T>()
             where T : ILoadBalancer
         {
-            _loadBalancer.ShouldBeOfType<T>();
+            _loadBalancer.Data.ShouldBeOfType<T>();
         }
 
         private void ThenTheLoadBalancerTypeIs(string type)

test/Ocelot.UnitTests/LoadBalancer/DelegateInvokingLoadBalancerCreatorTests.cs

@@ -16,11 +16,11 @@ namespace Ocelot.UnitTests.LoadBalancer
 {
     public class DelegateInvokingLoadBalancerCreatorTests
     {
-        private readonly DelegateInvokingLoadBalancerCreator<FakeLoadBalancer> _creator;
+        private DelegateInvokingLoadBalancerCreator<FakeLoadBalancer> _creator;
-        private readonly Func<DownstreamReRoute, IServiceDiscoveryProvider, ILoadBalancer> _creatorFunc;
+        private Func<DownstreamReRoute, IServiceDiscoveryProvider, ILoadBalancer> _creatorFunc;
         private readonly Mock<IServiceDiscoveryProvider> _serviceProvider;
         private DownstreamReRoute _reRoute;
-        private ILoadBalancer _loadBalancer;
+        private Response<ILoadBalancer> _loadBalancer;
         private string _typeName;
 
         public DelegateInvokingLoadBalancerCreatorTests()
@@ -51,6 +51,31 @@ namespace Ocelot.UnitTests.LoadBalancer
                 .BDDfy();
         }
 
+        [Fact]
+        public void should_return_error()
+        {
+            var reRoute = new DownstreamReRouteBuilder()
+                .Build();
+
+            this.Given(x => x.GivenAReRoute(reRoute))
+                .And(x => x.GivenTheCreatorFuncThrows())
+                .When(x => x.WhenIGetTheLoadBalancer())
+                .Then(x => x.ThenAnErrorIsReturned())
+                .BDDfy();
+        }
+
+        private void GivenTheCreatorFuncThrows()
+        {
+            _creatorFunc = (reRoute, serviceDiscoveryProvider) => throw new Exception();
+
+            _creator = new DelegateInvokingLoadBalancerCreator<FakeLoadBalancer>(_creatorFunc);
+        }
+
+        private void ThenAnErrorIsReturned()
+        {
+            _loadBalancer.IsError.ShouldBeTrue();
+        }
+
         private void GivenAReRoute(DownstreamReRoute reRoute)
         {
             _reRoute = reRoute;
@@ -69,7 +94,7 @@ namespace Ocelot.UnitTests.LoadBalancer
         private void ThenTheLoadBalancerIsReturned<T>()
             where T : ILoadBalancer
         {
-            _loadBalancer.ShouldBeOfType<T>();
+            _loadBalancer.Data.ShouldBeOfType<T>();
         }
 
         private void ThenTheLoadBalancerTypeIs(string type)

test/Ocelot.UnitTests/LoadBalancer/LeastConnectionCreatorTests.cs

@@ -1,20 +1,21 @@
-using Moq;
+namespace Ocelot.UnitTests.LoadBalancer
+{
+    using Moq;
     using Ocelot.Configuration;
     using Ocelot.Configuration.Builder;
     using Ocelot.LoadBalancer.LoadBalancers;
+    using Ocelot.Responses;
     using Ocelot.ServiceDiscovery.Providers;
     using Shouldly;
     using TestStack.BDDfy;
     using Xunit;
 
-namespace Ocelot.UnitTests.LoadBalancer
-{
     public class LeastConnectionCreatorTests
     {
         private readonly LeastConnectionCreator _creator;
         private readonly Mock<IServiceDiscoveryProvider> _serviceProvider;
         private DownstreamReRoute _reRoute;
-        private ILoadBalancer _loadBalancer;
+        private Response<ILoadBalancer> _loadBalancer;
         private string _typeName;
 
         public LeastConnectionCreatorTests()
@@ -62,7 +63,7 @@ namespace Ocelot.UnitTests.LoadBalancer
         private void ThenTheLoadBalancerIsReturned<T>()
             where T : ILoadBalancer
         {
-            _loadBalancer.ShouldBeOfType<T>();
+            _loadBalancer.Data.ShouldBeOfType<T>();
         }
 
         private void ThenTheLoadBalancerTypeIs(string type)

test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs

@@ -16,6 +16,8 @@ using Xunit;
 
 namespace Ocelot.UnitTests.LoadBalancer
 {
+    using System;
+
     public class LoadBalancerFactoryTests
     {
         private DownstreamReRoute _reRoute;
@@ -35,6 +37,7 @@ namespace Ocelot.UnitTests.LoadBalancer
                 new FakeLoadBalancerCreator<FakeLoadBalancerOne>(),
                 new FakeLoadBalancerCreator<FakeLoadBalancerTwo>(),
                 new FakeLoadBalancerCreator<FakeNoLoadBalancer>(nameof(NoLoadBalancer)),
+                new BrokenLoadBalancerCreator<BrokenLoadBalancer>(),
             };
             _factory = new LoadBalancerFactory(_serviceProviderFactory.Object, _loadBalancerCreators);
         }
@@ -87,6 +90,22 @@ namespace Ocelot.UnitTests.LoadBalancer
                 .BDDfy();
         }
 
+        [Fact]
+        public void should_return_error_response_if_creator_errors()
+        {
+            var reRoute = new DownstreamReRouteBuilder()
+                .WithLoadBalancerOptions(new LoadBalancerOptions("BrokenLoadBalancer", "", 0))
+                .WithUpstreamHttpMethod(new List<string> { "Get" })
+                .Build();
+
+            this.Given(x => x.GivenAReRoute(reRoute))
+                .And(x => GivenAServiceProviderConfig(new ServiceProviderConfigurationBuilder().Build()))
+                .And(x => x.GivenTheServiceProviderFactoryReturns())
+                .When(x => x.WhenIGetTheLoadBalancer())
+                .Then(x => x.ThenAnErrorResponseIsReturned())
+                .BDDfy();
+        }
+
         [Fact]
         public void should_call_service_provider()
         {
@@ -183,9 +202,25 @@ namespace Ocelot.UnitTests.LoadBalancer
                 Type = type;
             }
 
-            public ILoadBalancer Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider)
+            public Response<ILoadBalancer> Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider)
             {
-                return new T();
+                return new OkResponse<ILoadBalancer>(new T());
+            }
+            
+            public string Type { get; }
+        }
+
+        private class BrokenLoadBalancerCreator<T> : ILoadBalancerCreator
+            where T : ILoadBalancer, new()
+        {
+            public BrokenLoadBalancerCreator()
+            {
+                Type = typeof(T).Name;
+            }
+
+            public Response<ILoadBalancer> Create(DownstreamReRoute reRoute, IServiceDiscoveryProvider serviceProvider)
+            {
+                return new ErrorResponse<ILoadBalancer>(new ErrorInvokingLoadBalancerCreator(new Exception()));
             }
 
             public string Type { get; }
@@ -229,5 +264,19 @@ namespace Ocelot.UnitTests.LoadBalancer
                 throw new System.NotImplementedException();
             }
         }
+
+        private class BrokenLoadBalancer : ILoadBalancer
+        {
+            public Task<Response<ServiceHostAndPort>> Lease(DownstreamContext context)
+            {
+                throw new System.NotImplementedException();
+            }
+
+            public void Release(ServiceHostAndPort hostAndPort)
+            {
+                throw new System.NotImplementedException();
+            }
+        }
+
     }
 }

test/Ocelot.UnitTests/LoadBalancer/NoLoadBalancerCreatorTests.cs

@@ -1,20 +1,21 @@
-using Moq;
+namespace Ocelot.UnitTests.LoadBalancer
+{
+    using Moq;
     using Ocelot.Configuration;
     using Ocelot.Configuration.Builder;
     using Ocelot.LoadBalancer.LoadBalancers;
+    using Ocelot.Responses;
     using Ocelot.ServiceDiscovery.Providers;
     using Shouldly;
     using TestStack.BDDfy;
     using Xunit;
 
-namespace Ocelot.UnitTests.LoadBalancer
-{
     public class NoLoadBalancerCreatorTests
     {
         private readonly NoLoadBalancerCreator _creator;
         private readonly Mock<IServiceDiscoveryProvider> _serviceProvider;
         private DownstreamReRoute _reRoute;
-        private ILoadBalancer _loadBalancer;
+        private Response<ILoadBalancer> _loadBalancer;
         private string _typeName;
 
         public NoLoadBalancerCreatorTests()
@@ -61,7 +62,7 @@ namespace Ocelot.UnitTests.LoadBalancer
         private void ThenTheLoadBalancerIsReturned<T>()
             where T : ILoadBalancer
         {
-            _loadBalancer.ShouldBeOfType<T>();
+            _loadBalancer.Data.ShouldBeOfType<T>();
         }
 
         private void ThenTheLoadBalancerTypeIs(string type)

test/Ocelot.UnitTests/LoadBalancer/RoundRobinCreatorTests.cs

@@ -1,20 +1,21 @@
-using Moq;
+namespace Ocelot.UnitTests.LoadBalancer
+{
+    using Moq;
     using Ocelot.Configuration;
     using Ocelot.Configuration.Builder;
     using Ocelot.LoadBalancer.LoadBalancers;
+    using Ocelot.Responses;
     using Ocelot.ServiceDiscovery.Providers;
     using Shouldly;
     using TestStack.BDDfy;
     using Xunit;
 
-namespace Ocelot.UnitTests.LoadBalancer
-{
     public class RoundRobinCreatorTests
     {
         private readonly RoundRobinCreator _creator;
         private readonly Mock<IServiceDiscoveryProvider> _serviceProvider;
         private DownstreamReRoute _reRoute;
-        private ILoadBalancer _loadBalancer;
+        private Response<ILoadBalancer> _loadBalancer;
         private string _typeName;
 
         public RoundRobinCreatorTests()
@@ -61,7 +62,7 @@ namespace Ocelot.UnitTests.LoadBalancer
         private void ThenTheLoadBalancerIsReturned<T>()
             where T : ILoadBalancer
         {
-            _loadBalancer.ShouldBeOfType<T>();
+            _loadBalancer.Data.ShouldBeOfType<T>();
         }
 
         private void ThenTheLoadBalancerTypeIs(string type)

test/Ocelot.UnitTests/Responder/ErrorsToHttpStatusCodeMapperTests.cs

@@ -48,6 +48,7 @@ namespace Ocelot.UnitTests.Responder
         [Theory]
         [InlineData(OcelotErrorCode.UnableToCompleteRequestError)]
         [InlineData(OcelotErrorCode.CouldNotFindLoadBalancerCreator)]
+        [InlineData(OcelotErrorCode.ErrorInvokingLoadBalancerCreator)]
         public void should_return_internal_server_error(OcelotErrorCode errorCode)
         {
             ShouldMapErrorToStatusCode(errorCode, HttpStatusCode.InternalServerError);
@@ -133,7 +134,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(40, "Looks like the number of error codes has changed. Do you need to modify ErrorsToHttpStatusCodeMapper?");
+            Enum.GetNames(typeof(OcelotErrorCode)).Length.ShouldBe(41, "Looks like the number of error codes has changed. Do you need to modify ErrorsToHttpStatusCodeMapper?");
         }
 
         private void ShouldMapErrorToStatusCode(OcelotErrorCode errorCode, HttpStatusCode expectedHttpStatusCode)