-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
ASGI websocket recv text or bytes #2640
Conversation
Codecov ReportBase: 88.699% // Head: 88.637% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #2640 +/- ##
=============================================
- Coverage 88.699% 88.637% -0.063%
=============================================
Files 82 82
Lines 6761 6768 +7
Branches 1156 1156
=============================================
+ Hits 5997 5999 +2
- Misses 527 532 +5
Partials 237 237
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
return message["text"] | ||
except KeyError: | ||
try: | ||
return message["bytes"].decode() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't bytes be received as is, without decoding? This is what the built-in server does, to support binary data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to receive it for now without breaking the existing contract. I think we need to revisit this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we need to. I would think this being necessary for 23.12.1 release because we cannot have LTS processing it wrong like that. Apparently not many use ASGI mode with binary WebSockets because this wasn't caught earlier.
The binary format is specifically meant for binary data, for which my own use cases include MsgPack, audio streaming and other raw data that will never be able to be decoded into text (if not for a freak accident of chance that still breaks the software). Never JSON which can be sent in text mode instead.
Applications can also employ the two different types on the same socket to differentiate raw data stream (binary) of control commands (text).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, probably sooner than a year from now and back port it.
Closes #2638