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

r,a=justdave

16 files changed, 168 insertions, 180 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;
 
diff --git a/CGI.pl b/CGI.pl
index 8b33ce102..75174dcba 100644
--- a/CGI.pl
+++ b/CGI.pl
@@ -95,14 +95,15 @@ sub CheckFormField (\%$;\@) {
 
         SendSQL("SELECT description FROM fielddefs WHERE name=" . SqlQuote($fieldname));
         my $result = FetchOneColumn();
+        my $field;
         if ($result) {
-            $vars->{'field'} = $result;
+            $field = $result;
         }
         else {
-            $vars->{'field'} = $fieldname;
+            $field = $fieldname;
         }
         
-        ThrowCodeError("illegal_field", undef, "abort");
+        ThrowCodeError("illegal_field", { field => $field }, "abort");
       }
 }
 
@@ -113,8 +114,7 @@ sub CheckFormFieldDefined (\%$) {
        ) = @_;
 
     if (!defined $formRef->{$fieldname}) {
-        $vars->{'field'} = $fieldname;  
-        ThrowCodeError("undefined_field");
+        ThrowCodeError("undefined_field", { field => $fieldname });
     }
 }
 
@@ -241,81 +241,6 @@ sub PutFooter {
       || ThrowTemplateError($::template->error());
 }
 
-###############################################################################
-# Error handling
-#
-# If you are doing incremental output, set $vars->{'header_done'} once you've
-# done the header.
-#
-# You can call Throw*Error with extra template variables in one pass by using
-# the $extra_vars hash reference parameter:
-# ThrowUserError("some_tag", { bug_id => $bug_id, size => 127 });
-###############################################################################
-
-# For "this shouldn't happen"-type places in the code.
-# The contents of $extra_vars get printed out in the template - useful for
-# debugging info.
-sub ThrowCodeError {
-  ($vars->{'error'}, my $extra_vars, my $unlock_tables) = (@_);
-
-  SendSQL("UNLOCK TABLES") if $unlock_tables;
-
-  # If we don't have this test here, then the %@extra_vars vivifies
-  # the hashref, and then setting $vars->{'variables'} uses an empty hashref
-  # so the error template prints out a bogus header for the empty hash
-  if (defined $extra_vars) {
-      # Copy the extra_vars into the vars hash 
-      foreach my $var (keys %$extra_vars) {
-          $vars->{$var} = $extra_vars->{$var};
-      }
-
-      # We may one day log something to file here also.
-      $vars->{'variables'} = $extra_vars;
-  }
-  
-  print Bugzilla->cgi->header();
-  $template->process("global/code-error.html.tmpl", $vars)
-    || ThrowTemplateError($template->error());
-    
-  exit;
-}
-
-# This function should only be called if a template->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 on
-# a print statement.
-# The Content-Type will already have been printed.
-sub ThrowTemplateError {
-    ($vars->{'template_error_msg'}) = (@_);
-    $vars->{'error'} = "template_error";
-    
-    # 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;  
-}
-
 sub CheckIfVotedConfirmed {
     my ($id, $who) = (@_);
     SendSQL("SELECT bugs.votes, bugs.bug_status, products.votestoconfirm, " .
diff --git a/Token.pm b/Token.pm
index 400d7d4fc..f7be40ab3 100644
--- a/Token.pm
+++ b/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/attachment.cgi b/attachment.cgi
index 07dbe5e51..d020b4f57 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -153,7 +153,7 @@ elsif ($action eq "update")
 }
 else 
 { 
-  ThrowCodeError("unknown_action");
+  ThrowCodeError("unknown_action", { action => $action });
 }
 
 exit;
@@ -302,8 +302,8 @@ sub validateContentType
   }
   else
   {
-    $vars->{'contenttypemethod'} = $::FORM{'contenttypemethod'};
-    ThrowCodeError("illegal_content_type_method");
+    ThrowCodeError("illegal_content_type_method",
+                   { contenttypemethod => $::FORM{'contenttypemethod'} });
   }
 
   if ( $::FORM{'contenttype'} !~ /^(application|audio|image|message|model|multipart|text|video)\/.+$/ )
@@ -387,13 +387,11 @@ sub validateObsolete
   # Make sure the attachment id is valid and the user has permissions to view
   # the bug to which it is attached.
   foreach my $attachid (@{$::MFORM{'obsolete'}}) {
-    # my $vars after ThrowCodeError is updated to not use the global
-    # vars hash
-
+    my $vars = {};
     $vars->{'attach_id'} = $attachid;
     
     detaint_natural($attachid)
-      || ThrowCodeError("invalid_attach_id_to_obsolete");
+      || ThrowCodeError("invalid_attach_id_to_obsolete", $vars);
   
     SendSQL("SELECT bug_id, isobsolete, description 
              FROM attachments WHERE attach_id = $attachid");
@@ -410,12 +408,12 @@ sub validateObsolete
     {
       $vars->{'my_bug_id'} = $::FORM{'bugid'};
       $vars->{'attach_bug_id'} = $bugid;
-      ThrowCodeError("mismatched_bug_ids_on_obsolete");
+      ThrowCodeError("mismatched_bug_ids_on_obsolete", $vars);
     }
 
     if ( $isobsolete )
     {
-      ThrowCodeError("attachment_already_obsolete");
+      ThrowCodeError("attachment_already_obsolete", $vars);
     }
 
     # Check that the user can modify this attachment
diff --git a/buglist.cgi b/buglist.cgi
index bff5e75e0..f56b007ad 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -544,14 +544,14 @@ if ($order) {
                 # Accept an order fragment matching a column name, with
                 # asc|desc optionally following (to specify the direction)
                 if (!grep($fragment =~ /^\Q$_\E(\s+(asc|desc))?$/, @columnnames)) {
-                    $vars->{'fragment'} = $fragment;
+                    my $vars = { fragment => $fragment };
                     if ($order_from_cookie) {
                         $cgi->send_cookie(-name => 'LASTORDER',
                                           -expires => 'Tue, 15-Sep-1998 21:49:00 GMT');
-                        ThrowCodeError("invalid_column_name_cookie");
+                        ThrowCodeError("invalid_column_name_cookie", $vars);
                     }
                     else {
-                        ThrowCodeError("invalid_column_name_form");
+                        ThrowCodeError("invalid_column_name_form", $vars);
                     }
                 }
             }
diff --git a/post_bug.cgi b/post_bug.cgi
index f0d4f0816..9a1ea747f 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -477,7 +477,7 @@ $vars->{'id'} = $id;
 my $bug = new Bug($id, $::userid);
 $vars->{'bug'} = $bug;
 
-ThrowCodeError("bug_error") if $bug->error;
+ThrowCodeError("bug_error", { bug => $bug }) if $bug->error;
 
 $vars->{'sentmail'} = [];
 
diff --git a/process_bug.cgi b/process_bug.cgi
index cf9abf4cb..fde735434 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -976,9 +976,8 @@ SWITCH: for ($::FORM{'knob'}) {
 
         last SWITCH;
     };
-    
-    $vars->{'action'} = $::FORM{'knob'};
-    ThrowCodeError("unknown_action");
+
+    ThrowCodeError("unknown_action", { action => $::FORM{'knob'} });
 }
 
 
@@ -1746,8 +1745,7 @@ foreach my $id (@idlist) {
 if ($next_bug) {
     if (detaint_natural($next_bug) && CanSeeBug($next_bug, $::userid)) {
         my $bug = new Bug($next_bug, $::userid);
-        $vars->{'bug'} = $bug;
-        ThrowCodeError("bug_error") if $bug->error;
+        ThrowCodeError("bug_error", { bug => $bug }) if $bug->error;
 
         $template->process("bug/process/next.html.tmpl", $vars)
           || ThrowTemplateError($template->error());
diff --git a/request.cgi b/request.cgi
index ae137959d..3672e0449 100755
--- a/request.cgi
+++ b/request.cgi
@@ -165,10 +165,10 @@ sub queue {
                     push(@criteria, "bugs.component_id = $component_id");
                     push(@excluded_columns, 'component') unless $::FORM{'do_union'};
                 }
-                else { ThrowCodeError("unknown_component", { %::FORM }) }
+                else { ThrowCodeError("unknown_component", { component => $::FORM{component} }) }
             }
         }
-        else { ThrowCodeError("unknown_product", { %::FORM }) }
+        else { ThrowCodeError("unknown_product", { product => $::FORM{product} }) }
     }
     
     # Filter results by flag types.
@@ -281,7 +281,8 @@ sub validateStatus {
     return if !defined($::FORM{'status'});
     
     grep($::FORM{'status'} eq $_, qw(? +- + - all))
-      || ThrowCodeError("flag_status_invalid", { status => $::FORM{'status'} });
+      || ThrowCodeError("flag_status_invalid",
+                        { status => $::FORM{'status'} });
 }
 
 sub validateGroup {
diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl
index 939c0c4dc..73501672b 100644
--- a/template/en/default/global/code-error.html.tmpl
+++ b/template/en/default/global/code-error.html.tmpl
@@ -39,7 +39,7 @@
 
 [% error_message = BLOCK %]
   [% IF    error == "action_unrecognized" %]
-    I don't recognize the value (<em>[% variables.action FILTER html %]</em>)
+    I don't recognize the value (<em>[% action FILTER html %]</em>)
     of the <em>action</em> variable.
   
   [% ELSIF error == "attachment_already_obsolete" %]
@@ -85,9 +85,6 @@
     Charts will not work without the GD Perl module being installed.
     Run checksetup.pl for installation instructions.
 
-  [% ELSIF error == "group_bit_invalid" %]
-    One of the group bits submitted was invalid.
-
   [% ELSIF error == "illegal_content_type_method" %]
     Your form submission got corrupted somehow.  The <em>content
     method</em> field, which specifies how the content type gets determined,
@@ -142,27 +139,31 @@
     [% terms.bug %] [%+ my_bug_id FILTER html %].
         
   [% ELSIF error == "flag_nonexistent" %]
-    There is no flag with ID #[% variables.id FILTER html %].
+    There is no flag with ID #[% id FILTER html %].
   
   [% ELSIF error == "flag_status_invalid" %]
-    The flag status <em>[% variables.status FILTER html %]</em> is invalid.
+    The flag status <em>[% status FILTER html %]</em>
+    [% IF id %]
+      for flag ID #[% id FILTER html %]
+    [% END %]
+    is invalid.
   
   [% ELSIF error == "flag_type_component_nonexistent" %]
-    The component <em>[% variables.component FILTER html %]</em> does not exist
-    in the product <em>[% variables.product FILTER html %]</em>.
+    The component <em>[% component FILTER html %]</em> does not exist
+    in the product <em>[% product FILTER html %]</em>.
   
   [% ELSIF error == "flag_type_component_without_product" %]
     A component was selected without a product being selected.
   
   [% ELSIF error == "flag_type_id_invalid" %]
-    The flag type ID <em>[% variables.id FILTER html %]</em> is not
+    The flag type ID <em>[% id FILTER html %]</em> is not
     a positive integer.
 
   [% ELSIF error == "flag_type_nonexistent" %]
-    There is no flag type with the ID <em>[% variables.id FILTER html %]</em>.
+    There is no flag type with the ID <em>[% id FILTER html %]</em>.
   
   [% ELSIF error == "flag_type_product_nonexistent" %]
-    The product <em>[% variables.product FILTER html %]</em> does not exist.
+    The product <em>[% product FILTER html %]</em> does not exist.
   
   [% ELSIF error == "flag_type_target_type_invalid" %]
     The target type was neither <em>[% terms.bug %]</em> nor <em>attachment</em>
@@ -171,10 +172,6 @@
   [% ELSIF error == "invalid_field_name" %]
     Can't use [% field FILTER html %] as a field name.
     
-  [% ELSIF error == "invalid_output_type" %]
-    [% title = "Invalid Output Type" %]
-    Invalid output type [% type FILTER html %].
-    
   [% ELSIF error == "missing_bug_id" %]
     No [% terms.bug %] ID was given.
     
@@ -195,10 +192,10 @@
     The group field <em>[% group FILTER html %]</em> is invalid.
 
   [% ELSIF error == "report_axis_invalid" %]
-    <em>[% variables.val FILTER html %]</em> is not a valid value for
-    [%+ IF    variables.fld == "x" %]the horizontal axis
-    [%+ ELSIF variables.fld == "y" %]the vertical axis
-    [%+ ELSIF variables.fld == "z" %]the multiple tables/images
+    <em>[% val FILTER html %]</em> is not a valid value for 
+    [%+ IF    fld == "x" %]the horizontal axis
+    [%+ ELSIF fld == "y" %]the vertical axis
+    [%+ ELSIF fld == "z" %]the multiple tables/images
     [%+ ELSE %]a report axis[% END %] field.
 
   [% ELSIF error == "token_generation_error" %]
@@ -222,21 +219,19 @@
 
   [% ELSIF error == "unknown_component" %]
     [% title = "Unknown Component" %]
-    There is no component named <em>[% variables.component FILTER html %]</em>.
+    There is no component named <em>[% component FILTER html %]</em>.
 
   [% ELSIF error == "unknown_product" %]
     [% title = "Unknown Product" %]
-    There is no product named <em>[% variables.product FILTER html %]</em>.
+    There is no product named <em>[% product FILTER html %]</em>.
 
   [% ELSE %]
-    [%# Give sensible error if error functions are used incorrectly.
-      #%]
-    You are using [% terms.Bugzilla %]'s ThrowCodeError() function incorrectly. 
-    You passed in the string '[% error FILTER html %]'. The correct use is to 
-    pass in a tag, and define that tag in the file code-error.html.tmpl.<br>
-    <br>
-    If you are a [% terms.Bugzilla %] end-user seeing this message, please save this
-    page and send it to [% Param('maintainer') %].
+    [% title = "Internal error" %]
+    An internal error has occured, but [% terms.Bugzilla %] doesn't know
+    what <code>[% error FILTER html %]</code> means.
+
+    If you are a [% terms.Bugzilla %] end-user seeing this message, please save
+    this page and send it to [% Param('maintainer') %].
   [% END %]
 [% END %]
 
diff --git a/token.cgi b/token.cgi
index 25f68b70e..76c1a2e11 100755
--- a/token.cgi
+++ b/token.cgi
@@ -146,7 +146,7 @@ if ($::action eq 'reqpw') {
     # If the action that the user wants to take (specified in the "a" form field)
     # is none of the above listed actions, display an error telling the user 
     # that we do not understand what they would like to do.
-    ThrowCodeError("unknown_action");
+    ThrowCodeError("unknown_action", { action => $::action });
 }
 
 exit;