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

GreenFileDescriptorIO.seek should raise IOError, not OSError #1323

Closed
ricardokirkner opened this Issue Nov 29, 2018 · 1 comment

Comments

Projects
None yet
2 participants
@ricardokirkner

ricardokirkner commented Nov 29, 2018

  • gevent version: 1.3.7
  • Python version: "cPython 2.7.15"
  • Operating System: "Arch on amd64"

Description:

GreenFileDescriptorIO should raise an IOError exception instead of an OSError exception during it's seek method in order to remain compatible with the io module (as it's subclassing RawIOBase).

With the current implementation (using os.lseek internally) raising an OSError will break expectations of the file-like interface. For example, when using gevent in conjunction with boto, boto will catch IOError while trying to seek a file and handle it accordingly, but will fail to notice gevent raising an OSError (as it's not what the __builtins__ or io based file-like objects do).

What I've run:

Trying to seek a non-seekable file descriptor raises an OSError

>>> from gevent._fileobjectposix import GreenFileDescriptorIO
>>> gio = GreenFileDescriptorIO(0)
>>> gio.seek(0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python2.7/site-packages/gevent/_fileobjectposix.py", line 143, in seek
    return os.lseek(self._fileno, offset, whence)
OSError: [Errno 29] Illegal seek

but doing the same with the stdlib raises IOError

>>> import io
>>> f = io.open(0, 'rb')
>>> repr(f)
'<_io.BufferedReader name=0>'
>>> f.fileno()
0
>>> f.seek(0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
IOError: [Errno 29] Illegal seek
>>> import os
>>> f2 = os.fdopen(0, 'rb')
>>> repr(f2)
"<open file '<fdopen>', mode 'rb' at 0x7facbdd185d0>"
>>> f2.fileno()
0
>>> f2.seek(0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
IOError: [Errno 29] Illegal seek

For consistency, GreenFileDescriptorIO.seek should also raise IOError and not OSError.

@jamadden

This comment has been minimized.

Member

jamadden commented Nov 29, 2018

Thanks for the report! On Python 3, IOError is just a deprecated alias for OSError, so this issue only affects Python 2 (and only on POSIX platforms). Python 2 does document that seek should raise IOError, so we're not in compliance.

I would happily review a PR that makes and tests this change for Python 2. It may be somewhat tricky to get right, as it would need to preserve the errno and other attributes of the underlying exception while changing its type and preserving the original traceback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment