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.

Sign up for GitHub

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

Jump to bottom

websocket: message.json() fails with a TypeError when the message exceeds the size limit #7313

Open
1 task done
justvanrossum opened this issue Jun 7, 2023 · 5 comments · May be fixed by #7319
Open
1 task done

websocket: message.json() fails with a TypeError when the message exceeds the size limit #7313

justvanrossum opened this issue Jun 7, 2023 · 5 comments · May be fixed by #7319
Assignees
@asvetlov
Labels
question StackOverflow

Comments

@justvanrossum
Copy link

justvanrossum commented Jun 7, 2023 •
edited

Describe the bug

When a websocket message exceeds the size limit, calling message.json() raises a TypeError instead of an informative exception

To Reproduce

Send a large websocket message to an aiohttp server.

In the handler, calling message.json() raises TypeError:

TypeError: the JSON object must be str, bytes or bytearray, not WebSocketError

message.data is set to a WebSocketError instance:

aiohttp.http_websocket.WebSocketError: Message size 29818 exceeds limit 8192

(I lowered the limit to more easily trigger the problem)

Expected behavior

I would expect calling message.json() to raise the WebSocketError.

Logs/tracebacks

Traceback (most recent call last):
  File "/Users/just/code/git/BlackFoundry/fontra/src/fontra/core/remote.py", line 47, in _handleConnection
    message = message.json()
              ^^^^^^^^^^^^^^
  File "/Users/just/code/git/BlackFoundry/fontra/venv/lib/python3.11/site-packages/aiohttp/http_websocket.py", line 98, in json
    return loads(self.data)
           ^^^^^^^^^^^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/json/__init__.py", line 339, in loads
    raise TypeError(f'the JSON object must be str, bytes or bytearray, '
TypeError: the JSON object must be str, bytes or bytearray, not WebSocketError

Python Version

Python 3.11.2

aiohttp Version

Name: aiohttp
Version: 3.8.4

multidict Version

Name: multidict
Version: 6.0.4

yarl Version

Name: yarl
Version: 1.8.2

OS

macOS 10.15

Related component

Server

Additional context

No response

Code of Conduct

  • I agree to follow the aio-libs Code of Conduct
@Dreamsorcerer
Copy link
Member

Dreamsorcerer commented Jun 9, 2023 •
edited

OK, I've been working through a PR to try and make this more type safe.

2 options we can do. First is to raise the exception from .json(), but I'm not sure how much sense this makes, and it would still fail on other message types (e.g. CLOSE has an int, which would still produce a TypeError). This would not affect type safety.

Second is to have .json() only on TEXT and BINARY messages. This means the code would need to do something like if msg.type in (WSMsgType.TEXT, WSMsgType.BINARY). But, this could be checked with mypy etc. If you don't check the type it'll complain that there's no .json() method.

@Dreamsorcerer
Copy link
Member

Dreamsorcerer commented Jun 10, 2023 •
edited

Note that you can implement the second approach in your code now. My changes would just make it a requirement and flag up any issues when checked with mypy.

@Dreamsorcerer Dreamsorcerer linked a pull request Jun 10, 2023 that will close this issue
Draft
@justvanrossum
Copy link
Author

I think I am puzzled (as a newbie aiohttp user) why the error is delivered as a message instead of a raised exception.

@Dreamsorcerer
Copy link
Member

I'm not too sure what the reasoning is for this, but you'll note from the websockets example that the expected behaviour is to check for WSMsgType.ERROR and then break (because the connection will be closed on an error):
https://docs.aiohttp.org/en/stable/client_quickstart.html#websockets

The data will only be an exception if the type is ERROR.

Maybe @asvetlov can provide some reasoning why we don't raise the exception.

@Dreamsorcerer Dreamsorcerer added question StackOverflow and removed bug labels Jun 11, 2023
@justvanrossum
Copy link
Author

The data will only be an exception if the type is ERROR.

Thanks, I've adjusted my code to check for WSMsgType.ERROR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@asvetlov asvetlov

Labels
question StackOverflow
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Refactor WSMessage to use tagged unions aio-libs/aiohttp
3 participants
@asvetlov @Dreamsorcerer @justvanrossum