-
-
Notifications
You must be signed in to change notification settings - Fork 800
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
Ensure text message exists before handling on WebsocketConsumer
#2097
Ensure text message exists before handling on WebsocketConsumer
#2097
Conversation
Hey @cacosandon, this looks reasonable to me. Can you add a regression test? |
e8a795e
to
26fdd81
Compare
Sorry for the super late reaction @bigfootjon, I've added a regression test! Had to use |
e539976
to
43a83c6
Compare
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.
LGTM. Thanks @cacosandon
@bigfootjon any last thoughts?
(Sorry, @cacosandon : can you fix the lint error? Thanks.) |
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.
LGTM!
When using
uvicorn
ordaphne
, only one oftext
orbytes
is sent.But on
hypercorn
, both of them are sent, but withNone
value:https://github.com/pgjones/hypercorn/blob/31639ec2f4d03aa920b95c84686163901224c6cf/src/hypercorn/protocol/ws_stream.py#L159
So if you just send
bytes
,text
will beNone
and you won't be able to handle the message.I just add an extra check.