asgiref icon indicating copy to clipboard operation
asgiref copied to clipboard

Spec question: `websocket.disconnect` doesn't support the `reason` field

Open frankie567 opened this issue 1 year ago • 10 comments

From the WebSocket specification, it's possible to set a "reason" when either the server or the client closes the connection: https://datatracker.ietf.org/doc/html/rfc6455#section-7.1.6

In the ASGI specification, this is well supported for the "Close - send event" (i.e. close sent by the server):

https://github.com/django/asgiref/blob/db3ff43e9fa1daf7c6b1cb67db8eec1885be911d/specs/www.rst?plain=1#L476-L496

However, for the "Disconnect - receive event (i.e. close sent by the client), it is not supported; only the code can be set:

https://github.com/django/asgiref/blob/db3ff43e9fa1daf7c6b1cb67db8eec1885be911d/specs/www.rst?plain=1#L440-L458

Is there a particular reason to omit this field for this particular event?

frankie567 avatar Apr 08 '24 11:04 frankie567

I do not remember a specific reason; it was likely either an oversight or the libraries I developed the spec against last decade didn't support it.

andrewgodwin avatar Apr 10 '24 00:04 andrewgodwin

Thank you for your feedback 🙂 Then, do you see any inconvenience if I propose a PR to add this to the spec?

frankie567 avatar Apr 10 '24 05:04 frankie567

Can you give me some days to reply here? I'm on vacation, but I'm back next week.

Kludex avatar Apr 10 '24 12:04 Kludex

Sure! Enjoy your holidays 🌴

frankie567 avatar Apr 10 '24 12:04 frankie567

Hey @Kludex 👋 Did you have some time to think about this? 🙂

frankie567 avatar May 03 '24 07:05 frankie567

It looks like an oversight, but... No one complained in years... Do you need it?

Kludex avatar May 03 '24 07:05 Kludex

Well, not a really big issue but yes, I had one complain downstream in httpx-ws. Relevant context:

  • https://github.com/encode/uvicorn/discussions/2299
  • https://github.com/frankie567/httpx-ws/issues/70

IMHO, it looks like a harmless addition that would make the spec a bit more compliant regarding WS spec. Happy to make the PR, if you agree 🙂

frankie567 avatar May 03 '24 07:05 frankie567

Do the WebSockets clients send the "reason"?

Kludex avatar May 03 '24 07:05 Kludex

They can, if I understand correctly the spec. It's apparently supported by browsers API: https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/close

frankie567 avatar May 03 '24 07:05 frankie567

Then I don't mind adding it.

Kludex avatar May 03 '24 08:05 Kludex