Last Comment Bug 277073 - Make whining trap errors thrown by Search.pm
: Make whining trap errors thrown by Search.pm
Status: RESOLVED FIXED
[4.0.3 only; 4.2 and 5.0 fixed by bug...
:
Product: Bugzilla
Classification: Server Software
Component: Whining (show other bugs)
: 2.19.1
: All All
: -- normal (vote)
: Bugzilla 4.0
Assigned To: Frédéric Buclin
: default-qa
Mentors:
Depends on: 257344
Blocks:
  Show dependency treegraph
 
Reported: 2005-01-04 17:19 PST by Joel Peshkin
Modified: 2011-11-27 15:01 PST (History)
2 users (show)
LpSolit: approval4.0+
LpSolit: blocking4.0.3+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1 (1.90 KB, patch)
2005-11-23 15:52 PST, A. Karl Kornel
no flags Details | Diff | Review
Patch v1.1 (2.28 KB, patch)
2005-12-09 11:55 PST, A. Karl Kornel
goobix: review-
Details | Diff | Review
patch, v2 (1.56 KB, patch)
2011-11-25 18:27 PST, Frédéric Buclin
wicked: review+
Details | Diff | Review

Description Joel Peshkin 2005-01-04 17:19:35 PST
Now that bug 257344 has landed, take advantage of it.
Comment 1 A. Karl Kornel 2005-09-10 13:02:25 PDT
Well, look at this...

I was looking for a way to trap Search.pm errors, and I find this!  I can take this as soon as bug 304989 
lands.
Comment 2 Erik Stambaugh 2005-09-21 17:39:11 PDT
If you'd like.  I can still finish it if your queue is too full, though.

I'm working my way back through all of the neglected whining bugs.
Comment 3 A. Karl Kornel 2005-09-21 19:01:34 PDT
I'd like to take this, as (In reply to comment #2)
> If you'd like.  I can still finish it if your queue is too full, though.

Oh, that's not a problem!  I don't want to do anything with this right now because it will probably bitrot 
my patches to bug 304989 (which is why I marked bug 304989 as blocking it).
Comment 4 A. Karl Kornel 2005-10-31 06:24:05 PST
I'm pulling off the dependency on bug 304989 so that maybe this can be fixed sometime soon.
Comment 5 A. Karl Kornel 2005-11-23 15:52:09 PST
Created attachment 204083 [details] [diff] [review]
Patch v1

Well, since there are going to be no major whine.pl changes any time soon, we might as well fix this for now.

This patch wraps the Bugzilla::CGI creation, Bugzilla::Search creation, and search SQL execution inside an eval block.  A death inside the block causes a message to be printed, including the the title of the query, the login of the author, and the error message.  Execution then continues with the next query.
Comment 6 Erik Stambaugh 2005-11-29 16:30:05 PST
Comment on attachment 204083 [details] [diff] [review]
Patch v1

>Index: whine.pl
>+    # Get, run, and process one query.  We may have trouble here, so mark the
>+    # loop for easy `next`ing.
>+    QUERY:
>     for my $query_id (keys %{$queries}) {
>         my $thisquery = $queries->{$query_id};
>         next unless $thisquery->{'name'};   # named query is blank
          ~~~~

This is kind of a style nit, but since we have the block marked now, I'd like to use the label in all of its next statements to avoid confusion by people reading it in the future.

>+        if ($@) {
>+            print 'WARNING: Error constructing and/or executing query "', 
>+                  $thisquery->{'title'}, '" of user ',
>+                  $args->{'author'}->login, ": $@\n";
>+            next QUERY;
>+        }

Should that go to STDERR?  Willing to listen to reason if it shouldn't.
Comment 7 A. Karl Kornel 2005-12-09 11:55:13 PST
Created attachment 205407 [details] [diff] [review]
Patch v1.1

Modification of attachment 204083 [details] [diff] [review] with respect to comment 6:

><<<snip>>>
> This is kind of a style nit, but since we have the block marked now, I'd like
> to use the label in all of its next statements to avoid confusion by people
> reading it in the future.

Fine with me.

><<<snip>>>
> 
> Should that go to STDERR?  Willing to listen to reason if it shouldn't.

Also fine with me.
Comment 8 A. Karl Kornel 2005-12-27 14:47:47 PST
erik: Ping?
Comment 9 Vlad Dascalu 2006-06-18 11:11:41 PDT
Comment on attachment 205407 [details] [diff] [review]
Patch v1.1

Sorry for the delay.

I tried to take a look at this today, but the patch no longer applies cleanly to the trunk. Nowadays the queries are in an array as opposed to a hash.

So the patch could be updated with minimal effort to apply again to the trunk.
Comment 10 Frédéric Buclin 2006-10-19 12:11:42 PDT
This bug is retargetted to Bugzilla 3.2 for one of the following reasons:

- it has no assignee (except the default one)
- we don't expect someone to fix it in the next two weeks (i.e. before we freeze the trunk to prepare Bugzilla 3.0 RC1)
- it's not a blocker

If you are working on this bug and you think you will be able to submit a patch in the next two weeks, retarget this bug to 3.0.

If this bug is something you would like to see implemented in 3.0 but you are not a developer or you don't think you will be able to fix this bug yourself in the next two weeks, please *do not* retarget this bug.

If you think this bug should absolutely be fixed before we release 3.0, either ask on IRC or use the "blocking3.0 flag".
Comment 11 Frédéric Buclin 2011-11-25 16:20:33 PST
This problem has been fixed in 4.2 and 5.0 by bug 255606. It needs to be backported to 4.0.3.
Comment 12 Frédéric Buclin 2011-11-25 18:27:20 PST
Created attachment 577030 [details] [diff] [review]
patch, v2

Backported from 4.2/trunk, see bug 255606. Tested, works.
Comment 13 Teemu Mannermaa (:wicked) 2011-11-27 05:54:04 PST
Comment on attachment 577030 [details] [diff] [review]
patch, v2

>=== modified file 'whine.pl'
>@@ -444,11 +444,17 @@
>+            print get_text('whine_query_failed', { query_name => $thisquery->{'name'},

Nit: This probably should have gone to STDERR like the other error message from the script.
Comment 14 Frédéric Buclin 2011-11-27 09:49:03 PST
(In reply to Teemu Mannermaa (:wicked) from comment #13)
> Nit: This probably should have gone to STDERR like the other error message
> from the script.

Ah, good idea. I will fix that on checkin. Thanks! :)
Comment 15 Frédéric Buclin 2011-11-27 15:01:28 PST
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/
modified whine.pl
modified template/en/default/global/messages.html.tmpl
Committed revision 7658.

Note You need to log in before you can comment on or make changes to this bug.