mhammond / pywin32

Python for Windows (pywin32) Extensions
5.06k stars 799 forks source link

pywintypes.error should inherit from OSError #486

Open ghost opened 14 years ago

ghost commented 14 years ago

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&gt;.

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.

Reported by: *anonymous

Original Ticket: pywin32/bugs/486

ghost commented 14 years ago

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.

Original comment by: mhammond

ghost commented 14 years ago

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&gt;

There's the ignore-and-hope approach:

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

<http://twistedmatrix.com/trac/browser/trunk/twisted/internet/\_pollingfile.py?rev=27469\#L179&gt; 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&gt; 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&gt; 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&gt; 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&gt; 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&gt; except pywintypes.error: raise IOError("Unable to obtain lock")

<http://www.google.com/codesearch/p\#yqvQ9RM69FY/mercurial/util\_win32.py&l=161&gt; 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&gt; 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&gt;

Original comment by: *anonymous

ghost commented 9 years ago

I hope this will still be considered. It causesheadaches and code duplications in new code, as Anonymous has shown above, some resort to translating it to a WindowsError. Perhaps it could be introduced as a backwards-incompatible change. Like you said, it's a very rare case for old code to be broken and easy to fix.

Original comment by: simonzack

Avasam commented 8 months ago

As a note, WindowsError, and many more, are an alias to OSError as of Python 3.3 https://docs.python.org/3/library/exceptions.html#OSError .

Easy to fix, but not necessarily easy to find. Although I agree it'd be nice to "do things right", it will be a breaking change.

mhammond commented 8 months ago

I don't have any appetite for this, mainly as I can't see a compelling reason for the breaking change.