[Patches] [ python-Patches-1117398 ] cookielib LWPCookieJar and MozillaCookieJar exceptions

[SourceForge.net]

Patches item #1117398, was opened at 2005-02-06 17:39
Message generated for change (Comment added) made by jjlee
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1117398&group_id=5470

Category: Library (Lib)
Group: Python 2.4
Status: Open
Resolution: None
Priority: 5
Submitted By: John J Lee (jjlee)
Assigned to: Nobody/Anonymous (nobody)
Summary: cookielib LWPCookieJar and MozillaCookieJar exceptions

Initial Comment:
cookielib.LWPCookieJar and .MozillaCookieJar are
documented to raise cookielib.LoadError on attempt to
load an invalid cookies file, but do not.

I think this should be backported to the 2.4
maintenance branch.
Reason: I suspect more people will be bitten by the bug
than will be bitten by the patch, since cookies files
will rarely be invalid, so people are likely to have
written except statements based on the docs in this
case, rather than based on the actual exception that
currently occurs.


----------------------------------------------------------------------

>Comment By: John J Lee (jjlee)
Date: 2005-03-04 14:16

Message:
Logged In: YES 
user_id=261020

LoadError is not currently not used anywhere.

Without LoadError, how would one distinguish (for the
purpose of error handling) an error due to, on the one hand,
a missing file, insufficient permissions, etc. (IOError)
from an error due, on the other hand, to an invalid cookies
file (LoadError)?  This is already a problem with IOError,
of course, but I don't see why we should make it worse.


----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2005-03-03 11:03

Message:
Logged In: YES 
user_id=21627

Is it true that LoadError is currently not used anywhere? If
so, I think it would be best to eliminate this exception,
both from the code and the documentation. To support code
that might refer to LoadError already, you could make it a
synonym to IOError.

----------------------------------------------------------------------

Comment By: John J Lee (jjlee)
Date: 2005-02-11 18:57

Message:
Logged In: YES 
user_id=261020

I agree the backport (but not the trunk) should have
LoadError derive from IOError.

I'd be happy for comments to go in noting the change in all
places where LoadError is raised, if that's considered good
practice.

Any committers reading: should I submit an updated patch
with just those added comments?  Should I submit a patch for
the backport, with just that change plus the added (IOError)
for backwards-compat.?

(Jim: Besides the point for the matter at hand, but I should
point out that the "pretty-for-printing" cookielib docs and
the docstrings in cookielib.py form almost disjoint sets. 
Most of the documentation lives under Doc/lib, the stuff in
the module source is the more obscure detail.  And of
course, you have just demonstrated to yourself how not
reading the docs leaves you with an incomplete understanding
of the intent, which can be useful!  Not that I *ever* do
that, of course ;-)


----------------------------------------------------------------------

Comment By: Jim Jewett (jimjjewett)
Date: 2005-02-11 16:06

Message:
Logged In: YES 
user_id=764593

The first sentence was pointing out that many people 
(including me) never see the pretty-for-printing 
documentation, and even the html form isn't generally 
convenient when I'm actually programming.  So I rely on 
the introspection tools.  I see object names, signatures, 
and docstrings.  If I need more information, I look at the 
code, which raises IOError.

While I don't yet have code catching this particular error, I 
do write in a way that would break; it wouldn't have 
occurred to me to put both types of error in an except 
clause just in case it changed later.  (Well, unless the 
docstring or at least a comment in the code itself warned 
me to.)

So I would definately prefer that it remain (at least a 
subclass of) IOError for at least 2.4.x  I would also 
appreciate comments in the fixed 2.4 code if it is going to 
change for 2.5.


----------------------------------------------------------------------

Comment By: John J Lee (jjlee)
Date: 2005-02-10 22:25

Message:
Logged In: YES 
user_id=261020

Jim, I don't understand the first sentence in your comment.

Re a 2.4 backport that makes LoadError derive from IOError:
it makes me wince, but I can't think of an argument against it.

No, LoadError should not be a subclass of IOError in the
trunk, because the cases where LoadError is documented to be
raised do not involve failures of I/O, but rather invalid
data.  See the docs for IOError.  (FWIW, EnvironmentError
(IOError's base class) wouldn't be a suitable subclass
either: eg. what would we want with an .errno attribute?)


----------------------------------------------------------------------

Comment By: Jim Jewett (jimjjewett)
Date: 2005-02-08 16:58

Message:
Logged In: YES 
user_id=764593

I often look at the code in a second idle window rather 
than starting a web browser.  

Would it work to make LoadError a subclass of IOError, at 
least for the backport?  People who followed the docs will 
get a bugfix, but people who followed the code would get 
no breakage.

Should LoadError be a subclass of IOError even in the 
main trunk?

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1117398&group_id=5470