372537 - Improve CodeErrors: show the traceback (without function arguments)

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Max Kanat-Alexander]

Right now if Bugzilla::Object throws a CodeError, you have *no idea* where the actual problem is, since it doesn't tell you who the caller is. During a customization, I fixed this so that certain errors would give you more information about where you came from, and we should have that code upstream.

Comment 1

[Max Kanat-Alexander]

Attachment: v1

Okay, I just pulled this straight out of the NASA patch--I don't even know if it applies, but I think it should... :-)

Comment 2

[Max Kanat-Alexander]

Okay, I verified that it applies and compiles.

Comment 3

[Frédéric Buclin]

Comment on attachment 257182 [details] [diff] [review]
v1

>=== modified file 'Bugzilla/Util.pm'

> sub css_class_quote {
>     my ($toencode) = (@_);
>-    $toencode =~ s/ /_/g;
>+    $toencode =~ s/[ :\/]/_/g;

I don't see why this change is required for this fix.

string_caller() has no POD.



>=== modified file 'Bugzilla/Object.pm'

>         detaint_natural($id)
>           || ThrowCodeError('param_must_be_numeric',
>+                            {function => $class . '::_init',
>+                             caller   => caller_string(2) });

caller_string(2) won't return anything if you call |new Bugzilla::Foo()| from a CGI script and new() is not defined in the .pm module, such as |new Bugzilla::Product| called from editproducts.cgi.

IMO, you should display both caller_string(1) and caller_string(2), so that you get the complete path (caller_string(0) is useless as you would get Object->new in all cases). Except the problem above, your patch works fine.

Comment 4

[Max Kanat-Alexander]

Attachment: v2

Okay, so I had a better idea. For all CodeErrors, show the full traceback. I leave out method arguments from the traceback, so it should always be pretty safe. Also, thanks to the way that code-error.html.tmpl already works, this only affects ERROR_MODE_WEBPAGE, so WebService clients won't suddenly start getting Bugzilla tracebacks, which is good.

Comment 5

[Frédéric Buclin]

Comment on attachment 444359 [details] [diff] [review]
v2

Yeah, nice to have some more debug info. r=LpSolit

Note that this will not only affect Bugzilla::Object, but all calls to ThrowCodeError.

Comment 6

[Max Kanat-Alexander]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Error.pm
modified template/en/default/global/code-error.html.tmpl
Committed revision 7168.

Comment 7

[Max Kanat-Alexander]

Added to the release notes in bug 604256.