Bug 1762504 - Update Exception handling to allow native Mojo code and…

… older legacy code to work along side each other
mozilla-bteam · Apr 20, 2022 · 0f31983 · 0f31983
1 parent b517f32
commit 0f31983
Show file tree

Hide file tree

Showing 9 changed files with 66 additions and 37 deletions.
diff --git a/Bugzilla.pm b/Bugzilla.pm
@@ -518,7 +518,7 @@ sub usage_mode {
     elsif ($newval == USAGE_MODE_REST) {
       $class->error_mode(ERROR_MODE_REST);
     }
-    elsif ($newval == USAGE_MODE_MOJO) {
+    elsif ($newval == USAGE_MODE_MOJO || $newval == USAGE_MODE_MOJO_REST) {
       $class->error_mode(ERROR_MODE_MOJO);
     }
     else {

diff --git a/Bugzilla/API/V1/Configuration.pm b/Bugzilla/API/V1/Configuration.pm
@@ -27,6 +27,7 @@ sub setup_routes {
 
 sub configuration {
   my ($self) = @_;
+  Bugzilla->usage_mode(USAGE_MODE_MOJO_REST);
   my $user = $self->bugzilla->login;
 
   my $can_cache = !$user->id && !$self->param('product') && !$self->param('flags');

diff --git a/Bugzilla/API/V1/Teams.pm b/Bugzilla/API/V1/Teams.pm
@@ -18,6 +18,7 @@ sub setup_routes {
 
 sub component_teams {
   my ($self) = @_;
+  Bugzilla->usage_mode(USAGE_MODE_MOJO_REST);
   $self->bugzilla->login();
   my $result;
   if (my $team = $self->param('team')) {

diff --git a/Bugzilla/API/V1/User.pm b/Bugzilla/API/V1/User.pm
@@ -20,6 +20,7 @@ sub setup_routes {
 
 sub user_profile {
   my ($self) = @_;
+  Bugzilla->usage_mode(USAGE_MODE_MOJO_REST);
   my $user = $self->bugzilla->oauth('user:read');
   if ($user && $user->id) {
     $self->render(

diff --git a/Bugzilla/App.pm b/Bugzilla/App.pm
@@ -143,6 +143,8 @@ sub startup {
       Bugzilla->clear_request_cache();
       # We also need to clear CGI's globals.
       CGI::initialize_globals();
+      # Store away the default controller for use by non-Mojo legacy code
+      Bugzilla->request_cache->{mojo_controller} = $_[0];
     }
   );
   $self->hook(after_dispatch => sub {

diff --git a/Bugzilla/App/Plugin/Error.pm b/Bugzilla/App/Plugin/Error.pm
@@ -13,33 +13,63 @@ use Bugzilla::Constants;
 use Bugzilla::Logging;
 use Bugzilla::WebService::Constants;
 
+my $EXCEPTION_HELPER;
+
 sub register {
   my ($self, $app) = @_;
+
+  # For Bugzilla::Error exceptions when using Mojo native
+  # code and some function calls Throw{Code,User}Error().
+  $EXCEPTION_HELPER = $app->renderer->get_helper('reply.exception');
+  $app->helper('reply.exception' => sub { _render_error('code', @_); });
+
   $app->helper('code_error' => sub { _render_error('code', @_); });
   $app->helper('user_error' => sub { _render_error('user', @_); });
 }
 
 sub _render_error {
   my ($type, $c, $error, $vars) = @_;
+
+  # If values are defined in the stash, use those instead
+  my $stash = $c->stash;
+  $type  = $stash->{type}  if $stash->{type};
+  $error = $stash->{error} if $stash->{error};
+  $vars  = $stash->{vars}  if $stash->{vars};
+
   my $logfunc = _make_logfunc(ucfirst($type));
 
+  # Find the full error message
+  my $message;
+  $vars->{error} = $error;
+  my $template = Bugzilla->template;
+  $template->process("global/$type-error.html.tmpl", $vars, \$message)
+    || die $template->error();
+
   # Errors displayed in a web page
-  if (Bugzilla->error_mode == ERROR_MODE_MOJO
-    || Bugzilla->error_mode == ERROR_MODE_WEBPAGE)
-  {
+  if (Bugzilla->usage_mode == USAGE_MODE_MOJO) {
     $logfunc->("webpage error: $error");
 
-    $c->render(
-      handler  => 'bugzilla',
-      template => "global/$type-error",
-      format   => 'html',
-      error    => $error,
-      %{$vars}
-    );
+    if ($c->app->mode eq 'development') {
+      use Bugzilla::Logging;
+      my $class = $type ? 'Bugzilla::Error::' . ucfirst($type) : 'Mojo::Exception';
+      my $e     = $class->new($error)->trace(2);
+      $e->vars($vars) if $e->can('vars');
+      $EXCEPTION_HELPER->($c, $e->inspect);
+    }
+    else {
+      $c->render(
+        handler  => 'bugzilla',
+        template => "global/$type-error",
+        format   => 'html',
+        error    => $error,
+        status   => 200,
+        %{$vars}
+      );
+    }
   }
 
   # Errors returned in an API request
-  elsif (Bugzilla->error_mode == ERROR_MODE_REST) {
+  elsif (Bugzilla->usage_mode == USAGE_MODE_MOJO_REST) {
     my %error_map = %{WS_ERROR_CODE()};
     my $code      = $error_map{$error};
 
@@ -53,13 +83,6 @@ sub _render_error {
 
     $logfunc->("REST error: $error (HTTP $status_code, internal code $code)");
 
-    # Find the full error message
-    my $message;
-    $vars->{error} = $error;
-    my $template = Bugzilla->template;
-    $template->process("global/$type-error.html.tmpl", $vars, \$message)
-      || die $template->error();
-
     my $error = {
       error         => 1,
       code          => $code,

diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
@@ -140,6 +140,7 @@ use Memoize;
   USAGE_MODE_TEST
   USAGE_MODE_REST
   USAGE_MODE_MOJO
+  USAGE_MODE_MOJO_REST
 
   ERROR_MODE_WEBPAGE
   ERROR_MODE_DIE
@@ -148,6 +149,7 @@ use Memoize;
   ERROR_MODE_TEST
   ERROR_MODE_REST
   ERROR_MODE_MOJO
+  ERROR_MODE_MOJO_REST
 
   COLOR_ERROR
   COLOR_SUCCESS
@@ -507,14 +509,15 @@ use constant contenttypes => {
 };
 
 # Usage modes. Default USAGE_MODE_BROWSER. Use with Bugzilla->usage_mode.
-use constant USAGE_MODE_BROWSER => 0;
-use constant USAGE_MODE_CMDLINE => 1;
-use constant USAGE_MODE_XMLRPC  => 2;
-use constant USAGE_MODE_EMAIL   => 3;
-use constant USAGE_MODE_JSON    => 4;
-use constant USAGE_MODE_TEST    => 5;
-use constant USAGE_MODE_REST    => 6;
-use constant USAGE_MODE_MOJO    => 7;
+use constant USAGE_MODE_BROWSER   => 0;
+use constant USAGE_MODE_CMDLINE   => 1;
+use constant USAGE_MODE_XMLRPC    => 2;
+use constant USAGE_MODE_EMAIL     => 3;
+use constant USAGE_MODE_JSON      => 4;
+use constant USAGE_MODE_TEST      => 5;
+use constant USAGE_MODE_REST      => 6;
+use constant USAGE_MODE_MOJO      => 7;
+use constant USAGE_MODE_MOJO_REST => 8;
 
 # Error modes. Default set by Bugzilla->usage_mode (so ERROR_MODE_WEBPAGE
 # usually). Use with Bugzilla->error_mode.
@@ -525,6 +528,7 @@ use constant ERROR_MODE_JSON_RPC       => 3;
 use constant ERROR_MODE_TEST           => 4;
 use constant ERROR_MODE_REST           => 5;
 use constant ERROR_MODE_MOJO           => 6;
+use constant ERROR_MODE_MOJO_REST      => 7;
 
 # The ANSI colors of messages that command-line scripts use
 use constant COLOR_ERROR   => 'red';

diff --git a/Bugzilla/Error.pm b/Bugzilla/Error.pm
@@ -52,14 +52,6 @@ sub _throw_error {
   $dbh->bz_rollback_transaction()
     if ($dbh && $dbh->bz_in_transaction() && !_in_eval());
 
-  if (Bugzilla->error_mode == ERROR_MODE_MOJO) {
-    my ($type) = $name =~ /^global\/(user|code)-error/;
-    my $class = $type ? 'Bugzilla::Error::' . ucfirst($type) : 'Mojo::Exception';
-    my $e = $class->new($error)->trace(2);
-    $e->vars($vars) if $e->can('vars');
-    CORE::die $e->inspect;
-  }
-
   $vars->{error} = $error;
   my $template = Bugzilla->template;
   my $message;
@@ -86,6 +78,12 @@ sub _throw_error {
     die Template::Exception->new("bugzilla.$type.$error", $vars);
   }
 
+  if (Bugzilla->error_mode == ERROR_MODE_MOJO) {
+    my ($type) = $name =~ /^global\/(user|code)-error/;
+    my $c = Bugzilla->request_cache->{mojo_controller};
+    $c->stash({type => $type, error => $error, message => $message, vars => $vars,}) and die;
+  }
+
   if (Bugzilla->error_mode == ERROR_MODE_WEBPAGE) {
     Bugzilla->check_rate_limit("webpage_errors", remote_ip(), sub { $vars->{rate_limit_error} = 1 });
     my $cgi = Bugzilla->cgi;

diff --git a/t/quicksearch.t b/t/quicksearch.t
@@ -22,12 +22,11 @@ use ok 'Bugzilla::Search';
 use ok 'Bugzilla::Search::Quicksearch';
 
 my $CGI = mock 'Bugzilla::CGI' => (add_constructor => [fake_new => 'hash',]);
-Bugzilla->usage_mode(USAGE_MODE_MOJO);
 Bugzilla->request_cache->{cgi} = Bugzilla::CGI->fake_new();
 
 like(
   dies { quicksearch('') },
-  qr/buglist_parameters_required/,
+  qr/without any search terms/,
   "Got right exception"
 );