Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

socket.error exceptions not subclass of StandardError #44892

Closed
nagle mannequin opened this issue Apr 24, 2007 · 7 comments
Closed

socket.error exceptions not subclass of StandardError #44892

nagle mannequin opened this issue Apr 24, 2007 · 7 comments
Assignees
Labels
stdlib Python modules in the Lib dir

Comments

@nagle
Copy link
Mannequin

nagle mannequin commented Apr 24, 2007

BPO 1706815
Nosy @gpshead
Files
  • python-bug-1706815.diff: patch implementing 1,2,3,5,6 and maybe 4
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/gpshead'
    closed_at = <Date 2007-09-09.23:38:26.046>
    created_at = <Date 2007-04-24.18:09:54.000>
    labels = ['library']
    title = 'socket.error exceptions not subclass of StandardError'
    updated_at = <Date 2007-09-09.23:38:26.044>
    user = 'https://bugs.python.org/nagle'

    bugs.python.org fields:

    activity = <Date 2007-09-09.23:38:26.044>
    actor = 'gregory.p.smith'
    assignee = 'gregory.p.smith'
    closed = True
    closed_date = <Date 2007-09-09.23:38:26.046>
    closer = 'gregory.p.smith'
    components = ['Library (Lib)']
    creation = <Date 2007-04-24.18:09:54.000>
    creator = 'nagle'
    dependencies = []
    files = ['2359']
    hgrepos = []
    issue_num = 1706815
    keywords = []
    message_count = 7.0
    messages = ['31874', '31875', '31876', '31877', '55301', '55302', '55770']
    nosy_count = 3.0
    nosy_names = ['gregory.p.smith', 'janssen', 'nagle']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue1706815'
    versions = []

    @nagle
    Copy link
    Mannequin Author

    nagle mannequin commented Apr 24, 2007

    The "socket.error" exception is a subclass of Exception, but not of StandardError. It needs to be placed properly in the exception hierarchy, presumably somewhere under IOError.

    Socket errors have some known problems. See also:

    [ 805194 ] Inappropriate error received using socket timeout
    [ 1019808 ] wrong socket error returned
    [ 1571878 ] Improvements to socket module exceptions
    [ 708927 ] socket timeouts produce wrong errors in win32

    Just figuring out what exceptions can be raised from the socket module is tough. I've seen exceptions derived from "socket.error", exceptions from IOError, and exceptions from the SSL layer, which patches the
    sockets module when loaded. These are non-bug exceptions; that is, the problem is out in the network, external to the program.

    Some are retryable, some indicate that a different approach (different port, different protocol) should be tried, and some mean that some named resource doesn't exist. Programs need to make those distinctions reliably.

    The most important distinction with sockets is "external network problem" vs. "local program program". To resolve this, I suggest a "NetworkException" in the exception hierarchy, with all the things that can go wrong due to conditions external to the local machine under that exception.

    I'd suggest the following:

    1. Add "NetworkError" under "IOError" in the exception hierarchy.

    2. Put the existing "socket.error" under "NetworkError". Since "socket.error" needs to be reparented anyway (it's currently a direct descendant of "Exception") this provides a good place for it.

    3. Find any places where the socket module can raise IOError or OSError due to an external network condition, and make them raise something under NetworkError instead. Code that catches IOError will still work.

    4. Put all errors in the various SSL modules (SSLError, etc.) which can be raised due to external network conditions under "NetworkError"

    5. Move "urllib2.URLError", which is currently under IOError, down a level under NetworkError.

    6. Move the misc. errors from "urllib", like "ContentTooShortError", which are currently under IOError, down a level under NetworkError.

    7. URL translation errors from the IDNA (Unicode URL encoding) module probably should raise an error similar to that for an incorrect URL, rather than raising a UnicodeError.

    Then, programs that catch NetworkError could be sure of catching all network trouble conditions, but not local code bugs.

    With these changes, any exception that's being caught now will still be caught.

    I'd suggest doing 1) above immediately, since that's a clear bug, but the others need to be discussed.

    @nagle nagle mannequin assigned gpshead Apr 24, 2007
    @nagle nagle mannequin added the stdlib Python modules in the Lib dir label Apr 24, 2007
    @nagle nagle mannequin assigned gpshead Apr 24, 2007
    @nagle nagle mannequin added the stdlib Python modules in the Lib dir label Apr 24, 2007
    @nagle
    Copy link
    Mannequin Author

    nagle mannequin commented Apr 24, 2007

    (See also PEP-352).

    @gpshead
    Copy link
    Member

    gpshead commented Jul 2, 2007

    agreed! the above suggestions sound good.

    for number (3) if there are any places that raise OSError, that could lead to code upgrade headaches as the new NetworkError would not be a subclass of OSError. imho thats fine but others may disagree.

    i am looking at implementing the immediate (1) and (2) as a starting point.

    @gpshead
    Copy link
    Member

    gpshead commented Jul 4, 2007

    heres a patch against 2.6a0 svn HEAD implementing items 1 thru 6 (attached). I didn't look deeply to see if 4 is really implemented or not but the SSL errors were derived from socket.error so if they all use that as their base then they should be covered.

    test suites pass.
    File Added: python-bug-1706815.diff

    @janssen
    Copy link
    Mannequin

    janssen mannequin commented Aug 26, 2007

    It's not clear to me that having the SSL errors inherit from socket.error
    is a good idea. Many of them have nothing to do with the socket, but are
    errors in choice of cipher, certificate validation, etc.

    @gps
    Copy link
    Mannequin

    gps mannequin commented Aug 26, 2007

    Does the existing python SSL implementation allow it to be used over
    something other than a socket? If so then yes that makes sense, but
    otherwise its best to leave its inheritance from socket.error so that
    code that works when handed a regular socket can work over an SSL
    socket without knowing the difference.

    fwiw, regarding this bug the last comment I heard from guido on the
    python-dev list was that socket.error should at least be a subclass of
    EnvironmentError. I'm still a fan of having it a subclass of IOError
    myself for similar reason as above (things already written to use a
    file object as a stream could use a socket object and still handle
    errors properly; most code check for IOError rather than
    EnvironmentError if for no reason other than IOError is easier to type
    and easier to read and understand what it means)

    On 8/25/07, Bill Janssen <report@bugs.python.org> wrote:

    Bill Janssen added the comment:

    It's not clear to me that having the SSL errors inherit from socket.error
    is a good idea. Many of them have nothing to do with the socket, but are
    errors in choice of cipher, certificate validation, etc.

    ----------
    nosy: +janssen


    Tracker <report@bugs.python.org>
    <http://bugs.python.org/issue1706815\>


    @gpshead
    Copy link
    Member

    gpshead commented Sep 9, 2007

    socket.error now inherits from IOError as of trunk r58067:

    Change socket.error to inherit from IOError rather than being a stand
    alone class. This addresses the primary concern in

    http://bugs.python.org/issue1706815

    python-dev discussion here:

    http://mail.python.org/pipermail/python-dev/2007-July/073749.html

    I chose IOError rather than EnvironmentError as the base class since
    socket objects are often used as transparent duck typed file objects
    in code already prepared to deal with IOError exceptions.

    also a minor fix:

    urllib2 - fix a couple places where IOError was raised rather than
    URLError.
    for better or worse, URLError already inherits from IOError so
    this won't break any existing code.

    test_urllib2net - replace bad ftp urls.

    @gpshead gpshead closed this as completed Sep 9, 2007
    @gpshead gpshead closed this as completed Sep 9, 2007
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant