Add reason support for websocket.close #1339
Conversation
|
Re "it's not part of the spec", django/asgiref@6469d0f |
There was a problem hiding this comment.
There are some approaches we can take with the "reason". On the "code" key, we choose to send 1000 by default, but that's not quite what is written in the specs. We could have choose to not send it, and the server would assume 1000.
As for the "reason" key, we can do the same, and assume default being empty string. Or we can just do as the suggested snippets and only send the reason if a reason is added.
I've just noticed an issue on uvicorn on the way we retrieve the reason, as we don't check if reason is None, which is allowed by the spec. I'll be opening a PR there.
Reference for the new ASGI spec: https://asgi.readthedocs.io/en/latest/specs/www.html#close-send-event
|
|
||
|
|
||
| class WebSocketClose: | ||
| def __init__(self, code: int = 1000) -> None: | ||
| def __init__(self, code: int = 1000, reason: typing.Optional[str] = "") -> None: |
There was a problem hiding this comment.
| def __init__(self, code: int = 1000, reason: typing.Optional[str] = "") -> None: | |
| def __init__(self, code: int = 1000, reason: str = None) -> None: |
@michallowasrzechonek-silvair I didn't get what you said. 😗 |
Co-authored-by: Marcelo Trylesinski <[email protected]>
Co-authored-by: Marcelo Trylesinski <[email protected]>
|
Would you mind also adding tests for this? 🙏 |
|
I will fix it tomorrow afternoon got some studying to do |
|
@Chaostheorie Are you still interested on working on this? |
|
@Kludex Not really. Sorry to leave this PR hanging, will close it asap. |
Recreate old changes
Carbon-Copy of #992
Aims to resolve #991