From 68c266124de8e212143d4b94c55982f986f6945c Mon Sep 17 00:00:00 2001 From: Andrew Rodland Date: Thu, 3 Dec 2009 20:00:37 -0600 Subject: [PATCH 1/5] Change Serialize::execute to expect serializers to throw an exception --- lib/Catalyst/Action/Serialize.pm | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/Catalyst/Action/Serialize.pm b/lib/Catalyst/Action/Serialize.pm index d47f97c..2c23151 100644 --- a/lib/Catalyst/Action/Serialize.pm +++ b/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; From 71cbfa6b6eb4670d838af0f640c1c2402637f2c4 Mon Sep 17 00:00:00 2001 From: Andrew Rodland Date: Thu, 3 Dec 2009 20:04:59 -0600 Subject: [PATCH 2/5] Port Serialize classes except for View. --- lib/Catalyst/Action/Serialize/Data/Serializer.pm | 10 ++-------- lib/Catalyst/Action/Serialize/JSON.pm | 8 +------- lib/Catalyst/Action/Serialize/XML/Simple.pm | 10 ++-------- lib/Catalyst/Action/Serialize/YAML.pm | 8 +------- 4 files changed, 6 insertions(+), 30 deletions(-) diff --git a/lib/Catalyst/Action/Serialize/Data/Serializer.pm b/lib/Catalyst/Action/Serialize/Data/Serializer.pm index ca42ce7..c58d192 100644 --- a/lib/Catalyst/Action/Serialize/Data/Serializer.pm +++ b/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; } diff --git a/lib/Catalyst/Action/Serialize/JSON.pm b/lib/Catalyst/Action/Serialize/JSON.pm index d4b12df..c462e58 100644 --- a/lib/Catalyst/Action/Serialize/JSON.pm +++ b/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; } diff --git a/lib/Catalyst/Action/Serialize/XML/Simple.pm b/lib/Catalyst/Action/Serialize/XML/Simple.pm index a562078..a367596 100644 --- a/lib/Catalyst/Action/Serialize/XML/Simple.pm +++ b/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; } diff --git a/lib/Catalyst/Action/Serialize/YAML.pm b/lib/Catalyst/Action/Serialize/YAML.pm index 911191b..c3f9003 100644 --- a/lib/Catalyst/Action/Serialize/YAML.pm +++ b/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; } From efab952125dde293960623eae501b1ad57ec9f6f Mon Sep 17 00:00:00 2001 From: Andrew Rodland Date: Thu, 3 Dec 2009 20:09:38 -0600 Subject: [PATCH 3/5] View is halfway there. Do we want to poke in $c->error? Maybe not. --- lib/Catalyst/Action/Serialize/View.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Catalyst/Action/Serialize/View.pm b/lib/Catalyst/Action/Serialize/View.pm index 7aa66df..baaec85 100644 --- a/lib/Catalyst/Action/Serialize/View.pm +++ b/lib/Catalyst/Action/Serialize/View.pm @@ -16,7 +16,7 @@ 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); From 067d48ee88e1337e1bfc35b56188fbb438377155 Mon Sep 17 00:00:00 2001 From: Andrew Rodland Date: Thu, 17 Dec 2009 04:55:27 -0600 Subject: [PATCH 4/5] Add some real error-handling for Serialize::View --- lib/Catalyst/Action/Serialize/View.pm | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/Catalyst/Action/Serialize/View.pm b/lib/Catalyst/Action/Serialize/View.pm index baaec85..286fc88 100644 --- a/lib/Catalyst/Action/Serialize/View.pm +++ b/lib/Catalyst/Action/Serialize/View.pm @@ -19,7 +19,15 @@ sub execute { 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; From 2f97104239a5737ffc792d188530258361a57942 Mon Sep 17 00:00:00 2001 From: Andrew Rodland Date: Thu, 17 Dec 2009 04:55:44 -0600 Subject: [PATCH 5/5] Add tests for Serialize::View error behavior --- t/lib/Test/Serialize/Controller/REST.pm | 1 + t/lib/Test/Serialize/View/Awful.pm | 27 +++++++++++++++++++++++++ t/view.t | 8 +++++++- 3 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 t/lib/Test/Serialize/View/Awful.pm diff --git a/t/lib/Test/Serialize/Controller/REST.pm b/t/lib/Test/Serialize/Controller/REST.pm index 74b921c..118e237 100644 --- a/t/lib/Test/Serialize/Controller/REST.pm +++ b/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', }, ); diff --git a/t/lib/Test/Serialize/View/Awful.pm b/t/lib/Test/Serialize/View/Awful.pm new file mode 100644 index 0000000..9c9abf9 --- /dev/null +++ b/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; diff --git a/t/view.t b/t/view.t index a5a379f..07b3974 100644 --- a/t/view.t +++ b/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;