| msg101605 - (view) |
Author: Eric V. Smith (eric.smith) *  |
Date: 2010-03-23 22:12 |
| Sean Reifschneider proposed [1] adding the ability to log an exception using the syslog module. My proposed implementation is along the lines of: def logexceptions(chain=True): import sys import traceback import syslog # Should we chain to the existing sys.excepthook? current_hook = sys.excepthook if chain else None def syslog_exception(etype, evalue, etb): if current_hook: current_hook(etype, evalue, etb) # The result of traceback.format_exception might contain # embedded newlines, so we have the nested loops. for line in traceback.format_exception(etype, evalue, etb): for line in line.rstrip().split('\n'): syslog.syslog(line) sys.excepthook = syslog_exception Although it would need to be written in C to work in the existing syslog module, and of course it would call syslog.syslog directly. [1] http://mail.python.org/pipermail/python-ideas/2010-March/006927.html |
|
|
| msg101668 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2010-03-25 01:58 |
| syslog_exception() should be declared outside logexceptions(), so it's possible to call it directly. Example: try: ... except Exception: syslog_exception(*sys.exc_info()) |
|
|
| msg101681 - (view) |
Author: Eric V. Smith (eric.smith) * |
Date: 2010-03-25 09:17 |
| Good point. So that makes the implementation more like: import traceback import syslog import sys def syslog_exception(etype, evalue, etb): # The result of traceback.format_exception might contain # embedded newlines, so we have the nested loops. for line in traceback.format_exception(etype, evalue, etb): for line in line.rstrip().split('\n'): syslog.syslog(line) def logexceptions(chain=True): # Should we chain to the existing sys.excepthook? current_hook = sys.excepthook if chain else None def hook(etype, evalue, etb): if current_hook: current_hook(etype, evalue, etb) syslog_exception(etype, evalue, etb) sys.excepthook = hook We could then make syslog_exception take a tuple instead of the 3 values. I'm not sure which is more convenient, or more widely used in other APIs. Once it's moved into syslog, maybe syslog_exception named syslog.log_exception. |
|
|
| msg101682 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2010-03-25 09:26 |
| > Once it's moved into syslog, maybe syslog_exception named syslog.log_exception. In that case, how would be called the second function? Can write a patch with an unit test? |
|
|
| msg101683 - (view) |
Author: Eric V. Smith (eric.smith) * |
Date: 2010-03-25 09:30 |
| >> Once it's moved into syslog, maybe syslog_exception named syslog.log_exception. That's "should be named", of course. Although on second thought maybe syslog.syslog_exception really is the right name, to mirror syslog.syslog. > In that case, how would be called the second function? Can write a patch with an unit test? I'm not sure which second function you mean. As far as the implementation, Sean's working on it, although I'm willing to help. |
|
|
| msg101685 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2010-03-25 09:35 |
| > I'm not sure which second function you mean. "logexceptions" which replaces sys.excepthook. "log_exception" and "log_exceptions" are very close. cgitb has a function "enable" which sets sys.excepthook. |
|
|
| msg101686 - (view) |
Author: Eric V. Smith (eric.smith) * |
Date: 2010-03-25 09:41 |
| Ah, I see. Yes, logexceptions needs a better name. Maybe enable_exception_logging. |
|
|
| msg101687 - (view) |
Author: Eric V. Smith (eric.smith) * |
Date: 2010-03-25 09:56 |
| Here's a version that would be more analogous to what the C implementation would look like. It uses a class instead of a closure to capture the "chain" value. The 2 exposed functions syslog_exception and enable_exception_logging are the new APIs in this proposal, which would be written in C as part of the syslog module. The class is a hidden implementation detail. ######################## import traceback import syslog import sys def syslog_exception(etype, evalue, etb): # The result of traceback.format_exception might contain # embedded newlines, so we have the nested loops. for line in traceback.format_exception(etype, evalue, etb): for line in line.rstrip().split('\n'): syslog.syslog(line) class _Hook(object): def __init__(self, chain): # Should we chain to the existing sys.excepthook? self._current_hook = sys.excepthook if chain else None def _hook(self, etype, evalue, etb): if self._current_hook: self._current_hook(etype, evalue, etb) syslog_exception(etype, evalue, etb) def enable_exception_logging(chain=True): sys.excepthook = _Hook(chain)._hook |
|
|
| msg104211 - (view) |
Author: Sean Reifschneider (jafo) * |
Date: 2010-04-26 10:38 |
| I believe I have the first function implemented. See the attached patch for that code and feel free to review. |
|
|
| msg104693 - (view) |
Author: Sean Reifschneider (jafo) * |
Date: 2010-05-01 06:14 |
| I have completed the exception handling code as prototyped in . Please review. |
|
|
| msg104746 - (view) |
Author: Sean Reifschneider (jafo) * |
Date: 2010-05-01 20:19 |
| Jack Diederich commented: I don't have my tracker login on this computer so I'll post here. I'd +1 on making the module python with just the core functionality imported from C (it releases the GIL when doing IO). Then you could replace the few hundred lines of C with just the few lines of python from the prototype. That said... The parens in "return (NULL)" are extra and against PEP 7 (though there are already a bunch in syslogmodule.c). You need to NULL check the saved_hook in newhookobject() before INCREF'ing it. Should the saved hook be called after the syslog call? It might do anything. The patch needs unit tests. |
|
|
| msg104754 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-05-01 22:04 |
| Rewriting another implementation of traceback formatting in C is bad. Just import the "traceback" module and call the desired function in that module. |
|
|
| msg104755 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-05-01 22:05 |
| Oh, well, sorry. That's what you are already doing. Forget me :) |
|
|
| msg104756 - (view) |
Author: Sean Reifschneider (jafo) * |
Date: 2010-05-01 22:40 |
| Thanks for the review Jack. I was very tempted to split it into C and Python components, but I decided against it because it's so close to the 2.7 release. I think it would be best to defer that for the Python 3 release, because of potential packaging issues. I'm open to discussion on that though. I've changed all the return(NULL)s in the package. NULL check on saved_hook is done. I also had the same thought about the saved_hook/syslog ordering, so I've changed it. I've added soe unit tests. I tried getting fancy and testing the exception handling, but had to fork to do it and then unittests were still catching the exception, so I just left it the minimal set of tests I put in there. Thanks. |
|
|
| msg104757 - (view) |
Author: Brian Curtin (brian.curtin) * |
Date: 2010-05-01 23:40 |
| The test file should use test_support.import_module("syslog") instead of the try/except, which would then make the skip decorators unnecessary. |
|
|
| msg104759 - (view) |
Author: Sean Reifschneider (jafo) * |
Date: 2010-05-02 00:41 |
| Changed to use import_module. |
|
|
| msg104761 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2010-05-02 01:15 |
| You shouln't use fail*. They're deprecated. |
|
|
| msg104762 - (view) |
Author: Sean Reifschneider (jafo) * |
Date: 2010-05-02 01:40 |
| Switched to using assertTrue/assertFalse. |
|
|
| msg105181 - (view) |
Author: Sean Reifschneider (jafo) * |
Date: 2010-05-07 04:28 |
| This won't go into 2.7.0. I will do a patch for 3.x, and wait for 2.7.0 to see if it can go into 2.7.1. |
|
|
| msg221669 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2014-06-27 00:19 |
| @Sean is this something you can pick up again, it seems a shame to let your past efforts gather dust? |
|
|