Bugs item #411881, was opened at 2001-03-28 04:58
Message generated for change (Comment added) made by sf-robot
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=411881&group_id=5470
Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Python Library
Group: None
>Status: Closed
Resolution: Fixed
Priority: 2
Private: No
Submitted By: Itamar Shtull-Trauring (itamar)
Assigned to: Vinay Sajip (vsajip)
Summary: Use of "except:" in logging module
Initial Comment:
A large amount of modules in the standard library use
"except:" instead of specifying the exceptions to be
caught. In some cases this may be correct, but I think
in most cases this not true and this may cause
problems. Here's the list of modules, which I got by
doing:
grep "except:" *.py | cut -f 1 -d " " | sort | uniq
Bastion.py
CGIHTTPServer.py
Cookie.py
SocketServer.py
anydbm.py
asyncore.py
bdb.py
cgi.py
chunk.py
cmd.py
code.py
compileall.py
doctest.py
fileinput.py
formatter.py
getpass.py
htmllib.py
imaplib.py
inspect.py
locale.py
locale.py
mailcap.py
mhlib.py
mimetools.py
mimify.py
os.py
pdb.py
popen2.py
posixfile.py
pre.py
pstats.py
pty.py
pyclbr.py
pydoc.py
repr.py
rexec.py
rfc822.py
shelve.py
shutil.py
tempfile.py
threading.py
traceback.py
types.py
unittest.py
urllib.py
zipfile.py
----------------------------------------------------------------------
>Comment By: SourceForge Robot (sf-robot)
Date: 2007-01-22 19:20
Message:
Logged In: YES
user_id=1312539
Originator: NO
This Tracker item was closed automatically by the system. It was
previously set to a Pending status, and the original submitter
did not respond within 14 days (the time period specified by
the administrator of this Tracker).
----------------------------------------------------------------------
Comment By: Vinay Sajip (vsajip)
Date: 2007-01-08 10:55
Message:
Logged In: YES
user_id=308438
Originator: NO
The following changes have been checked into trunk:
logging.handlers: bare except clause removed from SMTPHandler.emit. Now,
only ImportError is trapped.
logging.handlers: bare except clause removed from
SocketHandler.createSocket. Now, only socket.error is trapped.
logging: bare except clause removed from LogRecord.__init__. Now, only
ValueError, TypeError and AttributeError are trapped.
I'm marking this as Pending; please submit a change if you think these
changes are insufficient. With the default setting of raiseExceptions, all
exceptions caused by programmer error should be re-thrown by logging.
----------------------------------------------------------------------
Comment By: Skip Montanaro (montanaro)
Date: 2006-12-22 04:52
Message:
Logged In: YES
user_id=44345
Originator: NO
Vinay,
In LogRecord.__init__ what exceptions do you expect to catch? Looking at
the code for basename and splitext in os.py it's pretty hard to see how
they would raise an exception unless they were passed something besides
string or unicode objects. I think all you are doing here is masking
programmer error. In StreamHandler.emit what might you get besides
ValueError (if self.stream is closed)?
I don't have time to go through each of the cases, but in general, it
seems like the set of possible exceptions that could be raised at any
given point in the code is generally pretty small. You should catch those
exceptions and let the other stuff go. They are generally going to be
programmer's errors and shouldn't be silently squashed.
Skip
----------------------------------------------------------------------
Comment By: Vinay Sajip (vsajip)
Date: 2006-12-21 23:42
Message:
Logged In: YES
user_id=308438
Originator: NO
The reason for the fair number of bare excepts in logging is this: in many
cases (e.g. long-running processes like Zope servers) users don't want
their application to change behaviour just because of some exception
thrown in logging. So, logging aims to be very quiet indeed and swallows
exceptions, except SystemExit and KeyboardInterrupt in certain
situations.
Also, logging is one of the modules which is (meant to be) 1.5.2
compatible, and string exceptions are not that uncommon in older code.
I've looked at bare excepts in logging and here's my summary on them:
logging/__init__.py:
====================
currentframe(): Backward compatibility only, sys._getframe is used where
available so currentframe() will only be called on rare occasions.
LogRecord.__init__(): There's a try/bare except around calls to
os.path.basename() and os.path.splitext(). I could add a raise clause for
SystemExit/KeyboardInterrupt.
StreamHandler.emit(): Reraises SystemExit and KeyboardInterrupt, and
otherwise calls handleError() which raises everything if raiseExceptions
is set (the default).
shutdown(): Normally only called at system exit, and will re-raise
everything if raiseExceptions is set (the default).
logging/handlers.py:
====================
BaseRotatingHandler.emit(): Reraises SystemExit and KeyboardInterrupt, and
otherwise calls handleError() which raises everything if raiseExceptions is
set (the default).
SocketHandler.createSocket(): I could add a raise clause for
SystemExit/KeyboardInterrupt.
SocketHandler.emit(): Reraises SystemExit and KeyboardInterrupt, and
otherwise calls handleError() which raises everything if raiseExceptions
is set (the default).
SysLogHandler.emit(): Reraises SystemExit and KeyboardInterrupt, and
otherwise calls handleError() which raises everything if raiseExceptions
is set (the default).
SMTPHandler.emit(): Should change bare except to ImportError for the
formatdate import.
Elsewhere, reraises SystemExit and KeyboardInterrupt, and otherwise calls
handleError() which raises everything if raiseExceptions is set (the
default).
NTEventLogHandler.emit(): Reraises SystemExit and KeyboardInterrupt, and
otherwise calls handleError() which raises everything if raiseExceptions
is set (the default).
HTTPHandler.emit(): Reraises SystemExit and KeyboardInterrupt, and
otherwise calls handleError() which raises everything if raiseExceptions
is set (the default).
logging/config.py:
====================
listen.ConfigStreamHandler.handle(): Reraises SystemExit and
KeyboardInterrupt, prints everything else and continues - seems OK for a
long-running thread.
What do you think?
----------------------------------------------------------------------
Comment By: A.M. Kuchling (akuchling)
Date: 2006-12-21 06:09
Message:
Logged In: YES
user_id=11375
Originator: NO
Raymond said (in 2003) most of the remaining except: statements looked
reasonable, so I'm changing this bug's summary to refer to the logging
module and reassigning to vsajip.
PEP 8 doesn't say anything about bare excepts; I'll bring this up on
python-dev.
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2003-12-13 03:21
Message:
Logged In: YES
user_id=80475
Hold-off on logging for a bit. Vinay Sajip has other
patches already
under review. I'll ask him to fix-up the bare excepts in
conjuction with those patches.
For the other modules, patches are welcome.
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2003-12-11 12:54
Message:
Logged In: YES
user_id=6380
You're right. The logging module uses more blank except:
clauses than I'm comfortable with. Anyone want to upload a
patch set?
----------------------------------------------------------------------
Comment By: Grant Monroe (gmonroe)
Date: 2003-12-11 12:50
Message:
Logged In: YES
user_id=929204
A good example of an incorrect use of a blanket "except:"
clause is in __init__.py in the logging module. The emit
method of the StreamHandler class should special case
KeyboardInterrupt. Something like this:
try:
....
except KeyboardInterrupt:
raise
except:
self.handleError(record)
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2003-09-01 19:47
Message:
Logged In: YES
user_id=80475
Some efforts were made to remove many bare excepts prior
to Py2.3a1. Briefly scanning those that remain, it looks like
many of them are appropriate or best left alone.
I recommend that this bug be closed unless someone sees
something specific that demands a change.
----------------------------------------------------------------------
Comment By: Brett Cannon (bcannon)
Date: 2003-05-16 16:30
Message:
Logged In: YES
user_id=357491
threading.py is clear. It's blanket exceptions are for printing debug
output
since exceptions in threads don't get passed back to the original frame
anyway.
----------------------------------------------------------------------
Comment By: Skip Montanaro (montanaro)
Date: 2002-08-13 20:15
Message:
Logged In: YES
user_id=44345
checked in fileinput.py (v 1.15) with three except:'s tightened up. The
comment in the code about IOError notwithstanding, I don't see how any
of the three situations would have caught anything other than OSError.
----------------------------------------------------------------------
Comment By: Skip Montanaro (montanaro)
Date: 2002-08-12 12:58
Message:
Logged In: YES
user_id=44345
Note that this particular item was expected to be an ongoing item, with
no obvious closure. Some of the bare excepts will have subtle
ramifications, and it's not always obvious what specific exceptions
should be caught. I've made a few changes to my local source tree
which I should check in.
Rather than opening new tracker items, I believe those with checkin
privileges should correct those flaws they identify and attach a comment
which will alert those monitoring the item. Those people without checkin
privileges should just attach a patch with a note.
Skip
----------------------------------------------------------------------
Comment By: Martin v. Löwis (loewis)
Date: 2002-08-12 00:22
Message:
Logged In: YES
user_id=21627
My proposal would be to track this under a different issue:
Terry, if you volunteer, please produce a new list of
offenders (perhaps in an attachment to the report so it can
be updated), and attach any fixes that you have to that report.
People with CVS write access can then apply those patches
and delete them from the report.
If you do so, please post the new issue number in this
report, so we have a link.
----------------------------------------------------------------------
Comment By: Terry J. Reedy (tjreedy)
Date: 2002-08-11 11:16
Message:
Logged In: YES
user_id=593130
Remove types.py from the list. As distributed with 2.2.1, it
has 5 'except xxxError:' statements but no offending bare
except:'s.
Skip (or anyone else): if/when you pursue this, I volunteer
to do occasional sleuthing and send reports with
suggestions and/or questions. Example: getpass.py has
one 'offense':
try:
fd = sys.stdin.fileno()
except:
return default_getpass(prompt)
According to lib doc 2.2.8 File Objects (as I interpret) fileno
() should either work without exception or *not* be
implemented. Suggestion: insert AttributeError . Question:
do we protect against pseudofile objects that ignore doc
and have fake .fileno() that raises NotImplementedError or
whatever?
----------------------------------------------------------------------
Comment By: Skip Montanaro (montanaro)
Date: 2002-03-22 22:02
Message:
Logged In: YES
user_id=44345
as partial fix, checked in changes for the following
modules:
mimetools.py (1.24)
popen2.py (1.23)
quopripy (1.19)
CGIHTTPServer.py (1.22)
----------------------------------------------------------------------
Comment By: Skip Montanaro (montanaro)
Date: 2002-03-20 13:24
Message:
Logged In: YES
user_id=44345
Here is a context diff with proposed changes for the
following modules: CGIHTTPServer, cgi, cmd, code,
fileinput, httplib, inspect, locale, mimetools, popen2,
quopri
----------------------------------------------------------------------
Comment By: Martin v. Löwis (loewis)
Date: 2001-08-11 08:06
Message:
Logged In: YES
user_id=21627
Fixed urllib in 1.131 and types in 1.19.
----------------------------------------------------------------------
Comment By: Fred L. Drake, Jr. (fdrake)
Date: 2001-07-04 00:11
Message:
Logged In: YES
user_id=3066
Fixed modules mhlib and rfc822 (SF is having a problem
generating the checkin emails, though).
----------------------------------------------------------------------
Comment By: Fred L. Drake, Jr. (fdrake)
Date: 2001-05-11 12:40
Message:
Logged In: YES
user_id=3066
OK, I've fixed up a few more modules:
anydbm
chunk
formatter
htmllib
mailcap
pre
pty
I made one change to asyncore as well, but other bare except
clauses remain there; I'm not sufficiently familiar with
that code to just go digging into those.
I also fixed an infraction in pstats, but left others for now.
----------------------------------------------------------------------
Comment By: Tim Peters (tim_one)
Date: 2001-04-23 01:14
Message:
Logged In: YES
user_id=31435
Ping's intent is that pydoc work under versions of Python
as early as 1.5.2, so that sys._getframe is off-limits in
pydoc and its supporting code (like inspect.py).
----------------------------------------------------------------------
Comment By: Martin v. Löwis (loewis)
Date: 2001-04-23 00:32
Message:
Logged In: YES
user_id=21627
For inspect.py, why is it necessary to keep the old code at
all? My proposal: remove currentframe altogether, and do
currentframe = sys._getframe
unconditionally.
----------------------------------------------------------------------
Comment By: Itamar Shtull-Trauring (itamar)
Date: 2001-04-22 07:52
Message:
Logged In: YES
user_id=32065
I submitted a 4th patch. I'm starting to run out of easy
cases...
----------------------------------------------------------------------
Comment By: Skip Montanaro (montanaro)
Date: 2001-04-19 02:15
Message:
Logged In: YES
user_id=44345
I believe the following patch is correct for the try/except
in inspect.currentframe. Note that it fixes two problems.
One, it avoids a bare except. Two, it gets rid of a string
argument to the raise statement (string exceptions are now
deprecated, right?).
*** /tmp/skip/inspect.py Thu Apr 19 04:13:36 2001
--- /tmp/skip/inspect.py.~1.16~ Thu Apr 19 04:13:36 2001
***************
*** 643,650 ****
def currentframe():
"""Return the frame object for the caller's stack
frame."""
try:
! 1/0
! except ZeroDivisionError:
return sys.exc_traceback.tb_frame.f_back
if hasattr(sys, '_getframe'): currentframe = sys._getframe
--- 643,650 ----
def currentframe():
"""Return the frame object for the caller's stack
frame."""
try:
! raise 'catch me'
! except:
return sys.exc_traceback.tb_frame.f_back
if hasattr(sys, '_getframe'): currentframe = sys._getframe
----------------------------------------------------------------------
Comment By: Itamar Shtull-Trauring (itamar)
Date: 2001-04-17 08:27
Message:
Logged In: YES
user_id=32065
inspect.py uses sys_getframe if it's there, the other code
is for backwards compatibility.
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2001-04-11 10:24
Message:
Logged In: YES
user_id=6380
Actually, inspect.py should use sys._getframe()!
And yes, KeyboardError is definitely one of the reasons why
this is such a bad idiom...
----------------------------------------------------------------------
Comment By: Walter Dörwald (doerwalter)
Date: 2001-04-11 10:15
Message:
Logged In: YES
user_id=89016
> Can you identify modules where catching everything
> is incorrect
If "everything" includes KeyboardInterrupt, it's
definitely incorrect, even in inspect.py's simple
try:
raise 'catch me'
except:
return sys.exc_traceback.tb_frame.f_back
which should probably be:
try:
raise 'catch me'
except KeyboardInterrupt:
raise
except:
return sys.exc_traceback.tb_frame.f_back
----------------------------------------------------------------------
Comment By: Walter Dörwald (doerwalter)
Date: 2001-04-11 10:13
Message:
Logged In: YES
user_id=89016
> Can you identify modules where catching everything
> is incorrect
If "everything" includes KeyboardInterrupt, it's
definitely incorrect, even in inspect.py's simple
try:
raise 'catch me'
except:
return sys.exc_traceback.tb_frame.f_back
which should probably be:
try:
raise 'catch me'
except KeyboardInterrupt:
raise
except:
return sys.exc_traceback.tb_frame.f_back
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2001-04-10 08:45
Message:
Logged In: YES
user_id=6380
I've applied the three patches you supplied.
I agree with Martin that to do this right we'll have to
tread carefully. But please go on!
(No way more of this will find its way into 2.1 though.)
----------------------------------------------------------------------
Comment By: Itamar Shtull-Trauring (itamar)
Date: 2001-03-30 02:54
Message:
Logged In: YES
user_id=32065
inspect.py should be removed from this list, the use is
correct.
In general, I just submitted this bug so that when people
are editing a module they'll notice these things, since in
some cases only someone who knows the code very well can
know if the "expect:" is needed or not.
----------------------------------------------------------------------
Comment By: Martin v. Löwis (loewis)
Date: 2001-03-29 22:59
Message:
Logged In: YES
user_id=21627
Can you identify modules where catching everything is
incorrect, and propose changes to correct them. This should
be done one-by-one, with careful analysis in each case, and
may take well months or years to complete.
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=411881&group_id=5470
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
