Improve OSError constructor

The purpose of introducing multiple OSError subclasses (PEP 3151) was to allow user to write

except FileNotFoundError:
    ...

instead of

except OSError as e:
    if e.errno != errno.ENOENT:
        raise
    ...

OSError constructor now determines the actual type of exception by the errno argument, so it works with both codes above. OSError(errno.ENOENT, ...) returns an instance of FileNotFoundError.

Of course, you can also call the subclass constructor explicitly. But if you call it with less than 2 arguments, the errno and strerror attributes are set to None. FileNotFoundError(f'Directory does not exist: {dst}') works with the former code, but not with the latter code.

I propose to infer the default value for errno and strerror from exception class if the constructor gets less than 2 positional arguments. You may want to pass additional arguments like filename when skipping errno and maybe strerror. For this, the constructor needs to support keyword arguments. The constructor signature will be a union of the following signatures:

(errno, strerror, /, filename=None, winerror=None, filename2=None)
(strerror, /, *, filename=None, winerror=None, filename2=None)
(*, filename=None, winerror=None, filename2=None)

So, you would be able to write FileNotFound('Directory does not exist', filename=dst) or even simply FileNotFound(filename=dst).

In the past I explicitly added errno argument in my changes and suggested to do this in reviewd changes. Still, there is number of calls without errno. The following PR demonstrates the changes necessary for passing errno and strerror explicitly. It is enough combersome, so it would be more convenient to set this automatically.

8 Likes

(copying and expanding my reply from the GH issue)

Hmm, I wouldnā€™t be against it, but the reason I didnā€™t do it at the time is that errno is the system error indicator at the time was exception was raised. If you synthetize your own errno then it starts meaning something else: you canā€™t assume anymore that the system did set errno to that value at some point.

I have no idea how useful that assumption is, but it might be for some people.

Also, in practice, Iā€™m not sure why someone would choose to manually write the e.errno != errno.ENOENT test instead of catching FileNotFoundError.

2 Likes
  • It may be an old code, written before PEP 3151. It works well with FileNotFoundError raised implicitly by open but stops working when some other code changes EAFP to LBYL and raise FileNotFoundError explicitly. Also, we can introduce new OSError subclasses in future, the code that currently tests errno should still work.

  • The error handler may be common for all OSError, but some parts only specific for specific errno. With separate except clauses you need to duplicate the common code.

  • It may be test e.errno in IGNORED_ERRNOS where IGNORED_ERRNOS is a set of errno values that correspond to different OSError subclasses or even do not have dedicated OSError subclass. You can see many examples in the stdlib and tests.

  • On my past work, we translated some errors from server to client. OSError was translated as errno-message pair. If errno is None, it cannot be recreated at client side, so client could get general ClientError instead of more specific FileNotFoundError.

1 Like

Iā€™m also not necessarily against it, but just to point out for sake of discussion, PEP 3151 was Python 3.3 so it probably is the least of your worries if youā€™re updating code that old to something more modern.

2 Likes

I also just took a look and the WIP PR doesnā€™t make any documentation changes. The proposed signature change is complex enough that Iā€™d really like to see how the new signature would be documented.

1 Like

You can just write isinstance(e, FileNotFoundError), which is more natural than testing an obscure errno value. Symbolic names of errno values are so cryptic that few people remember them without having to look up the docs. It probably took me years to reliably remember ā€œENOENTā€, which was probably the most frequently tested of all errno values.

I admit thatā€™s a more reasonable use for errno checks.

Hmm, either you use pickle, or if you canā€™t for security reasons, you can just send the exception class name to recreate it on the client.

You could still replace that with isinstance(e, IGNORED_OSERROR_TYPES)

Is it guarrenteed that thereā€™s a type for every errno.THING name? I
did a quick grep over my person code, which has plenty of e.errno == errno.ENOENT
style tests (yes, a lot well predate 3.3), and 2 examples particularly
catch my eye:

 if e.errno == errno.ENAMETOOLONG and crop_ok:
 if e.errno not in (errno.ENOTSUP, errno.ENOENT, errno.ENODATA):

Do we have classes for all of these? I could look, but I tend to think
of these names as a bit open ended - do we ensure class coverage for all
the names errno.THING names when thereā€™s a release for a platform?

And what happens on some oddball POSIXy system with an errno value not
covered by the stdlib for some esoteric system call? Iā€™d still need to
do a numeric check against e.errno in that situation.

Cheers,
Cameron Simpson cs@cskk.id.au

Not all errno values have a specific OSError subclass, and many highly related errno values share the same subclass.

This was decided way back in the PEP 3151 ā€“ Reworking the OS and IO exception hierarchy | peps.python.org implementation days. The most commonly referenced ones were categorized and used to come up with the list of names. PEP 3151 ā€“ Reworking the OS and IO exception hierarchy | peps.python.org

If you find an errno value lacking a named OSError subclass that you have need for, feel free to file a feature request. They can be added in the future.

1 Like

Overall Iā€™m in favor of this, we left ourselves with a design issue when creating the subclasses. It shouldnā€™t have been possible to create them without the proper errno matching what the class is for, set, at a minimum.

I believe the WIP PR #109720 is an example of what is required to fix all our existing bugs without having made the OSError & subclass constructor improvements. ie: it is backfilling places where we inadvertently did not pass the right arguments.

Agreed otherwise that itā€™d be good to see how we document the new signature, but a PR implementing that hasnā€™t been linked to from here or the issue yet.

New OSError subclasses can be added in future.

You can write it in less formal way as

([[errno,] strerror,] filename=None, winerror=None, filename2=None)

It is not new, the bracket syntax, even with nested brackets is used in other functions documentation. And optional first arguments are not new, see range() and a number of curses functions. But this syntax is less precise, for example it is not clear that first two parameters are positional-only and others are positional-or-keyword. I works on support of multi-signatures (postponed until 3.12 release due to more urgent matters), so I think that multiple signatures will be more familiar in near future.

Yes, of course. But there is less advantage of isinstance(e, FileNotFoundError) over e.errno = errno.ENOENT than of simple except FileNotFoundError. Be that as it may, such code exists.

No, it was not pickle. It was simpler to do with errno than with exception class name. It worked well, because all implicitly raised OSError exceptions (except TimeoutError and UnsupportedOperation, but they did not play role) have errno, and for explicitly raised exceptions I provided it for constructor.

2 Likes