From: John Napiorkowski <jjnapiork@cpan.org>
Date: Thu, 26 Dec 2013 23:40:19 +0000 (-0600)
Subject: if an exception is http, let middleware handle it
X-Git-Tag: 5.90060~35
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Runtime.git;a=commitdiff_plain;h=660f9bb0ce5e5ccb7d126608cf8b908194167b59

if an exception is http, let middleware handle it
---

diff --git a/lib/Catalyst.pm b/lib/Catalyst.pm
index d84ddbf..b9dc3f1 100644
--- a/lib/Catalyst.pm
+++ b/lib/Catalyst.pm
@@ -43,6 +43,7 @@ use Plack::Middleware::IIS7KeepAliveFix;
 use Plack::Middleware::LighttpdScriptNameFix;
 use Plack::Middleware::ContentLength;
 use Plack::Middleware::Head;
+use Plack::Middleware::HTTPExceptions;
 use Plack::Util;
 use Class::Load 'load_class';
 
@@ -1896,7 +1897,25 @@ Finalizes error.
 
 =cut
 
-sub finalize_error { my $c = shift; $c->engine->finalize_error( $c, @_ ) }
+sub finalize_error {
+    my $c = shift;
+    if($#{$c->error} > 0) {
+        $c->engine->finalize_error( $c, @_ );
+    } else {
+        my ($error) = @{$c->error};
+        if(
+          blessed $error &&
+          ($error->can('as_psgi') || $error->can('code'))
+        ) {
+            # In the case where the error 'knows what it wants', becauses its PSGI
+            # aware, just rethow and let middleware catch it
+            $error->can('rethrow') ? $error->rethrow : croak $error;
+            croak $error;
+        } else {
+            $c->engine->finalize_error( $c, @_ )
+        }
+    }
+}
 
 =head2 $c->finalize_headers
 
@@ -2013,10 +2032,13 @@ sub handle_request {
         my $c = $class->prepare(@arguments);
         $c->dispatch;
         $status = $c->finalize;
-    }
-    catch {
+    } catch {
         chomp(my $error = $_);
         $class->log->error(qq/Caught exception in engine "$error"/);
+        #rethow if this can be handled by middleware
+        if(blessed $error && ($error->can('as_psgi') || $error->can('code'))) {
+            $error->can('rethrow') ? $error->rethrow : croak $error;
+        }
     };
 
     $COUNT++;
@@ -3105,6 +3127,7 @@ sub registered_middlewares {
     my $class = shift;
     if(my $middleware = $class->_psgi_middleware) {
         return (
+          Plack::Middleware::HTTPExceptions->new,
           Plack::Middleware::ContentLength->new,
           Plack::Middleware::Head->new,
           @$middleware);
diff --git a/t/http_exceptions.t b/t/http_exceptions.t
new file mode 100644
index 0000000..c8fae7c
--- /dev/null
+++ b/t/http_exceptions.t
@@ -0,0 +1,117 @@
+use warnings;
+use strict;
+use Test::More;
+use HTTP::Request::Common;
+use HTTP::Message::PSGI;
+use Plack::Util;
+use Plack::Test;
+
+# Test to make sure we let HTTP style exceptions bubble up to the middleware
+# rather than catching them outselves.
+
+{
+  package MyApp::Exception;
+
+  sub new {
+    my ($class, $code, $headers, $body) = @_;
+    return bless +{res => [$code, $headers, $body]}, $class;
+  }
+
+  sub throw { die shift->new(@_) }
+
+  sub as_psgi {
+    my ($self, $env) = @_;
+    my ($code, $headers, $body) = @{$self->{res}};
+
+    return [$code, $headers, $body]; # for now
+
+    return sub {
+      my $responder = shift;
+      $responder->([$code, $headers, $body]);
+    };
+  }
+  
+  package MyApp::Controller::Root;
+
+  use base 'Catalyst::Controller';
+
+  my $psgi_app = sub {
+    my $env = shift;
+    die MyApp::Exception->new(
+      404, ['content-type'=>'text/plain'], ['Not Found']);
+  };
+
+  sub from_psgi_app :Local {
+    my ($self, $c) = @_;
+    $c->res->from_psgi_response(
+      $psgi_app->(
+        $c->req->env));
+  }
+
+  sub from_catalyst :Local {
+    my ($self, $c) = @_;
+    MyApp::Exception->throw(
+      403, ['content-type'=>'text/plain'], ['Forbidden']);
+  }
+
+  sub classic_error :Local {
+    my ($self, $c) = @_;
+    Catalyst::Exception->throw("Ex Parrot");
+  }
+
+  sub just_die :Local {
+    my ($self, $c) = @_;
+    die "I'm not dead yet";
+  }
+
+  package MyApp;
+  use Catalyst;
+
+  sub debug { 1 }
+
+  MyApp->setup_log('fatal');
+}
+
+$INC{'MyApp/Controller/Root.pm'} = '1'; # sorry...
+MyApp->setup_log('error');
+
+Test::More::ok(MyApp->setup);
+
+ok my $psgi = MyApp->psgi_app;
+
+test_psgi $psgi, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/root/from_psgi_app");
+    is $res->code, 404;
+    is $res->content, 'Not Found', 'NOT FOUND';
+};
+
+test_psgi $psgi, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/root/from_catalyst");
+    is $res->code, 403;
+    is $res->content, 'Forbidden', 'Forbidden';
+};
+
+test_psgi $psgi, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/root/classic_error");
+    is $res->code, 500;
+    like $res->content, qr'Ex Parrot', 'Ex Parrot';
+};
+
+test_psgi $psgi, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/root/just_die");
+    is $res->code, 500;
+    like $res->content, qr'not dead yet', 'not dead yet';
+};
+
+
+
+# We need to specify the number of expected tests because tests that live
+# in the callbacks might never get run (thus all ran tests pass but not all
+# required tests run).
+
+done_testing(10);
+