diff options
| -rw-r--r-- | Bugzilla/Error.pm | 19 | ||||
| -rwxr-xr-x | Bugzilla/WebService.pm | 37 | ||||
| -rwxr-xr-x | Bugzilla/WebService/Constants.pm | 41 | ||||
| -rw-r--r-- | t/012throwables.t | 23 |
4 files changed, 110 insertions, 10 deletions
diff --git a/Bugzilla/Error.pm b/Bugzilla/Error.pm index 1bb0556af..95088850e 100644 --- a/Bugzilla/Error.pm +++ b/Bugzilla/Error.pm @@ -81,16 +81,21 @@ sub _throw_error { $template->process($name, $vars) || ThrowTemplateError($template->error()); } - elsif (Bugzilla->error_mode == ERROR_MODE_DIE) { + else { my $message; $template->process($name, $vars, \$message) || ThrowTemplateError($template->error()); - die("$message\n"); - } - elsif (Bugzilla->error_mode == ERROR_MODE_DIE_SOAP_FAULT) { - die SOAP::Fault - ->faultcode(ERROR_GENERAL) - ->faultstring($error); + if (Bugzilla->error_mode == ERROR_MODE_DIE) { + die("$message\n"); + } + elsif (Bugzilla->error_mode == ERROR_MODE_DIE_SOAP_FAULT) { + my $code = WS_ERROR_CODE->{$error}; + if (!$code) { + $code = ERROR_UNKNOWN_FATAL if $name =~ /code/i; + $code = ERROR_UNKNOWN_TRANSIENT if $name =~ /user/i; + } + die SOAP::Fault->faultcode($code)->faultstring($message); + } } exit; } diff --git a/Bugzilla/WebService.pm b/Bugzilla/WebService.pm index edb3bd1f9..efe8258df 100755 --- a/Bugzilla/WebService.pm +++ b/Bugzilla/WebService.pm @@ -13,6 +13,7 @@ # The Original Code is the Bugzilla Bug Tracking System. # # Contributor(s): Marc Schumann <wurblzap@gmail.com> +# Max Kanat-Alexander <mkanat@bugzilla.org> package Bugzilla::WebService; @@ -75,3 +76,39 @@ Certain parts of a method's description may be marked as B<UNSTABLE>, in which case those parts are not guaranteed to stay the same between Bugzilla versions. +=head1 ERRORS + +If a particular webservice call fails, it will throw a standard XML-RPC +error. There will be a numeric error code, and then the description +field will contain descriptive text of the error. Each error that Bugzilla +can throw has a specific code that will not change between versions of +Bugzilla. + +The various errors that functions can throw are specified by the +documentation of those functions. + +If your code needs to know what error Bugzilla threw, use the numeric +code. Don't try to parse the description, because that may change +from version to version of Bugzilla. + +Note that if you display the error to the user in an HTML program, make +sure that you properly escape the error, as it will not be HTML-escaped. + +=head2 Transient vs. Fatal Errors + +If the error code is a number greater than 0, the error is considered +"transient," which means that it was an error made by the user, not +some problem with Bugzilla itself. + +If the error code is a number less than 0, the error is "fatal," which +means that it's some error in Bugzilla itself that probably requires +administrative attention. + +Negative numbers and positive numbers don't overlap. That is, if there's +an error 302, there won't be an error -302. + +=head2 Unknown Errors + +Sometimes a function will throw an error that doesn't have a specific +error code. In this case, the code will be C<-32000> if it's a "fatal" +error, and C<32000> if it's a "transient" error. diff --git a/Bugzilla/WebService/Constants.pm b/Bugzilla/WebService/Constants.pm index 2e9457add..f9728e246 100755 --- a/Bugzilla/WebService/Constants.pm +++ b/Bugzilla/WebService/Constants.pm @@ -13,6 +13,7 @@ # The Original Code is the Bugzilla Bug Tracking System. # # Contributor(s): Marc Schumann <wurblzap@gmail.com> +# Max Kanat-Alexander <mkanat@bugzilla.org> package Bugzilla::WebService::Constants; @@ -20,11 +21,49 @@ use strict; use base qw(Exporter); @Bugzilla::WebService::Constants::EXPORT = qw( + WS_ERROR_CODE + ERROR_UNKNOWN_FATAL + ERROR_UNKNOWN_TRANSIENT + ERROR_AUTH_NODATA ERROR_UNIMPLEMENTED - ERROR_GENERAL ); +# This maps the error names in global/*-error.html.tmpl to numbers. +# Generally, transient errors should have a number above 0, and +# fatal errors should have a number below 0. +# +# This hash should generally contain any error that could be thrown +# by the WebService interface. If it's extremely unlikely that the +# error could be thrown (like some CodeErrors), it doesn't have to +# be listed here. +# +# "Transient" means "If you resubmit that request with different data, +# it may work." +# +# "Fatal" means, "There's something wrong with Bugzilla, probably +# something an administrator would have to fix." +# +# NOTE: Numbers must never be recycled. If you remove a number, leave a +# comment that it was retired. Also, if an error changes its name, you'll +# have to fix it here. +use constant WS_ERROR_CODE => { + # Bug errors usually occupy the 100-200 range. + invalid_bug_id_or_alias => 100, + invalid_bug_id_non_existent => 101, + bug_access_denied => 102, + + # Authentication errors are usually 300-400. + invalid_username_or_password => 300, + account_disabled => 301, + auth_invalid_email => 302, + extern_id_conflict => -303, +}; + +# These are the fallback defaults for errors not in ERROR_CODE. +use constant ERROR_UNKNOWN_FATAL => -32000; +use constant ERROR_UNKNOWN_TRANSIENT => 32000; + use constant ERROR_AUTH_NODATA => 410; use constant ERROR_UNIMPLEMENTED => 910; use constant ERROR_GENERAL => 999; diff --git a/t/012throwables.t b/t/012throwables.t index 8bc749686..b846ab907 100644 --- a/t/012throwables.t +++ b/t/012throwables.t @@ -19,6 +19,7 @@ # Rights Reserved. # # Contributor(s): Dennis Melentyev <dennis.melentyev@infopulse.com.ua> +# Max Kanat-Alexander <mkanat@bugzilla.org> @@ -30,6 +31,8 @@ use strict; use lib 't'; +use Bugzilla::WebService::Constants; + use File::Spec; use Support::Files; use Support::Templates; @@ -63,8 +66,8 @@ foreach my $include_path (@include_paths) { } } -# Count the tests -my $tests = (scalar keys %test_modules) + (scalar keys %test_templates); +# Count the tests. The +1 is for checking the WS_ERROR_CODE errors. +my $tests = (scalar keys %test_modules) + (scalar keys %test_templates) + 1; exit 0 if !$tests; # Set requested tests counter. @@ -162,11 +165,27 @@ foreach my $errtype (keys %Errors) { } } +# And make sure that everything defined in WS_ERROR_CODE +# is actually a valid error. +foreach my $err_name (keys %{WS_ERROR_CODE()}) { + if (!defined $Errors{'code'}{$err_name} + && !defined $Errors{'user'}{$err_name}) + { + Register(\%test_modules, 'WS_ERROR_CODE', + "Error tag '$err_name' is used in WS_ERROR_CODE in" + . " Bugzilla/WebService/Constants.pm" + . " but not defined in any template, and not used in any code."); + } +} + # Now report modules results foreach my $file (sort keys %test_modules) { Report($file, @{$test_modules{$file}}); } +# And report WS_ERROR_CODE results +Report('WS_ERROR_CODE', @{$test_modules{'WS_ERROR_CODE'}}); + # Now report templates results foreach my $file (sort keys %test_templates) { Report($file, @{$test_templates{$file}}); |