Skip to content
This repository has been archived by the owner on Sep 13, 2024. It is now read-only.

AttributeError when using aiohttp #116

Closed
haael opened this issue Nov 16, 2023 · 10 comments · Fixed by #60
Closed

AttributeError when using aiohttp #116

haael opened this issue Nov 16, 2023 · 10 comments · Fixed by #60
Labels
bug A crash or error in behavior.

Comments

@haael
Copy link

haael commented Nov 16, 2023

Describe the bug

When doing a GET request from aiohttp, console shows AttributeError. The error doesn't happen when using aiohttp outside of gbulb.

Steps to reproduce

import aiohttp
def main(url):
    client = aiohttp.ClientSession()
    async with client.get(url) as response:
        response.raise_for_status()
        print(await response.read())
    await client.close()

Expected behavior

Success.

Screenshots

No response

Environment

Ubuntu, Python 3.11

Logs

Traceback (most recent call last):
  File "/usr/lib/python3.11/asyncio/events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
  File "/usr/local/lib/python3.11/dist-packages/gbulb/transports.py", line 177, in _loop_reading
    self._submit_read_data(data)
  File "/usr/local/lib/python3.11/dist-packages/gbulb/transports.py", line 127, in _submit_read_data
    self._protocol.data_received(data)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'SSLProtocol' object has no attribute 'data_received'

Additional context

No response

@haael haael added the bug A crash or error in behavior. label Nov 16, 2023
@freakboy3742
Copy link
Member

Thanks for the report - unfortunately, I can't reproduce this failure.

Firstly, because it looks like you've trimmed a little too much from your reproduction example - it can't work as written because main isn't declared async. I had to modify it to read:

import asyncio
import aiohttp

async def main(url):
    client = aiohttp.ClientSession()
    async with client.get(url) as response:
        response.raise_for_status()
        print(await response.read())
    await client.close()

if __name__ == "__main__":
    asyncio.run(main("https://www.wikipedia.org/"))

Even then, on Python 3.11 on Fedora 38, it works fine for me on multiple runs.

The one detail you didn't include in your bug report was the version of gbulb you are using; is it possible you're running an older gbulb version that didn't have Python 3.11 support?

@freakboy3742
Copy link
Member

Note to self: don't review tickets before the coffee kicks in. I just realised I didn't test this against gbulb - I was just running it a virtual environment that had gbulb installed.

Apologies for the confusion; I'll take a second look.

@freakboy3742
Copy link
Member

I can now confirm I see this, but only on Python 3.11.

The test case is:

import asyncio
import aiohttp
import gbulb

async def main(url):
    client = aiohttp.ClientSession()
    async with client.get(url) as response:
        response.raise_for_status()
        print(await response.read())
    await client.close()

if __name__ == "__main__":
    gbulb.install()
    asyncio.run(main("https://www.wikipedia.org/"))

odahoda added a commit to odahoda/gbulb that referenced this issue Dec 14, 2023
@odahoda
Copy link

odahoda commented Dec 14, 2023

Just ran into the same issue here (downgrading to python3.10 does indeed fix it).

To add a few more details: Looks like gbulb does not know how to deal with BufferedProtocol, which were added in 3.7, but it doesn't look like there were used in python's stdlib until 3.11, when SSLProtocol became a BufferedProtocol (python/cpython@13c10bf).

...ok, couldn't resist. It's really ugly, but it seems to work: odahoda@06c71af

@freakboy3742
Copy link
Member

Thanks for that debugging - that means #58 and #60 are likely related. It would be worth checking if #60 fixes this problem - if it does, then that increases the impetus to finish the work needed to land that patch.

@odahoda
Copy link

odahoda commented Dec 15, 2023

The PR #60 does indeed look like the proper fix, though I can't test it easily, because the branch is outdated and missing other py3.11 related fixes...

@bitstuffing
Copy link

bitstuffing commented Jan 7, 2024

So... I'm interested in this bug, how can I help you testing this issue? I'm in this situation with a simple WebSocket connection, but... reading this conversation I think we can check a "patch".

Anyway, I have currently in my project this problem, so "I'm able to reproduce this issue".

Regarding your comments, to clarify other developers, I saw that this bug is not present in python < 3.11.

@freakboy3742
Copy link
Member

The testing strategy is to set up an environment and install the code from the PR branch, then see if it resolves the issue.

The follow-up problem is that the neither the current codebase or the PR contains any tests that demonstrates the problem that has been reported. We need to add a test case that exercises the problematic code.

@bitstuffing
Copy link

bitstuffing commented Jan 12, 2024

The testing strategy is to set up an environment and install the code from the PR branch, then see if it resolves the issue.

The follow-up problem is that the neither the current codebase or the PR contains any tests that demonstrates the problem that has been reported. We need to add a test case that exercises the problematic code.

But... I see that this PR should have been tested 2 years before, it's not simple, there are a lot of incompatibilities with the current version.

It needs more work than a simple checkout to be checked.

I see the error, and apparently, the author of the PR made a good work because apparently, creates the protocol and solves the missing arguments than appears in the error:

AttributeError: 'SSLProtocol' object has no attribute 'data_received'

Anyway, 2 years before... it's not a simple python version upgrade of our system, this error is happening too much time before.

About the tests... I understand that your tests are not failing, and I see why. In my project, I have added and launched a bug.py with the code and a briefcase dev --test, and with that it's working. Why? because this error is related with gbulb and GTK, so this error just happens when you build a GUI and not from console.

@bitstuffing
Copy link

bitstuffing commented Jan 12, 2024

I have applied a "merge" manually with the PR, and, at least for me, it didn't solve the situation. So... need more work to be solved :'(

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug A crash or error in behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants