ASGIHandler doesn't handle http.disconnect if a request was sent before the disconnect

I have been working on possible solutions to ticket #33738

Accessing the queue where the receive is coming from is only possible because the tests in tests/asgi/ use the ApplicationCommunicator provided by asgiref.testing. This Communicator uses an asycio.queue.


 input_queue = receive.__self__._queue

to get the queue and using

 # Quit out if that's the end.
 if not input_queue and not message.get("more_body", None):

to check for when it’s empty to exit the while loop in read_body, seems to pass all the tests including the disconnect issue.

But this isn’t a good implementation as most backend web applications send one request at a time and don’t use queues that can be accessed from receive.

Trying to catch the errors thrown by message = await receive() has not been fruitful.

I feel catching an error when the request has fully been consumed, in other to exit the loop, is the right approach but I am a little bit confused as to how to go about catching the various async cancel & timeout errors and context var errors raised by SyncToAsync.thread_sensitive_context context manager.

I will like some guidance on how to approach this.

Hi @Th3nn3ss — thanks for looking at this.

Did you see my comment on the ticket about this? — Essentially, I think there are a couple of likely strategies.

The http.disconnect event comes from the server if the client connection is lost (for example). In Daphne, you can see it’s put onto the application’s recieve queue:

    def send_disconnect(self):
        Sends a http.disconnect message.
        Useful only really for long-polling.
        # If we don't yet have a path, then don't send as we never opened.
        if self.path:
            self.application_queue.put_nowait({"type": "http.disconnect"})

So, if this is sent, we can reasonably assume that the whole body has been read, the ASGIRequest has been constructed, and the view processing has begun.

We then have two options:

  1. Check in the request object if a disconnect has happened before doing something expensive.
  2. Concurrently listen in the ASGIHandler for a disconnect event whilst processing the view.

For 1, like the put_nowait() above, asyncio.Queue has a get_nowait that we could check in (e.g.) an is_disconnected method. A queue empty (expected) or any event other than http.disconnect (not sure how that happens TBH :thinking:) means False, an http.disconnect event means True (we could cache a True on the request).

Option 2 is more complex, but likely more what we’re after in an ideal world.

In ASGIHandler.handle(), after we’ve read the body_file — we have two task that we run together.

The first is the existing view dispatch.

The second calls await receive.get() and looks for the http.disconnect event. If it comes, we cancel the view dispatch task and exit. If view dispatch completes successfully, we cancel the disconnect listener task. (See Coroutines and Tasks — Python 3.11.2 documentation — Django 5.0 supports Python 3.10+, so no TaskGroup unless we consider a backport — but for a proof-of-concept whatever is easiest.)

Q: (with PoC in place) do we need to document checking for the CancelledError in the view to clean up? :thinking:

I think it’s worth looking at both these approaches. Option 1 is likely easy enough™. Option 2 is a bit more of a restructuring, but should be do-able.

I’ll cc @andrewgodwin here in case he has any thoughts as well.

Makes sense?

Yeah. Thanks for the reply. I love the example from the Daphne send_disconnect you shared. It really helps me understand better. This is an awesome reply.

1 Like

I made a Draft PR. Please checkout my implementation.

Submitted a possible fix for the issue using asyncio.wait_for to listen for a possible disconnect for 0.05 seconds and only proceeding to send a response to the client if the wait_for returns a TimeoutError else if a RequestAborted exception is raised due to an http.disconnect, the body_file is closed and handler returns control to the event loop.