[pywin32-bugs] [ pywin32-Bugs-3003706 ] pywintypes.error should inherit from WindowsError

[SourceForge.net <no...@so...>]

Bugs item #3003706, was opened at 2010-05-18 22:48
Message generated for change (Comment added) made by 
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=551954&aid=3003706&group_id=78018

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: win32
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: gz ()
Assigned to: Nobody/Anonymous (nobody)
Summary: pywintypes.error should inherit from WindowsError

Initial Comment:
Exceptions thrown by system calls in Python functions throw an EnvironmentError subclass, ctypes.WinError creates a WindowsError, extension modules using windows system calls throw some kind of OSError, but pywin32 has its own exception type that can't be caught at the same time as any of these.

This means we've got an odd case in our code, where there are two nearly identical implementations of a class, one using ctypes and one using pywin32, but the generic error handling code treated them differently because pywintypes.error isn't an EnvironmentError, see <https://bugs.launchpad.net/bzr/+bug/572201>.

The current code in win32/src/PyWinTypesmodule.cpp is like:
  class error(Exception):
    def __init__(self, *args, **kw):
      self.winerror, self.funcname, self.strerror = args[:3]
      Exception.__init__(self, *args, **kw)

It could be changed to something along the lines of:

  class error(WindowsException):
    def __init__(self, winerror, strerror, funcname, filename=None):
      WindowsException.__init__(self, winerror, strerror, filename)
      self.funcname = funcname

This would also mean it got pretty stringifying rather than the ugly 3-tuple repr it has currently.

Changing the base type probably would break some code with carefully ordered except blocks, but would be a good thing in the long run.

The pywintypes.com_error should probably remain descended from plain Exception, comtypes.COMError is as well.

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

>Comment By: gz ()
Date: 2010-05-19 06:44

Message:
You're right, that's the judgement that needs to be made. Looking to see
how the exception is handled in real code shows up less obvious breakage
than I'd expect, mostly because so little code is doing anything sensible
in response to errors.

Poking around with:
<http://www.google.com/codesearch?q=%22except+pywintypes.error%22>

There's the ignore-and-hope approach:

<http://www.google.com/codesearch/p#Dm6oHlSzlBo/src/zope/app/winservice/service.py&l=80>
    except pywintypes.error:
        # the process may already have been terminated
        pass

<http://twistedmatrix.com/trac/browser/trunk/twisted/internet/_pollingfile.py?rev=27469#L179>
    except pywintypes.error:
        # Maybe it's an invalid handle.  Who knows.
        pass

<http://www.google.com/codesearch/p#AaFjKoY6UPA/dl/BitTorrent-5.0.5.tar.gz%7CwOfsDlaYENg/BitTorrent-5.0.5/BitTorrent/platform.py&l=445>
    except pywintypes.error, e:
        # I've seen:
        # error: (1784, 'DeviceIoControl', 'The supplied user buffer is
not valid for the requested operation.')
        return

Or report, and optionally die:

<http://www.google.com/codesearch/p#bXPcfyPP8Uc/trunk/tepache.py&l=366>
    except pywintypes.error, e:
        printerr("You haven't installed any GnuWin32 program.")

<http://www.google.com/codesearch/p#mXwtIi4wwhA/software/changenav/cnav-1.1.0-src.zip%7Cd_heUe2jE1k/process.py&l=16>
    except pywintypes.error, e:
            fatalError('Error launching process\n'
                               '(%s):\n%s' % (command, e[2]))

Or, y'know, give me a real exception I can actually work with:

<http://svn.python.org/view/python/trunk/Lib/subprocess.py?revision=81179&view=markup>
    except pywintypes.error, e:
        # Translate pywintypes.error to WindowsError, which is
        # a subclass of OSError.  FIXME: We should really
        # translate errno using _sys_errlist (or simliar), but
        # how can this be done from Python?
        raise WindowsError(*e.args)

<http://www.google.com/codesearch/p#vsRFXGKPaKo/resources/lib/durus/durus/file.py&l=95>
    except pywintypes.error:
        raise IOError("Unable to obtain lock")

<http://www.google.com/codesearch/p#yqvQ9RM69FY/mercurial/util_win32.py&l=161>
    except pywintypes.error, details:
        raise WinOSError(details)

Mercurial seem to have removed that last file, 's a good thing, half of it
is just trying to fix up error codes as done by WindowsException.

Only thing I've found liable to break seriously is tuple-unpacking the
exception instance:

<http://www.google.com/codesearch/p#1IKf2ZWr9OM/tools/third_party/python/Lib/site-packages/win32/lib/win32serviceutil.py&l=420>
    except pywintypes.error, (hr, name, msg):
        if hr!=winerror.ERROR_SERVICE_NOT_ACTIVE:
            raise win32service.error, (hr, name, msg)

And hey, you fixed that!

<http://pywin32.cvs.sourceforge.net/viewvc/pywin32/pywin32/win32/Lib/win32serviceutil.py?r1=1.30&r2=1.31>

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

Comment By: Mark Hammond (mhammond)
Date: 2010-05-19 00:45

Message:
I agree that if the exception was created today it probably should inherit
from WindowsError, but I am skeptical that the pain this causes is worth
the pain created by breaking existing code with carefully written exception
handlers - particularly as exception handlers are often hard to test, so
the breakage may not be obvious until a rarely hit exception is
encountered.  The work-around for you would seem to simply use 2 except
clauses, but handle them in the same way.

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

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=551954&aid=3003706&group_id=78018