This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Replace custom exceptions for timeouts with TimeoutError
Type: enhancement Stage: resolved
Components: asyncio, Extension Modules, Library (Lib) Versions: Python 3.11
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: asvetlov Nosy List: alex, asvetlov, bquinlan, christian.heimes, corona10, davin, dstufft, janssen, kumaraditya, miss-islington, pitrou, serhiy.storchaka, vstinner, yselivanov
Priority: normal Keywords: patch

Created on 2020-11-19 19:10 by christian.heimes, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 23413 merged christian.heimes, 2020-11-19 19:34
PR 23520 closed asvetlov, 2020-11-26 08:26
PR 30197 merged kumaraditya, 2021-12-19 08:57
Messages (20)
msg381454 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2020-11-19 19:10
The socket module has a custom timeout exception "socket.timeout". The exception is documented https://docs.python.org/3/library/socket.html#socket.timeout as :

> A subclass of OSError, this exception is raised when a timeout occurs on a socket which has had timeouts enabled via a prior call to settimeout() (or implicitly through setdefaulttimeout()).

It's a distinct exception type and not the same as the global TimeoutError.

>>> import socket
>>> socket.timeout == TimeoutError
False

I like to follow the example of the deprecated "socket.error", replace the custom exception and make it an alias of TimeoutError. The risk is minimal. Both exceptions are subclasses of OSError.

The change would not only make exception handling more consistent. It would also allow me to simplify some code in ssl and socket C code. I might even be able to remove the socket CAPI import from _ssl.c completely.
msg381466 - (view) Author: miss-islington (miss-islington) Date: 2020-11-20 08:26
New changeset 03c8ddd9e94c7bddf1f06cf785027b8d2bf00ff0 by Christian Heimes in branch 'master':
bpo-42413: socket.timeout is now an alias of TimeoutError (GH-23413)
https://github.com/python/cpython/commit/03c8ddd9e94c7bddf1f06cf785027b8d2bf00ff0
msg381539 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-11-21 09:48
There are also other distinct exceptions:

   multiprocessing.TimeoutError
   concurrent.futures.TimeoutError
   asyncio.TimeoutError
msg381552 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2020-11-21 11:48
Thanks Serhiy!

Andrew, Antoine, Yury, do you want to replace the custom Timeout exceptions in asyncio, multiprocessing, and concurrent with global TimeoutError, too?
msg381553 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2020-11-21 11:58
In futures and asyncio TimeoutError has no errno.

I'm not sure should we care but I consider it as a show stopper.

On another hand, two distinct timeout classes confuse people. I had many conversions about this during the aiohttp support.  asyncio can raise both TimeoutError and asyncio.TimeoutError which is... unexpected at least.
msg381554 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2020-11-21 12:11
PyErr_SetString(PyErr_TimeoutError, "msg") and raise TimeoutError("msg") do not set any additional exception attributes. errno, strerror, filename, and filename2 are None:

>>> try:
...     raise TimeoutError("msg")
... except Exception as e:
...     err = e
... 
>>> err
TimeoutError('msg')
>>> (err.errno, err.filename, err.filename2, err.strerror)
(None, None, None, None)

Is that a problem?
msg381555 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2020-11-21 12:43
Perhaps it is a good compromise.
OSError-derived class without errno looks getter to me that different incompatible TimeoutError classes.

How many exceptions inherited from OSError have no errno set? Do we have a precedent in stdlib at all already?
msg381556 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-11-21 12:43
Yes, I think this is a problem. It is expected that an instance of OSError has valid errno. OSError is a merge of several old exception types, some of them did have errno, and others did not.

Maybe add BaseTimeoutError and make TimeoutError a subclass of BaseTimeoutError and OSError?

There as also several CancelledError, and exceptions with similar names and purpose, like asyncio.IncompleteReadError and http.client.IncompleteRead.
msg381557 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2020-11-21 12:50
They have an errno attribute, it's just to None by default. You have to call OSError with multiple arguments to set errno to a non-None value.
msg381558 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2020-11-21 12:59
I know that I just create OSError() with errno set to None.

My question is: has the standard library such code examples already?
In other words, how many third-party code will be broken by catching OSError with errno=None?
msg381559 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-11-21 13:02
I fixed many instantiations of OSError subclasses, but many are still left in Python code.
msg381560 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2020-11-21 13:04
Thus using bare TimeoutError in asyncio is safe, isn't it?
This is good news!
msg381562 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2020-11-21 14:03
There is another option: The subclasses of OSError could set a correct errno automatically.
msg381568 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-11-21 15:36
This is a good idea. Writing IsADirectoryError(errno.EISDIR, os.strerror(errno.EISDIR), filename) is boring.
msg381663 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-23 09:45
IMHO it's ok that exc.errno is None. It doesn't prevent to write code like:

except OSError as exc:
    if exc.errno == ...:
        ...
    else:
        ...

In the early days (first 5 years? :-D) of the asyncio documentation, TimeoutError was documented just as "TimeoutError", instead of "asyncio.TimeoutError". So if you followed carefully the asyncio documentation and wrote "except TimeoutError:", the except would never be reached beause asyncio.TimeoutError is *not* a subclass of the builtin Timeout...

>>> issubclass(asyncio.TimeoutError, TimeoutError)
False

It would be great to have a single TimeoutError class. I'm fine with having weird attributes depending who raise the exception. Honestly, in most cases "except TimeoutError:" is enough: there is no need to check for exception attributes.
msg381664 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-23 09:48
I see this issue as a follow-up of PEP 3151 which started to uniformize IOError, OSError and many variants that we had in Python 2.

socket.timeout was introduced as a subclass of TimeoutError according to:
https://www.python.org/dev/peps/pep-3151/#new-exception-classes

In Python 3.9, socket.timeout and TimeoutError are subclasses of OSError but are different. I agree that it was confusion.
msg381886 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2020-11-26 08:32
Pull Request https://github.com/python/cpython/pull/23520/ applies the discussed change to both asyncio and concurrent.futures.

I did the minimally invasive change, libraries still use `asyncio.TimeoutError` and `concurrent.futures.TimeoutError` internally but both names are aliases now.
msg404693 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-10-21 22:30
Andrew, could you please rebase your PR and get it submitted?
msg408893 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2021-12-19 11:23
New changeset da4b214304df38cf1831071804a2b83938f95923 by Kumar Aditya in branch 'main':
bpo-42413: Replace `concurrent.futures.TimeoutError` and `asyncio.TimeoutError` with builtin `TimeoutError` (GH-30197)
https://github.com/python/cpython/commit/da4b214304df38cf1831071804a2b83938f95923
msg408894 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2021-12-19 11:38
Done.
Thanks, Kumar!
History
Date User Action Args
2022-04-11 14:59:38adminsetgithub: 86579
2021-12-19 11:38:06asvetlovsetstatus: open -> closed
resolution: fixed
messages: + msg408894

stage: patch review -> resolved
2021-12-19 11:23:04asvetlovsetmessages: + msg408893
2021-12-19 08:57:28kumaradityasetnosy: + kumaraditya
pull_requests: + pull_request28418
2021-10-21 22:30:10christian.heimessetassignee: christian.heimes -> asvetlov
messages: + msg404693
versions: + Python 3.11, - Python 3.10
2020-11-26 08:32:58asvetlovsetmessages: + msg381886
2020-11-26 08:26:24asvetlovsetstage: patch review
pull_requests: + pull_request22404
2020-11-26 08:21:58asvetlovsetcomponents: + Library (Lib), asyncio
2020-11-26 08:21:43asvetlovsettitle: Replace custom exception socket.timeout with TimeoutError -> Replace custom exceptions for timeouts with TimeoutError
2020-11-23 09:48:41vstinnersetmessages: + msg381664
2020-11-23 09:45:38vstinnersetmessages: + msg381663
2020-11-21 15:36:34serhiy.storchakasetmessages: + msg381568
2020-11-21 14:03:04christian.heimessetmessages: + msg381562
2020-11-21 13:04:16asvetlovsetmessages: + msg381560
2020-11-21 13:02:47serhiy.storchakasetmessages: + msg381559
2020-11-21 12:59:52asvetlovsetmessages: + msg381558
2020-11-21 12:50:45christian.heimessetmessages: + msg381557
2020-11-21 12:43:43serhiy.storchakasetmessages: + msg381556
2020-11-21 12:43:37asvetlovsetmessages: + msg381555
2020-11-21 12:11:44christian.heimessetmessages: + msg381554
2020-11-21 11:58:39asvetlovsetmessages: + msg381553
2020-11-21 11:48:06christian.heimessetstatus: closed -> open

nosy: + bquinlan, pitrou, asvetlov, yselivanov, davin
messages: + msg381552

resolution: fixed -> (no value)
stage: resolved -> (no value)
2020-11-21 09:48:59serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg381539
2020-11-20 08:26:51christian.heimessetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2020-11-20 08:26:39miss-islingtonsetnosy: + miss-islington
messages: + msg381466
2020-11-19 19:34:04christian.heimessetkeywords: + patch
stage: patch review
pull_requests: + pull_request22306
2020-11-19 19:10:42christian.heimescreate