Merge remote branch 'hobbs/rework-serializer-return-2'

* hobbs/rework-serializer-return-2:
  Add tests for Serialize::View error behavior
  Add some real error-handling for Serialize::View
  View is halfway there. Do we want to poke in $c->error? Maybe not.
  Port Serialize classes except for View.
  Change Serialize::execute to expect serializers to throw an exception

lib/Catalyst/Action/Serialize.pm

@@ -34,15 +34,17 @@ sub execute {
         "Serializing with $sclass" . ( $sarg ? " [$sarg]" : '' ) ) if $c->debug;
 
     my $rc;
-    if ( defined($sarg) ) {
-        $rc = $sclass->execute( $controller, $c, $sarg );
-    } else {
-        $rc = $sclass->execute( $controller, $c );
-    }
-    if ( $rc eq 0 ) {
+    eval {
+        if ( defined($sarg) ) {
+            $rc = $sclass->execute( $controller, $c, $sarg );
+        } else {
+            $rc = $sclass->execute( $controller, $c );
+        }
+    };
+    if ($@) {
+        return $self->_serialize_bad_request( $c, $content_type, $@ );
+    } elsif (!$rc) {
         return $self->_unsupported_media_type( $c, $content_type );
-    } elsif ( $rc ne 1 ) {
-        return $self->_serialize_bad_request( $c, $content_type, $rc );
     }
 
     return 1;

lib/Catalyst/Action/Serialize/Data/Serializer.pm

@@ -23,16 +23,10 @@ sub execute {
     };
     if ($@) {
         $c->log->info("Could not load $serializer, refusing to serialize: $@");
-        return 0;
+        return;
     }
     my $dso = Data::Serializer->new( serializer => $serializer );
-    my $data;
-    eval {
-       $data = $dso->raw_serialize($c->stash->{$stash_key});
-    };
-    if ($@) {
-        return $@;
-    } 
+    my $data = $dso->raw_serialize($c->stash->{$stash_key});
     $c->response->output( $data );
     return 1;
 }

lib/Catalyst/Action/Serialize/JSON.pm

@@ -15,13 +15,7 @@ sub execute {
                 $controller->{'serialize'}->{'stash_key'} :
                 $controller->{'stash_key'} 
         ) || 'rest';
-    my $output;
-    eval {
-        $output = $self->serialize( $c->stash->{$stash_key} );
-    };
-    if ($@) {
-        return $@;
-    }
+    my $output = $self->serialize( $c->stash->{$stash_key} );
     $c->response->output( $output );
     return 1;
 }

lib/Catalyst/Action/Serialize/View.pm

@@ -16,10 +16,18 @@ sub execute {
 
     if ( !$c->view($view) ) {
         $c->log->error("Could not load $view, refusing to serialize");
-        return 0;
+        return;
     }
 
-    return $c->view($view)->process($c, $stash_key);
+    if ($c->view($view)->process($c, $stash_key)) {
+      return 1;
+    } else {
+      # This is stupid. Please improve it.
+      my $error = join("\n", @{ $c->error }) || "Error in $view";
+      $error .= "\n";
+      $c->clear_errors;
+      die $error;
+    }
 }
 
 1;

lib/Catalyst/Action/Serialize/XML/Simple.pm

@@ -15,7 +15,7 @@ sub execute {
     if ($@) {
         $c->log->debug("Could not load XML::Serializer, refusing to serialize: $@")
             if $c->debug;
-        return 0;
+        return;
     }
     my $xs = XML::Simple->new(ForceArray => 0,);
 
@@ -24,13 +24,7 @@ sub execute {
                 $controller->{'serialize'}->{'stash_key'} :
                 $controller->{'stash_key'} 
         ) || 'rest';
-    my $output;
-    eval {
-        $output = $xs->XMLout({ data => $c->stash->{$stash_key} });
-    };
-    if ($@) {
-        return $@;
-    }
+    my $output = $xs->XMLout({ data => $c->stash->{$stash_key} });
     $c->response->output( $output );
     return 1;
 }

lib/Catalyst/Action/Serialize/YAML.pm

@@ -15,13 +15,7 @@ sub execute {
                 $controller->{'serialize'}->{'stash_key'} :
                 $controller->{'stash_key'} 
         ) || 'rest';
-    my $output;
-    eval {
-        $output = $self->serialize($c->stash->{$stash_key});
-    };
-    if ($@) {
-        return $@;
-    }
+    my $output = $self->serialize($c->stash->{$stash_key});
     $c->response->output( $output );
     return 1;
 }

t/lib/Test/Serialize/Controller/REST.pm

@@ -25,6 +25,7 @@ __PACKAGE__->config(
         'text/x-php-serialization' =>
              [ 'Data::Serializer', 'PHP::Serialization' ],
         'text/view'   => [ 'View', 'Simple' ],
+        'text/explodingview' => [ 'View', 'Awful' ],
         'text/broken' => 'Broken',
     },
 );

t/lib/Test/Serialize/View/Awful.pm

@@ -0,0 +1,27 @@
+package Test::Serialize::View::Awful;
+
+use base Catalyst::View;
+
+sub render {
+  my ($self, $c, $template) = @_;
+  die "I don't know how to do that!";
+}
+
+sub process {
+  my ($self, $c) = @_;
+
+  my $output = eval {
+    $self->render($c, "blah");
+  };
+
+  if ($@) {
+    my $error = qq/Couldn't render template. Error: "$@"/;
+    $c->error($error);
+    return 0;
+  }
+
+  $c->res->body($output);
+  return 1;
+}
+
+1;

t/view.t

@@ -1,6 +1,6 @@
 use strict;
 use warnings;
-use Test::More tests => 4;
+use Test::More tests => 6;
 use FindBin;
 
 use lib ( "$FindBin::Bin/lib", "$FindBin::Bin/../lib" );
@@ -18,4 +18,10 @@ is( $mres->content, $monkey_template, "GET returned the right data" );
 my $mres_post = request( $t->post( url => '/monkey_put', data => 1 ) );
 ok( $mres_post->is_success, "POST to the monkey passed." );
 
+my $t2 = Test::Rest->new( 'content_type' => 'text/explodingview' );
+my $res = request( $t2->get( url => '/monkey_get' ) );
+
+ok (! $res->is_success, 'View errors result in failure');
+like( $res->content, qr/don't know how/, 'The error looks okay');
+
 1;