From: David Schmidt <mail@davidschmidt.at>
Date: Tue, 30 Dec 2014 19:20:32 +0000 (+0100)
Subject: backcompat http-exception handling by middleware
X-Git-Tag: 45.90078~4^2^2
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Runtime.git;a=commitdiff_plain;h=5998e9ca8c56a9af840743af927acb56b7a39801

backcompat http-exception handling by middleware
---

diff --git a/lib/Catalyst.pm b/lib/Catalyst.pm
index e0b1117..32aea57 100644
--- a/lib/Catalyst.pm
+++ b/lib/Catalyst.pm
@@ -1776,6 +1776,7 @@ sub execute {
     if ( my $error = $@ ) {
         #rethow if this can be handled by middleware
         if(
+          !$c->config->{always_catch_http_exceptions} &&
           blessed $error && (
             $error->can('as_psgi') ||
             (
@@ -1965,6 +1966,7 @@ sub finalize_error {
     } else {
         my ($error) = @{$c->error};
         if(
+          !$c->config->{always_catch_http_exceptions} &&
           blessed $error &&
           ($error->can('as_psgi') || $error->can('code'))
         ) {
@@ -2110,6 +2112,7 @@ sub handle_request {
     } catch {
         #rethow if this can be handled by middleware
         if(
+          !$class->config->{always_catch_http_exceptions} &&
           blessed($_) && (
             $_->can('as_psgi') ||
             (
@@ -3506,6 +3509,13 @@ There are a number of 'base' config variables which can be set:
 
 =item *
 
+C<always_catch_http_exceptions> - As of version 5.90060 Catalyst
+rethrows errors conforming to the interface described by
+L<Plack::Middleware::HTTPExceptions> and lets the middleware deal with it.
+Set true to get the deprecated behaviour and have Catakyst catch HTTP exceptions.
+
+=item *
+
 C<default_model> - The default model picked if you say C<< $c->model >>. See L<< /$c->model($name) >>.
 
 =item *
diff --git a/t/http_exceptions_backcompat.t b/t/http_exceptions_backcompat.t
new file mode 100644
index 0000000..7a1bd86
--- /dev/null
+++ b/t/http_exceptions_backcompat.t
@@ -0,0 +1,140 @@
+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 HTTP style exceptions do NOT bubble up to the middleware
+# if the backcompat setting 'always_catch_http_exceptions' is enabled.
+
+{
+  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::AnotherException;
+
+  sub new { bless +{}, shift }
+
+  sub code { 400 }
+
+  sub as_string { 'bad stringy bad' }
+
+  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 from_code_type :Local {
+    my $e = MyApp::AnotherException->new;
+    die $e;
+  }
+
+  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";
+  }
+
+  sub end : ActionClass('RenderView') {}
+
+  package MyApp;
+  use Catalyst;
+
+  MyApp->config(
+      abort_chain_on_error_fix=>1,
+      always_catch_http_exceptions=>1,
+  );
+
+  sub debug { 1 }
+
+  MyApp->setup_log('fatal');
+}
+
+$INC{'MyApp/Controller/Root.pm'} = __FILE__; # 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, 500;
+    like $res->content, qr/MyApp::Exception=HASH/;
+};
+
+test_psgi $psgi, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/root/from_catalyst");
+    is $res->code, 500;
+    like $res->content, qr/MyApp::Exception=HASH/;
+};
+
+test_psgi $psgi, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/root/from_code_type");
+    is $res->code, 500;
+    like $res->content, qr/MyApp::AnotherException=HASH/;
+};
+
+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(12);