added try catch calls to statuscode handlers (#2457)

NancyFx · May 26, 2016 · 9005a97 · 9005a97
1 parent 79ff386
commit 9005a97
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 1 deletion.
diff --git a/src/Nancy/NancyEngine.cs b/src/Nancy/NancyEngine.cs
@@ -2,6 +2,7 @@
 {
     using System;
     using System.Collections.Generic;
+    using System.IO;
     using System.Linq;
     using System.Threading;
     using System.Threading.Tasks;
@@ -252,7 +253,19 @@ private void CheckStatusCodeHandler(NancyContext context)
                 return;
             }
 
-            handler.Handle(context.Response.StatusCode, context);
+            try
+            {
+                handler.Handle(context.Response.StatusCode, context);
+            }
+            catch (Exception ex)
+            {
+                if (defaultHandler == null)
+                {
+                    throw;
+                }
+
+                defaultHandler.Handle(context.Response.StatusCode, context);
+            }
         }
 
         private async Task<NancyContext> InvokeRequestLifeCycle(NancyContext context, CancellationToken cancellationToken, IPipelines pipelines)

diff --git a/test/Nancy.Tests/Unit/NancyEngineFixture.cs b/test/Nancy.Tests/Unit/NancyEngineFixture.cs
@@ -339,6 +339,53 @@ public async Task Should_invoke_status_handler_if_supported_status_code()
             A.CallTo(() => this.statusCodeHandler.Handle(A<HttpStatusCode>.Ignored, A<NancyContext>.Ignored)).MustHaveHappened(Repeated.Exactly.Once);
         }
 
+        [Fact]
+        public async Task Should_catch_exception_inside_status_code_handler_and_return_default_error_response()
+        {
+            // Given
+            A.CallTo(() => this.requestDispatcher.Dispatch(A<NancyContext>.Ignored, A<CancellationToken>.Ignored))
+                .Returns(new Response() { StatusCode = HttpStatusCode.InternalServerError });
+
+            var statusCodeHandlers = new[]
+            {
+                this.statusCodeHandler,
+                new DefaultStatusCodeHandler(this.negotiator, this.environment),
+            };
+
+            var engine =
+                new NancyEngine(this.requestDispatcher, this.contextFactory, statusCodeHandlers,
+                    A.Fake<IRequestTracing>(),
+                    new DisabledStaticContentProvider(), this.negotiator, this.environment)
+                {
+                    RequestPipelinesFactory = ctx => new Pipelines()
+                };
+
+            var request = new Request("GET", "/", "http");
+
+            A.CallTo(() => this.statusCodeHandler.HandlesStatusCode(A<HttpStatusCode>.Ignored, A<NancyContext>.Ignored)).Returns(true);
+            A.CallTo(() => this.statusCodeHandler.Handle(A<HttpStatusCode>.Ignored, A<NancyContext>.Ignored))
+                .Throws<Exception>();
+
+            // When
+            await engine.HandleRequest(request);
+
+            // Then
+            this.context.Response.StatusCode.ShouldEqual(HttpStatusCode.InternalServerError);
+        }
+
+        [Fact]
+        public async Task Should_throw_exception_if_no_default_status_code_handler_present_when_custom_status_code_handler_throws()
+        {
+            // Given
+            var request = new Request("GET", "/", "http");
+            A.CallTo(() => this.statusCodeHandler.HandlesStatusCode(A<HttpStatusCode>.Ignored, A<NancyContext>.Ignored)).Returns(true);
+            A.CallTo(() => this.statusCodeHandler.Handle(A<HttpStatusCode>.Ignored, A<NancyContext>.Ignored))
+                .Throws<Exception>();
+
+            // When,Then
+            await AssertAsync.Throws<Exception>(async () => await this.engine.HandleRequest(request));
+        }
+
         [Fact]
         public async Task Should_set_status_code_to_500_if_route_throws()
         {