merged Mojolicious exception handling into the around_dispatch hook, …

…this will allow a whole new category of exception handling plugins
mojolicious · Mar 13, 2012 · 76e71e2 · 76e71e2
1 parent d506c1a
commit 76e71e2
Show file tree

Hide file tree

Showing 7 changed files with 82 additions and 121 deletions.
diff --git a/Changes b/Changes
@@ -1,6 +1,9 @@
 This file documents the revision history for Perl extension Mojolicious.
 
 2.61    2012-03-14 00:00:00
+        - Merged Mojolicious exception handling into the around_dispatch
+          hook, this will allow a whole new category of exception handling
+          plugins.
         - Improved documentation.
 
 2.60    2012-03-13 00:00:00

diff --git a/lib/Mojolicious.pm b/lib/Mojolicious.pm
@@ -2,6 +2,7 @@ package Mojolicious;
 use Mojo::Base 'Mojo';
 
 use Carp 'croak';
+use Mojo::Exception;
 use Mojolicious::Commands;
 use Mojolicious::Controller;
 use Mojolicious::Plugins;
@@ -97,6 +98,16 @@ sub new {
   $mode = $mode . '_mode';
   $self->$mode(@_) if $self->can($mode);
 
+  # Exception handling
+  $self->hook(
+    around_dispatch => sub {
+      my ($next, $c) = @_;
+      local $SIG{__DIE__} =
+        sub { ref $_[0] ? CORE::die($_[0]) : Mojo::Exception->throw(@_) };
+      $c->render_exception($@) unless eval { $next->(); 1 };
+    }
+  );
+
   # Startup
   $self->startup(@_);
 

diff --git a/lib/Mojolicious/Plugin/EPLRenderer.pm b/lib/Mojolicious/Plugin/EPLRenderer.pm
@@ -70,15 +70,8 @@ sub register {
         $cache->set($key => $mt);
       }
 
-      # Exception
-      if (ref $$output) {
-        my $e = $$output;
-        $$output = '';
-        $c->render_exception($e);
-      }
-
-      # Success or exception
-      return ref $$output ? 0 : 1;
+      # Exception or success
+      return ref $$output ? die($$output) : 1;
     }
   );
 }

diff --git a/lib/Mojolicious/Routes.pm b/lib/Mojolicious/Routes.pm
@@ -3,7 +3,6 @@ use Mojo::Base -base;
 
 use Carp 'croak';
 use Mojo::Cache;
-use Mojo::Exception;
 use Mojo::Loader;
 use Mojo::Util 'camelize';
 use Mojolicious::Routes::Match;
@@ -63,15 +62,10 @@ sub any { shift->_generate_route(ref $_[0] eq 'ARRAY' ? shift : [], @_) }
 # "Hey. What kind of party is this? There's no booze and only one hooker."
 sub auto_render {
   my ($self, $c) = @_;
-
-  # Try rendering template or not_found if the route never reached an action
-  eval {
-    my $stash = $c->stash;
-    $c->render
-      or ($stash->{'mojo.routed'} or $c->render_not_found)
-      unless $stash->{'mojo.rendered'} || $c->tx->is_websocket;
-    1;
-  } or $c->render_exception($@);
+  my $stash = $c->stash;
+  $c->render
+    or ($stash->{'mojo.routed'} or $c->render_not_found)
+    unless $stash->{'mojo.rendered'} || $c->tx->is_websocket;
 }
 
 sub bridge { shift->route(@_)->inline(1) }
@@ -123,7 +117,7 @@ sub dispatch {
   # No match
   return unless $m && @{$m->stack};
 
-  # Walk the stack
+  # Dispatch
   return if $self->_walk_stack($c);
   $self->auto_render($c);
   return 1;
@@ -316,17 +310,10 @@ sub websocket {
 
 sub _dispatch_callback {
   my ($self, $c, $field, $staging) = @_;
-
-  # Routed
   $c->stash->{'mojo.routed'}++;
   $c->app->log->debug(qq/Dispatching callback./);
-
-  # Dispatch
-  my $continue;
-  return Mojo::Exception->new($@)
-    unless eval { $continue = $field->{cb}->($c); 1 };
-  return 1 if !$staging || $continue;
-  return;
+  my $continue = $field->{cb}->($c);
+  return !$staging || $continue ? 1 : undef;
 }
 
 sub _dispatch_controller {
@@ -340,72 +327,38 @@ sub _dispatch_controller {
   $target .= "->$method" if $method;
   $c->app->log->debug(qq/Dispatching "$target"./);
 
-  # Load class
-  if (!ref $app && !$self->{loaded}->{$app}) {
-    if (my $e = Mojo::Loader->load($app)) {
-
-      # Doesn't exist
-      $c->app->log->debug("$app does not exist, maybe a typo?") and return
-        unless ref $e;
-
-      # Error
-      return $e;
-    }
-
-    # Check for controller and application
-    return
-      unless $app->isa($self->controller_base_class) || $app->isa('Mojo');
-    $self->{loaded}->{$app}++;
-  }
+  # Controller or application
+  return unless $self->_load_class($c, $app);
+  $app = $app->new($c) unless ref $app;
 
-  # Dispatch
+  # Action
   my $continue;
-  my $success = eval {
-    $app = $app->new($c) unless ref $app;
-
-    # Action
-    if ($method) {
-
-      # Call action
-      my $stash = $c->stash;
-      if ($app->can($method)) {
-        $stash->{'mojo.routed'}++ unless $staging;
-        $continue = $app->$method;
-      }
-
-      # Render
-      else {
-        $c->app->log->debug(
-          qq/Action "$target" not found, assuming template without action./);
-        $self->auto_render($app) unless $staging;
-      }
-
-      # Merge stash
-      my $new = $app->stash;
-      @{$stash}{keys %$new} = values %$new;
+  if ($method) {
+
+    # Call action
+    my $stash = $c->stash;
+    if (my $code = $app->can($method)) {
+      $stash->{'mojo.routed'}++ unless $staging;
+      $continue = $app->$code;
     }
 
-    # Application
+    # Render
     else {
-      if ($app->can('routes')) {
-        my $r = $app->routes;
-        weaken $r->parent($c->match->endpoint)->{parent} unless $r->parent;
-      }
-      $app->handler($c);
+      $c->app->log->debug(
+        qq/Action "$target" not found, assuming template without action./);
     }
+  }
 
-    1;
-  };
-
-  # Controller error
-  unless ($success) {
-    my $e = Mojo::Exception->new($@);
-    $app->render_exception($e) if $app->can('render_exception');
-    return $e;
+  # Application
+  else {
+    if (my $code = $app->can('routes')) {
+      my $r = $app->$code;
+      weaken $r->parent($c->match->endpoint)->{parent} unless $r->parent;
+    }
+    $app->handler($c);
   }
 
-  return 1 if !$staging || $continue;
-  return;
+  return !$staging || $continue ? 1 : undef;
 }
 
 sub _generate_class {
@@ -493,13 +446,29 @@ sub _generate_route {
     ->via($methods)->to($defaults)->name($name);
 }
 
+sub _load_class {
+  my ($self, $c, $app) = @_;
+
+  # Load unless already loaded or application
+  return 1 if $self->{loaded}->{$app} || ref $app;
+  if (my $e = Mojo::Loader->load($app)) {
+
+    # Doesn't exist
+    $c->app->log->debug("$app does not exist, maybe a typo?") and return
+      unless ref $e;
+
+    # Error
+    die $e;
+  }
+
+  # Check for controller and application
+  return unless $app->isa($self->controller_base_class) || $app->isa('Mojo');
+  return ++$self->{loaded}->{$app};
+}
+
 sub _walk_stack {
   my ($self, $c) = @_;
 
-  # Stacktrace
-  local $SIG{__DIE__} =
-    sub { ref $_[0] ? CORE::die($_[0]) : Mojo::Exception->throw(@_) };
-
   # Walk the stack
   my $stack   = $c->match->stack;
   my $stash   = $c->stash;
@@ -513,19 +482,13 @@ sub _walk_stack {
     @{$stash}{@keys} = @{$stash->{'mojo.captures'}}{@keys} = values %$field;
 
     # Dispatch
-    my $e =
+    my $continue =
         $field->{cb}
       ? $self->_dispatch_callback($c, $field, $staging)
       : $self->_dispatch_controller($c, $field, $staging);
 
-    # Exception
-    if (ref $e) {
-      $c->render_exception($e);
-      return 1;
-    }
-
     # Break the chain
-    return 1 if $staging && !$e;
+    return 1 if $staging && !$continue;
   }
 
   return;

diff --git a/t/mojolicious/app.t b/t/mojolicious/app.t
@@ -74,32 +74,30 @@ $t->get_ok('/foo/syntaxerror')->status_is(500)
 # Exceptional::this_one_dies (action dies)
 $t->get_ok('/exceptional/this_one_dies')->status_is(500)
   ->header_is(Server         => 'Mojolicious (Perl)')
-  ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
-  ->content_is("Action died: doh!\n");
+  ->header_is('X-Powered-By' => 'Mojolicious (Perl)')->content_is("doh!\n\n");
 
 # Exceptional::this_one_might_die (bridge dies)
 $t->get_ok('/exceptional_too/this_one_dies')->status_is(500)
   ->header_is(Server         => 'Mojolicious (Perl)')
   ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
-  ->content_is("Action died: double doh!\n");
+  ->content_is("double doh!\n\n");
 
 # Exceptional::this_one_dies (action behind bridge dies)
 $t->get_ok('/exceptional_too/this_one_dies', {'X-DoNotDie' => 1})
   ->status_is(500)->header_is(Server => 'Mojolicious (Perl)')
-  ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
-  ->content_is("Action died: doh!\n");
+  ->header_is('X-Powered-By' => 'Mojolicious (Perl)')->content_is("doh!\n\n");
 
 # Exceptional::this_one_does_not_exist (action does not exist)
-$t->get_ok('/exceptional/this_one_does_not_exist')->status_is(200)
+$t->get_ok('/exceptional/this_one_does_not_exist')->status_is(404)
   ->header_is(Server         => 'Mojolicious (Perl)')
   ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
-  ->json_content_is({error => 'not found!'});
+  ->content_like(qr/Page not found/);
 
 # Exceptional::this_one_does_not_exist (action behind bridge does not exist)
 $t->get_ok('/exceptional_too/this_one_does_not_exist', {'X-DoNotDie' => 1})
-  ->status_is(200)->header_is(Server => 'Mojolicious (Perl)')
+  ->status_is(404)->header_is(Server => 'Mojolicious (Perl)')
   ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
-  ->json_content_is({error => 'not found!'});
+  ->content_like(qr/Page not found/);
 
 # Foo::fun
 $t->get_ok('/fun/time', {'X-Test' => 'Hi there!'})->status_is(200)

diff --git a/t/mojolicious/lib/MojoliciousTest/Exceptional.pm b/t/mojolicious/lib/MojoliciousTest/Exceptional.pm
@@ -3,13 +3,6 @@ use Mojo::Base 'Mojolicious::Controller';
 
 # "Dr. Zoidberg, can you note the time and declare the patient legally dead?
 #  Can I! That’s my specialty!"
-sub render_exception {
-  my ($self, $e) = @_;
-  $self->render_text("Action died: $e");
-}
-
-sub render_not_found { shift->render_json({error => 'not found!'}) }
-
 sub this_one_dies { die "doh!\n" }
 
 sub this_one_might_die {

diff --git a/t/mojolicious/production_app.t b/t/mojolicious/production_app.t
@@ -45,31 +45,31 @@ $t->get_ok('/foo/syntaxerror')->status_is(500)
 $t->get_ok('/exceptional/this_one_dies')->status_is(500)
   ->header_is(Server         => 'Mojolicious (Perl)')
   ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
-  ->content_is("Action died: doh!\n");
+  ->content_like(qr/Internal Server Error/);
 
 # Exceptional::this_one_might_die (bridge dies)
 $t->get_ok('/exceptional_too/this_one_dies')->status_is(500)
   ->header_is(Server         => 'Mojolicious (Perl)')
   ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
-  ->content_is("Action died: double doh!\n");
+  ->content_like(qr/Internal Server Error/);
 
 # Exceptional::this_one_might_die (action dies)
 $t->get_ok('/exceptional_too/this_one_dies', {'X-DoNotDie' => 1})
   ->status_is(500)->header_is(Server => 'Mojolicious (Perl)')
   ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
-  ->content_is("Action died: doh!\n");
+  ->content_like(qr/Internal Server Error/);
 
 # Exceptional::this_one_does_not_exist (action does not exist)
-$t->get_ok('/exceptional/this_one_does_not_exist')->status_is(200)
+$t->get_ok('/exceptional/this_one_does_not_exist')->status_is(404)
   ->header_is(Server         => 'Mojolicious (Perl)')
   ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
-  ->json_content_is({error => 'not found!'});
+  ->content_like(qr/Page not found/);
 
 # Exceptional::this_one_does_not_exist (action behind bridge does not exist)
 $t->get_ok('/exceptional_too/this_one_does_not_exist', {'X-DoNotDie' => 1})
-  ->status_is(200)->header_is(Server => 'Mojolicious (Perl)')
+  ->status_is(404)->header_is(Server => 'Mojolicious (Perl)')
   ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
-  ->json_content_is({error => 'not found!'});
+  ->content_like(qr/Page not found/);
 
 # Static file /hello.txt in production mode
 $t->get_ok('/hello.txt')->status_is(200)