Skip to content
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

Wrong constructor called #8059

Open
1 task done
dmoklaf opened this issue Jan 26, 2024 · 3 comments
Open
1 task done

Wrong constructor called #8059

dmoklaf opened this issue Jan 26, 2024 · 3 comments
Labels

Comments

@dmoklaf
Copy link

dmoklaf commented Jan 26, 2024

Describe the bug

The ClientConnectorError class has 2 grand-parent classes: ClientConnectionError and OSError, in that order.

However, it seems to call the constructor of the wrong grand-parent:

super().__init__(os_error.errno, os_error.strerror)

Following the Python language specification ,a default super() calls the constructor of the first grand parent class, ClientConnectionError , with the wrong arguments (the constructor is supposed to take a message as first argument). The arguments provided, os_error.errno, and , os_error.strerror suggest that it should call instead the constructor of the second grand parent class, OSError .

But maybe my understanding of these classes is wrong.

To Reproduce

Read the code of the client_exceptions.py file:

super().__init__(os_error.errno, os_error.strerror)

Expected behavior

If my theory is true, then the constructor of the second grand-parent class shall be called, through:
super(OSError, self).__init__(os_error.errno, os_error.strerror)

Logs/tracebacks

Read the code (see above)

Python Version

$ python --version
Python 3.11.6

aiohttp Version

$ python -m pip show aiohttp
Name: aiohttp
Version: 3.9.1
Summary: Async http client/server framework (asyncio)
Home-page: https://github.com/aio-libs/aiohttp
Author: 
Author-email: 
License: Apache 2
Location: /opt/homebrew/Caskroom/miniforge/base/envs/work/lib/python3.11/site-packages
Requires: aiosignal, attrs, frozenlist, multidict, yarl
Required-by: aiohttp-jinja2, asyncpraw, asyncprawcore, datasets, httpstan, pystan

multidict Version

$ python -m pip show multidict
Name: multidict
Version: 6.0.4
Summary: multidict implementation
Home-page: https://github.com/aio-libs/multidict
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache 2
Location: /opt/homebrew/Caskroom/miniforge/base/envs/work/lib/python3.11/site-packages
Requires: 
Required-by: aiohttp, Carl

yarl Version

$ python -m pip show yarl
Name: yarl
Version: 1.9.4
Summary: Yet another URL library
Home-page: https://github.com/aio-libs/yarl
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache-2.0
Location: /opt/homebrew/Caskroom/miniforge/base/envs/work/lib/python3.11/site-packages
Requires: idna, multidict
Required-by: aiohttp, asyncprawcore

OS

MacOS Sonoma 14.2.1 (23C71)

Related component

Client

Additional context

No response

Code of Conduct

  • I agree to follow the aio-libs Code of Conduct
@dmoklaf dmoklaf added the bug label Jan 26, 2024
@Dreamsorcerer
Copy link
Member

Does this result in some wrong behaviour (like the error message being incorrect)? If so, then a simple test would be great.

@dmoklaf
Copy link
Author

dmoklaf commented Jan 27, 2024

Yes there is one wrong behavior: the OSError object is not properly initialized with the errno information. So any user of the library catching OSErrors will obtain wrongly initialized objects. This is an exotic case although.

I didn't focus on the error message as aiohttp is (like many Python libraries) very heterogeneous in terms of quality of the exception messages, from none (no message at all despite useful information available) to very chatty. So I don't think the error message is the issue there (and if it were it would require a separate issue to resolve it across several exception types).

@Dreamsorcerer
Copy link
Member

Yes there is one wrong behavior: the OSError object is not properly initialized with the errno information. So any user of the library catching OSErrors will obtain wrongly initialized objects. This is an exotic case although.

That's fine, if you can add a test showing that, then feel free to make a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants