I had a detector that raised an IOError exception due to a file it was 
trying to open that had the wrong ownership. The apache ssl_error_log 
showed this for the issue I was trying to edit:

Premature end of script headers: roundup.cgi, referer: 
https://tracker.labmed.uw.edu/systems/issue129

The problem was that client.py's Client.inner_main method has an except 
clause for IOError that simply was 'pass' causing no HTTP headers, hence 
the apache error.

I have a fix that works by generating the normal 'An error has occurred' 
message to the user, and an stack trace email to the roundup admin.

I'm not happy about the way it has to get access to the 
send_error_to_admin method. Hopefully you have a better way.

I came up with a better solution that I'm happy with. In the method 
fireReactors is a new try/except Exception block to catch detector 
execution errors. The except block creates a DetectorError with the 
error details (a new exception in roundup/cgi/exceptions.py). The big 
try/except block in Client.py's inner_main has a new except block to 
catch DetectorError and either write HTML to the user and send the 
traceback to the roundup admin, or write the traceback to the user.

This should be done with the method fireAuditors too. I'll work on that 
next (not much to do there) and post the hg diff file for the 3 files 
that were changed if that is OK with you.

In message <1387389078.18.0.615889458062.issue2550826@psf.upfronthosting.co.za>
 <1387389078.18.0.615889458062.issue2550826@psf.upfronthosting.co.za>,
Tom Ekberg writes:
>I came up with a better solution that I'm happy with. In the method 
>fireReactors is a new try/except Exception block to catch detector 
>execution errors. [...]
>This should be done with the method fireAuditors too. I'll work on that 
>next (not much to do there) and post the hg diff file for the 3 files 
>that were changed if that is OK with you.

Yeah that sounds great. Thanks for your work on doing the more
detailed fix.

				-- rouilj
John Rouillard
===========================================================================
My employers don't acknowledge my existence much less my opinions.

I've finished testing handling errors in both types of detectors: reactors 
and auditors. Both seem to work fine.

The detector-error-catch.txt file contains changes to 3 files and was 
generated with this command:

$ hg diff roundup/cgi/client.py roundup/cgi/exceptions.py 
roundup/hyperdb.py > /tmp/detector-error-catch.txt

Hopefully it isn't too difficult to generate a patch file from this. Here 
is a summary of the changes. Comments welcome.

roundup/cgi/client.py
  Catches the DetectorError exception and sends email and generates HTML.
roundup/cgi/exceptions.py
  Defines the DetectorError exception class.
roundup/hyperdb.py
  Catches Exceptions from auditors or reactors and throws a DetectorError 
exception with the error details.

With my previous fix, the roundup-admin program breaks with the 
following stack trace:

$ /usr/local/bin/roundup-admin
Traceback (most recent call last):
  File "/usr/local/bin/roundup-admin", line 2, in <module>
    from roundup.scripts.roundup_admin import run
  File "/usr/local/lib/python2.7/site-
packages/roundup/scripts/roundup_admin.py", line 40, in <module>
    from roundup.admin import AdminTool
  File "/usr/local/lib/python2.7/site-packages/roundup/admin.py", line 
26, in <module>
    from roundup import date, hyperdb, roundupdb, init, password, token
  File "/usr/local/lib/python2.7/site-packages/roundup/hyperdb.py", line 
28, in <module>
    import roundup.cgi.cgitb
  File "/usr/local/lib/python2.7/site-packages/roundup/cgi/cgitb.py", 
line 12, in <module>
    from roundup.cgi import templating, TranslationService
  File "/usr/local/lib/python2.7/site-
packages/roundup/cgi/templating.py", line 26, in <module>
    from roundup import hyperdb, date, support
ImportError: cannot import name hyperdb

It appears this is due to the cgitb import I added in hyperdb. There 
appears to be a loop importing hyperdb. According to Google 
'"ImportError: cannot import name"', first hit, this is a bad idea.

I removed the use of cgitb and replaced the call to that module's html() 
function to just put a <br> between each line of the stack trace, and 
change ' ' to '&nbsp;'. It's not as nice as cgitb.html() but roundup-
admin works again. I have attached a new diff file generated using the 
same command as before.

I had to modify the patch a little.
In roundup.cgi.exceptions added DetectorError the the import:

    from roundup.cgi.exceptions import (...}

This fixed an undefined global DetectorError at:

  except DetectorError as e:

later in the file.

I had to hand apply the import section for roundup/hyperdb.py.

Also I had to change the patch:

+            if not self.instance.config.WEB_DEBUG:
+                self.send_error_to_admin(e.subject, e.html, e.txt)
+                self.write_html(self._(error_message))

self._(error_message) is not defined. I replaced it with e.html which
is what is sent to the user when WEB_DEBUG is true (exposing the
traceback to the user) as well.

In the tracker's config.ini I tested with [web].debug set to true and
false and [mail].debug set to log.mail.txt.

I also installed test_detector.py in the tracker detectors directory.
I have attached test_detector.py. If the status of an issue changes it
raises an IOError during audit.  If the nosy list is not empty,
after the transaction, it raises an IOEerror from the reactor.
It passes all other update types through without error.

Running without the patch, I get a reset from server. With the patch I
get a traceback displayed to the user. If [web].debug is false, I also get
an email sent to the administrator.

I am going to commit this since it handles the error condition better.
However I am not positive that the error is being rendered to the user
when [web].debug is not set as the author expected.

Also I am not sure how to craft an automated test for this.
But if the attached detector is useful, that's fine.

Also I should mention I rand roundup_admin on the demo. Got tracebacks
for nosy list and status changes. Was able to make other changes
(assignedto) without error.

Committed: 65fef7858606

Re-opening because the original patch broke test cases that relied on
detectors being able to raise:

  ValueError
  pytz.UnknownTimeZoneError

I have a patch that fixes the test cases in the roundup trunk
[committed as 89d69a822e5c] but I am not happy with it. Emailed devel
list for ideas on how to better handle this.

I am considering changing the original patch, which captured all
Exceptions, to capturing: EnvironmentError (which include IOError and
OSError IIUC) and ArithmeticError and let the rest propagate up.

I wasn't able to find a canonical list of the exceptions that a
detector can raise that have to be propagated up into the parent, if I
could then the code could be structured to all these to pass up and
still capture all other exceptions.

Reclosing after committing 675b3b3d88f0.

Captures Environment and Arithmetic exceptions allowing other
exceptions: including Reject, ValueError ..., to be
properly handled by upper layers.