[issue42413] Replace custom exception socket.timeout with TimeoutError

2020-11-23 Thread STINNER Victor


STINNER Victor  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue42413] Replace custom exception socket.timeout with TimeoutError

2020-11-23 Thread STINNER Victor


STINNER Victor  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue42413] Replace custom exception socket.timeout with TimeoutError

2020-11-21 Thread Serhiy Storchaka


Serhiy Storchaka  added the comment:

This is a good idea. Writing IsADirectoryError(errno.EISDIR, 
os.strerror(errno.EISDIR), filename) is boring.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue42413] Replace custom exception socket.timeout with TimeoutError

2020-11-21 Thread Christian Heimes


Christian Heimes  added the comment:

There is another option: The subclasses of OSError could set a correct errno 
automatically.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue42413] Replace custom exception socket.timeout with TimeoutError

2020-11-21 Thread Andrew Svetlov


Andrew Svetlov  added the comment:

Thus using bare TimeoutError in asyncio is safe, isn't it?
This is good news!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue42413] Replace custom exception socket.timeout with TimeoutError

2020-11-21 Thread Serhiy Storchaka


Serhiy Storchaka  added the comment:

I fixed many instantiations of OSError subclasses, but many are still left in 
Python code.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue42413] Replace custom exception socket.timeout with TimeoutError

2020-11-21 Thread Andrew Svetlov


Andrew Svetlov  added the comment:

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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue42413] Replace custom exception socket.timeout with TimeoutError

2020-11-21 Thread Christian Heimes


Christian Heimes  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue42413] Replace custom exception socket.timeout with TimeoutError

2020-11-21 Thread Serhiy Storchaka


Serhiy Storchaka  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue42413] Replace custom exception socket.timeout with TimeoutError

2020-11-21 Thread Andrew Svetlov


Andrew Svetlov  added the comment:

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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue42413] Replace custom exception socket.timeout with TimeoutError

2020-11-21 Thread Christian Heimes


Christian Heimes  added the comment:

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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue42413] Replace custom exception socket.timeout with TimeoutError

2020-11-21 Thread Andrew Svetlov


Andrew Svetlov  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue42413] Replace custom exception socket.timeout with TimeoutError

2020-11-21 Thread Christian Heimes


Christian Heimes  added the comment:

Thanks Serhiy!

Andrew, Antoine, Yury, do you want to replace the custom Timeout exceptions in 
asyncio, multiprocessing, and concurrent with global TimeoutError, too?

--
nosy: +asvetlov, bquinlan, davin, pitrou, yselivanov
resolution: fixed -> 
stage: resolved -> 
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue42413] Replace custom exception socket.timeout with TimeoutError

2020-11-21 Thread Serhiy Storchaka


Serhiy Storchaka  added the comment:

There are also other distinct exceptions:

   multiprocessing.TimeoutError
   concurrent.futures.TimeoutError
   asyncio.TimeoutError

--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue42413] Replace custom exception socket.timeout with TimeoutError

2020-11-20 Thread Christian Heimes


Change by Christian Heimes :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue42413] Replace custom exception socket.timeout with TimeoutError

2020-11-20 Thread miss-islington


miss-islington  added the comment:


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


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue42413] Replace custom exception socket.timeout with TimeoutError

2020-11-19 Thread Christian Heimes


Change by Christian Heimes :


--
keywords: +patch
pull_requests: +22306
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/23413

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue42413] Replace custom exception socket.timeout with TimeoutError

2020-11-19 Thread Christian Heimes


New submission from Christian Heimes :

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.

--
assignee: christian.heimes
components: Extension Modules
messages: 381454
nosy: alex, christian.heimes, corona10, dstufft, janssen, vstinner
priority: normal
severity: normal
status: open
title: Replace custom exception socket.timeout with TimeoutError
type: enhancement
versions: Python 3.10

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com