Disconnect() method of WebSocketConsumer not being called

[KishanKishore]

I have the following MyConsumer class:

from channels.generic.websocket import WebsocketConsumer
from channels.exceptions import StopConsumer
import json

class MyConsumer(WebsocketConsumer):
   
       def connect(self):
          self.accept()
       
       def receive(self, text_data=None, bytes_data=None):
          data = json.loads(text_data)
          n = data["number"]
          
          for i in range(n):
              self.send(json.dumps({"number":i}))
              
       def disconnect(self, code):
           raise StopConsumer()

The input JSON contains only a single parameter called number. I am testing this code using a chrome plugin. When I open the connection and close it without sending any message, the disconnect method is executed as expected.

When the number is for example 100000 and the loop inside the receive method is not yet finished and I disconnect in between, the disconnect method is not called and I get the following error:

ERROR - server - Exception inside application: Attempt to send on a closed protocol.
File "MyConsumer.py", line 2, in receive
    self.send
File "python3.6/site-packages/channels/generic/websocket.py", line 69, in send
    {"type": "websocket.send", "text": text_data},
  File "python3.6/site-packages/channels/consumer.py", line 107, in send
    self.base_send(message)
  File "python3.6/site-packages/asgiref/sync.py", line 64, in __call__
    return call_result.result()
  File "/usr/local/var/pyenv/versions/3.6.10/lib/python3.6/concurrent/futures/_base.py", line 432, in result
    return self.__get_result()
  File "/usr/local/var/pyenv/versions/3.6.10/lib/python3.6/concurrent/futures/_base.py", line 384, in __get_result
    raise self._exception
  File "python3.6/site-packages/asgiref/sync.py", line 78, in main_wrap
    result = await self.awaitable(*args, **kwargs)
  File "python3.6/site-packages/channels/sessions.py", line 220, in send
    return await self.real_send(message)
  File "python3.6/site-packages/daphne/server.py", line 198, in handle_reply
    protocol.handle_reply(message)
  File "python3.6/site-packages/daphne/ws_protocol.py", line 179, in handle_reply
    self.serverSend(message["text"], False)
  File "site-packages/daphne/ws_protocol.py", line 223, in serverSend
    self.sendMessage(content.encode("utf8"), binary)
  File "python3.6/site-packages/autobahn/websocket/protocol.py", line 2216, in sendMessage
    raise Disconnected("Attempt to send on a closed protocol")
  Attempt to send on a closed protocol

OS: Mac OS X
runtime: python 3.6.10
daphne==2.2.0
Django==2.0.6
channels==2.1.1
channels-redis==2.2.1

• Expected: The docs mention that the disconnect method should be called when the client closes the connection and an event is received (https://channels.readthedocs.io/en/latest/topics/consumers.html#closing-consumers). I expect that the disconnect method is called even if the loop is not finished yet.

• I am running Channels using daphne.

[brubbel]

Same issue for AsyncWebsocketConsumer.

Issue caused by a recent change in autobahn:
https://autobahn.readthedocs.io/en/latest/changelog.html#id17

fix: WebSocket protocol instances now raise autobahn.exception.Disconnected when sending on a closed connection (#1002)
crossbario/autobahn-python#1002

Possible solution by subclassing AsyncWebsocketConsumer (channels/generic/websocket.py):

    async def send(self, *args, **kwargs):
        try:
            await super().send(*args, **kwargs)
        except autobahn.exception.Disconnected:
            await self.close()

[sknaumov]

I see the very same issue: if exception is raised inside, say, receive() method, then disconnect() method is never called. But, according to documentation, disconnect() should be called on websocket close.

It means that currently disconnect() couldn't be used as a reliable callback to cleanup things.

[carltongibson]

A failing test demonstrating the issue, and then the fix, would be appreciated here.

[sknaumov]

According to documentation of generic consumers:

def disconnect(self, close_code):
    # Called when the socket closes

But it is not always the case. Here is an example:

class ExampleConsumer(AsyncWebsocketConsumer):
    async def connect(self):
        await self.accept()

    async def disconnect(self, close_code):
        print("disconnect", flush=True)

    async def receive(self, text_data=None, bytes_data=None): 
        raise Exception()

On the client javascript side I have:

sock.onclose = (event) => {
    term.write("Connection closed\r\n");
};

and I see Connection closed output in the browser, so socket is actually closed, but disconnect method hasn't been called.

If to replace raising of exception in receive() with await self.close(), then disconnect is called as expected.

[brunogarciagonzalez]

I think this SO answer is what some searchers may be looking for (it was for me): https://stackoverflow.com/a/52534791
In my own words:
If you are looking to do cleanup on socket close, you can leverage your consumer's self.websocket_disconnect, which is triggered on actual socket close and ultimately calls disconnect.
Just remember to call super 👀

This of course does not address the potential issue directly: if disconnect should be triggered, and it isn't, then this issue stands.

[mikaraunio]

@carltongibson What's your view as to where in the code to best address this, and what the desired outcome would be?

This is really two problems coming together:

1. send() erroring if the connection is closed before the write completes
2. Channels not calling websocket_disconnect() if an exception occurs during receive()

Regarding the first point, not sure if an unhandled exception is really warranted considering that clients going away suddenly is a normal occurrence given the unreliable transport that is the Internet. While you can guard against this like @brubbel does above, Daphne and uvicorn raise different exceptions, so that's not a clear-cut solution. Maybe this should be addressed in the protocol servers?

As for the second point, websocket_disconnect is called with both Daphne and uvicorn in most other 'exceptional' circumstances, including the client abruptly dropping the connection (outside of a pending send()) and with a pong timeout, so I don't really see the rationale for it not being always called. Think that's what most users are expecting as well.

For anyone struggling with this, this is the workaround I'm currently using with AsyncJsonWebsocketConsumer:

from channels.generic.websocket import AsyncJsonWebsocketConsumer

class GuardedAsyncJsonWebsocketConsumer(AsyncJsonWebsocketConsumer):
    async def receive(self, text_data=None, bytes_data=None, **kwargs):
        error_code = 4011  # Daphne prohibits using 1011 / Server Error

        try:
            # super().receive() will call our receive_json()
            await super().receive(text_data=text_data, bytes_data=bytes_data, **kwargs)
        except Exception:
            await self.disconnect({'code': error_code})
            # # Or, if you need websocket_disconnect (eg. for autogroups), try this:
            #
            # try:
            #     await self.websocket_disconnect({'code': error_code})
            # except StopConsumer:
            #     pass

            # Try and close cleanly
            await self.close(error_code)

            raise

[wynshiter]

try:    
    await super().send(json_text_data)
except autobahn.exception.Disconnected:
    await self.disconnect()
    await self.close()

this kind of code can not solve the problem
Maybe the first situation upstairs mentioned：

1. send() erroring if the connection is closed before the write completes

Where can I set the buffer size?

i find same issue: #1835

[mikaraunio]

Repinging @carltongibson on this. Before work towards a potential fix, feel we would need decisions regarding the questions I raised in #1466 (comment), namely:

1. Is it appropriate for send() to cause an unhandled exception if the client disconnects before the write is finished
2. Should the disconnect() handler code be run even if an exception happens in receive()

I'm leaning towards No for 1 (disconnects can happen anytime, so why should the app get an exception for this particular case) and Yes for 2 (because otherwise, there's basically no way to do disconnect-time cleanup), but hesitant to look any further into this before an official take on how these should be handled.

[carltongibson]

@mikaraunio im still not sure, and still haven't had the time to ponder it sufficiently.

Yes, disconnects can happen any time, but I'm not really expecting the protocol server to raise an error whilst send an event to the application. Nor I'm expecting to send data during receive though either. So... 🤔

[mikaraunio]

Nor I'm expecting to send data during receive though either. So... 🤔

@carltongibson Not sure I understand what you mean here. Why would it be at all unexpected to send data from the receive() handler?

[carltongibson]

Well I guess I'm expecting to get a
message and process it... 🤔

If I'm going to send lots of data, I'm likely going to want to check for disconnect messages periodically, in order to know if I need to stop. I'm only replying quickly because you pinged me again, but, not sure how I can do that if all my logic is in receive. 🤔

But as I say, I haven't had the chance to sit down with this. (It's not an issue I've been stuck on.) of your proposals it's not clear to me which if any we should look into yet. That's likely not as helpful as you want.

[mikaraunio]

Do appreciate the quick reply.

You don't need to be sending lots of data – you're bound to hit this with a normal "get a message → process it → send reply or ack" cycle.

All you need is for a client to disconnect before send() is done, which is guaranteed to happen every now and then. That raises an exception, which causes the disconnect() handler to be skipped.

The end result is we can't currently implement any app logic that relies on stuff getting done in disconnect().