Bug 1240029 - The REST API doesn't work correctly with mod_perl

r=dylan

4 files changed, 17 insertions, 20 deletions

diff --git a/Bugzilla/API/1_0/Server.pm b/Bugzilla/API/1_0/Server.pm
index 743af59a1..627da1e58 100644
--- a/Bugzilla/API/1_0/Server.pm
+++ b/Bugzilla/API/1_0/Server.pm
@@ -211,12 +211,18 @@ sub _handle {
     my $method     = $self->method_name;
     my $controller = $self->controller;
     my $params     = Bugzilla->input_params;
+    my $cache      = Bugzilla->request_cache;
 
     unless ($controller->can($method)) {
         return $self->return_error(302, "No such a method : '$method'.");
     }
 
-    my $result = eval q| $controller->$method($params) |;
+    # Let Bugzilla::Error know we are inside an eval() for exceptions
+    $cache->{in_eval} = 1;
+    my $result = eval { $controller->$method($params) };
+    $cache->{in_eval} = 0;
+
+    return $self->return_error if $self->return_error;
 
     if ($@) {
         return $self->return_error(500, "Procedure error: $@");
diff --git a/Bugzilla/Error.pm b/Bugzilla/Error.pm
index ee40ccf8b..77a272e1e 100644
--- a/Bugzilla/Error.pm
+++ b/Bugzilla/Error.pm
@@ -23,30 +23,14 @@ use Carp;
 use Data::Dumper;
 use Date::Format;
 
-# We cannot use $^S to detect if we are in an eval(), because mod_perl
-# already eval'uates everything, so $^S = 1 in all cases under mod_perl!
-sub _in_eval {
-    my $in_eval = 0;
-    for (my $stack = 1; my $sub = (caller($stack))[3]; $stack++) {
-        last if $sub =~ /^(?:ModPerl|Plack|CGI::Compile)/;
-        # An eval followed by CGI::Compile is not a "real" eval.
-        $in_eval = 1 if $sub =~ /^\(eval\)/ && (caller($stack + 1))[3] !~ /^CGI::Compile/;
-    }
-    return $in_eval;
-}
-
 sub _throw_error {
     my ($name, $error, $vars) = @_;
     my $dbh = Bugzilla->dbh;
+    my $cache = Bugzilla->request_cache;
     $vars ||= {};
 
     $vars->{error} = $error;
 
-    # Make sure any transaction is rolled back (if supported).
-    # If we are within an eval(), do not roll back transactions as we are
-    # eval'uating some test on purpose.
-    $dbh->bz_rollback_transaction() if ($dbh->bz_in_transaction() && !_in_eval());
-
     my $datadir = bz_locations()->{'datadir'};
     # If a writable $datadir/errorlog exists, log error details there.
     if (-w "$datadir/errorlog") {
@@ -138,7 +122,7 @@ sub _throw_error {
             # of JSON::RPC. So, in that circumstance, instead of exiting,
             # we die with no message. JSON::RPC checks raise_error before
             # it checks $@, so it returns the proper error.
-            die if _in_eval();
+            die if $cache->{in_eval};
             $server->response($server->error_response_header);
         }
         else {
@@ -146,6 +130,7 @@ sub _throw_error {
             my %status_code_map = %{ $server->constants->{REST_STATUS_CODE_MAP} };
             my $status_code = $status_code_map{$code} || $status_code_map{'_default'};
             $server->return_error($status_code, $message, $code);
+            die if $cache->{in_eval};
             $server->response;
         }
     }
diff --git a/Bugzilla/WebService/Server/JSONRPC.pm b/Bugzilla/WebService/Server/JSONRPC.pm
index 3fa0b65a9..a75bba836 100644
--- a/Bugzilla/WebService/Server/JSONRPC.pm
+++ b/Bugzilla/WebService/Server/JSONRPC.pm
@@ -273,6 +273,9 @@ sub _handle {
 
     my $result = $self->SUPER::_handle(@_);
 
+    # Reset in_eval to so we get normal exceptions from here
+    Bugzilla->request_cache->{in_eval} = 0;
+
     # Set the ETag if not already set in the webservice methods.
     my $etag = $self->bz_etag;
     if (!$etag && ref $result) {
@@ -425,6 +428,9 @@ sub _argument_type_check {
         $params = [$params];
     }
 
+    # Let Bugzilla::Error know we are inside an eval() after this point
+    Bugzilla->request_cache->{in_eval} = 1;
+
     return $params;
 }
 
diff --git a/email_in.pl b/email_in.pl
index 448d75d0e..6d81e00dc 100755
--- a/email_in.pl
+++ b/email_in.pl
@@ -466,7 +466,7 @@ sub die_handler {
 
     # If this is inside an eval, then we should just act like...we're
     # in an eval (instead of printing the error and exiting).
-    die @_ if ($^S // Bugzilla::Error::_in_eval());
+    die @_ if ($^S // Bugzilla->request_cache->{in_eval});
 
     # We can't depend on the MTA to send an error message, so we have
     # to generate one properly.