Bug 208699 - Move Throw{Code,Template}Error into Error.pm

r,a=justdave

7 files changed, 114 insertions, 44 deletions

diff --git a/Bugzilla/Auth/CGI.pm b/Bugzilla/Auth/CGI.pm
index 3588b7037..e223c9fee 100644
--- a/Bugzilla/Auth/CGI.pm
+++ b/Bugzilla/Auth/CGI.pm
@@ -32,6 +32,7 @@ use strict;
 
 use Bugzilla::Config;
 use Bugzilla::Constants;
+use Bugzilla::Error;
 use Bugzilla::Util;
 
 sub login {
@@ -102,12 +103,12 @@ sub login {
 
     # An error may have occurred with the login mechanism
     if ($authres == AUTH_ERROR) {
-        $::vars->{'authmethod'} = lc($authmethod);
-        $::vars->{'userid'} = $userid;
-        $::vars->{'auth_err_tag'} = $extra;
-        $::vars->{'info'} = $info;
-
-        &::ThrowCodeError("auth_err");
+        ThrowCodeError("auth_err",
+                       { authmethod => lc($authmethod),
+                         userid => $userid,
+                         auth_err_tag => $extra,
+                         info => $info
+                       });
     }
 
     # We can load the page if the login was ok, or there was no data
@@ -134,7 +135,7 @@ sub login {
                              'caneditaccount' => Bugzilla::Auth->can_edit,
                            }
                           )
-          || &::ThrowTemplateError($template->error());
+          || ThrowTemplateError($template->error());
 
         # This seems like as good as time as any to get rid of old
         # crufty junk in the logincookies table.  Get rid of any entry
@@ -150,7 +151,7 @@ sub login {
     # the password was just wrong. (This makes it harder for a cracker
     # to find account names by brute force)
     if ($authres == AUTH_LOGINFAILED) {
-        &::ThrowUserError("invalid_username_or_password");
+        ThrowUserError("invalid_username_or_password");
     }
 
     # The account may be disabled
@@ -163,16 +164,16 @@ sub login {
                           -expires => "Tue, 15-Sep-1998 21:49:00 GMT");
 
         # and throw a user error
-        &::ThrowUserError("account_disabled",
-                          {'disabled_reason' => $extra});
+        ThrowUserError("account_disabled",
+                       {'disabled_reason' => $extra});
     }
 
     # If we get here, then we've run out of options, which shouldn't happen
-    &::ThrowCodeError("authres_unhandled",
-                      { authres => $authres,
-                        type => $type,
-                      }
-                     );
+    ThrowCodeError("authres_unhandled",
+                   { authres => $authres,
+                     type => $type,
+                   }
+                  );
 
 }
 
diff --git a/Bugzilla/Error.pm b/Bugzilla/Error.pm
index a23d6db15..e511a575c 100644
--- a/Bugzilla/Error.pm
+++ b/Bugzilla/Error.pm
@@ -19,16 +19,18 @@
 #
 # Contributor(s): Bradley Baetz <bbaetz@acm.org>
 
-use strict;
-
 package Bugzilla::Error;
 
+use strict;
 use base qw(Exporter);
 
-@Bugzilla::Error::EXPORT = qw(ThrowUserError);
+@Bugzilla::Error::EXPORT = qw(ThrowCodeError ThrowTemplateError ThrowUserError);
 
-sub ThrowUserError {
-    my ($error, $vars, $unlock_tables) = @_;
+use Bugzilla::Config;
+use Bugzilla::Util;
+
+sub _throw_error {
+    my ($name, $error, $vars, $unlock_tables) = @_;
 
     $vars ||= {};
 
@@ -39,12 +41,56 @@ sub ThrowUserError {
     print Bugzilla->cgi->header();
 
     my $template = Bugzilla->template;
-    $template->process("global/user-error.html.tmpl", $vars)
-      || &::ThrowTemplateError($template->error());
+    $template->process($name, $vars)
+      || ThrowTemplateError($template->error());
 
     exit;
 }
 
+sub ThrowUserError {
+    _throw_error("global/user-error.html.tmpl", @_);
+}
+
+sub ThrowCodeError {
+    _throw_error("global/code-error.html.tmpl", @_);
+}
+
+sub ThrowTemplateError {
+    my ($template_err) = @_;
+
+    my $vars = {};
+
+    $vars->{'template_error_msg'} = $template_err;
+    $vars->{'error'} = "template_error";
+
+    my $template = Bugzilla->template;
+
+    # Try a template first; but if this one fails too, fall back
+    # on plain old print statements.
+    if (!$template->process("global/code-error.html.tmpl", $vars)) {
+        my $maintainer = Param('maintainer');
+        my $error = html_quote($vars->{'template_error_msg'});
+        my $error2 = html_quote($template->error());
+        print <<END;
+        <tt>
+          <p>
+            Bugzilla has suffered an internal error. Please save this page and 
+            send it to $maintainer with details of what you were doing at the 
+            time this message appeared.
+          </p>
+          <script type="text/javascript"> <!--
+            document.write("<p>URL: " + document.location + "</p>");
+          // -->
+          </script>
+          <p>Template->process() failed twice.<br>
+          First error: $error<br>
+          Second error: $error2</p>
+        </tt>
+END
+    }
+    exit;
+}
+
 1;
 
 __END__
@@ -82,6 +128,25 @@ error handling code will unlock the database tables. In the long term, this
 argument will go away, to be replaced by transactional C<rollback> calls. There
 is no timeframe for doing so, however.
 
+=item C<ThrowCodeError>
+
+This function is used when an internal check detects an error of some sort.
+This usually indicates a bug in Bugzilla, although it can occur if the user
+manually constructs urls without correct parameters.
+
+This function's behaviour is similar to C<ThrowUserError>, except that the
+template used to display errors is I<global/code-error.html.tmpl>. In addition
+if the hashref used as the optional second argument contains a key I<variables>
+then the contents of the hashref (which is expected to be another hashref) will
+be displayed after the error message, as a debugging aid.
+
+=item C<ThrowTemplateError>
+
+This function should only be called if a C<template-<gt>process()> fails.
+It tries another template first, because often one template being
+broken or missing doesn't mean that they all are. But it falls back to
+a print statement as a last-ditch error.
+
 =back
 
 =head1 SEE ALSO
diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index 4a1752cd0..fe54e9d65 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -153,17 +153,17 @@ sub validate {
         
         # Make sure the flag exists.
         my $flag = get($id);
-        $flag || &::ThrowCodeError("flag_nonexistent", { id => $id });
+        $flag || ThrowCodeError("flag_nonexistent", { id => $id });
 
         # Make sure the user chose a valid status.
         grep($status eq $_, qw(X + - ?))
-          || &::ThrowCodeError("flag_status_invalid", 
-                               { id => $id , status => $status });
+          || ThrowCodeError("flag_status_invalid", 
+                            { id => $id, status => $status });
                 
         # Make sure the user didn't request the flag unless it's requestable.
         if ($status eq '?' && !$flag->{type}->{is_requestable}) {
             ThrowCodeError("flag_status_invalid", 
-                              { id => $id , status => $status });
+                           { id => $id, status => $status });
         }
         
         # Make sure the requestee is authorized to access the bug.
@@ -584,7 +584,7 @@ sub notify {
       $::template->process($template_file, $::vars, \$message);
     if (!$rv) {
         Bugzilla->cgi->header();
-        &::ThrowTemplateError($::template->error());
+        ThrowTemplateError($::template->error());
     }
     
     my $delivery_mode = Param("sendmailnow") ? "" : "-ODeliveryMode=deferred";
diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm
index 7fbe1f142..6c3492ba2 100644
--- a/Bugzilla/FlagType.pm
+++ b/Bugzilla/FlagType.pm
@@ -198,13 +198,13 @@ sub validate {
         # Make sure the flag type exists.
         my $flag_type = get($id);
         $flag_type 
-          || &::ThrowCodeError("flag_type_nonexistent", { id => $id });
+          || ThrowCodeError("flag_type_nonexistent", { id => $id });
 
         # Make sure the value of the field is a valid status.
         grep($status eq $_, qw(X + - ?))
-          || &::ThrowCodeError("flag_status_invalid", 
-                               { id => $id , status => $status });
-                
+          || ThrowCodeError("flag_status_invalid", 
+                            { id => $id , status => $status });
+
         # Make sure the user didn't request the flag unless it's requestable.
         if ($status eq '?' && !$flag_type->{is_requestable}) {
             ThrowCodeError("flag_status_invalid", 
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index e795f03f3..09c47d471 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -930,7 +930,7 @@ sub init {
                 # chart -1 is generated by other code above, not from the user-
                 # submitted form, so we'll blindly accept any values in chart -1
                 if ((!$chartfields{$f}) && ($chart != -1)) {
-                    &::ThrowCodeError("invalid_field_name", {field => $f});
+                    ThrowCodeError("invalid_field_name", {field => $f});
                 }
 
                 # This is either from the internal chart (in which case we
@@ -964,9 +964,10 @@ sub init {
                 }
                 else {
                     # This field and this type don't work together.
-                    $::vars->{'field'} = $params->param("field$chart-$row-$col");
-                    $::vars->{'type'} = $params->param("type$chart-$row-$col");
-                    &::ThrowCodeError("field_type_mismatch");
+                    ThrowCodeError("field_type_mismatch",
+                                   { field => $params->param("field$chart-$row-$col"),
+                                     type => $params->param("type$chart-$row-$col"),
+                                   });
                 }
             }
             if (@orlist) {
diff --git a/Bugzilla/Token.pm b/Bugzilla/Token.pm
index 400d7d4fc..f7be40ab3 100644
--- a/Bugzilla/Token.pm
+++ b/Bugzilla/Token.pm
@@ -30,6 +30,7 @@ use strict;
 package Token;
 
 use Bugzilla::Config;
+use Bugzilla::Error;
 
 use Date::Format;
 
@@ -88,7 +89,7 @@ sub IssueEmailChangeToken {
 
     my $message;
     $template->process("account/email/change-old.txt.tmpl", $vars, \$message)
-      || &::ThrowTemplateError($template->error());
+      || ThrowTemplateError($template->error());
 
     open SENDMAIL, "|/usr/lib/sendmail -t -i";
     print SENDMAIL $message;
@@ -99,7 +100,7 @@ sub IssueEmailChangeToken {
 
     $message = "";
     $template->process("account/email/change-new.txt.tmpl", $vars, \$message)
-      || &::ThrowTemplateError($template->error());
+      || ThrowTemplateError($template->error());
 
     open SENDMAIL, "|/usr/lib/sendmail -t -i";
     print SENDMAIL $message;
@@ -146,7 +147,7 @@ sub IssuePasswordToken {
     my $message = "";
     $template->process("account/password/forgotten-password.txt.tmpl", 
                                                                $vars, \$message)
-      || &::ThrowTemplateError($template->error());
+      || ThrowTemplateError($template->error());
 
     open SENDMAIL, "|/usr/lib/sendmail -t -i";
     print SENDMAIL $message;
@@ -176,7 +177,7 @@ sub GenerateUniqueToken {
 
         ++$tries;
         if ($tries > 100) {
-            &::ThrowCodeError("token_generation_error");
+            ThrowCodeError("token_generation_error");
         }
 
         $token = &::GenerateRandomPassword();
@@ -225,7 +226,7 @@ sub Cancel {
 
     my $message;
     $template->process("account/cancel-token.txt.tmpl", $vars, \$message)
-      || &::ThrowTemplateError($template->error());
+      || ThrowTemplateError($template->error());
 
     open SENDMAIL, "|/usr/lib/sendmail -t -i";
     print SENDMAIL $message;
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index f5df92063..32e624913 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -33,6 +33,7 @@ use strict;
 package Bugzilla::User;
 
 use Bugzilla::Config;
+use Bugzilla::Error;
 use Bugzilla::Util;
 
 ################################################################################
@@ -551,9 +552,10 @@ sub match_field {
         }
         else {
             # bad argument
-            $vars->{'argument'} = $fields->{$field}->{'type'};
-            $vars->{'function'} = 'Bugzilla::User::match_field';
-            &::ThrowCodeError('bad_arg');
+            ThrowCodeError('bad_arg',
+                           { argument => $fields->{$field}->{'type'},
+                             function =>  'Bugzilla::User::match_field',
+                           });
         }
 
         for my $query (@queries) {
@@ -623,7 +625,7 @@ sub match_field {
     print Bugzilla->cgi->header();
 
     $::template->process("global/confirm-user-match.html.tmpl", $vars)
-      || &::ThrowTemplateError($::template->error());
+      || ThrowTemplateError($::template->error());
 
     exit;