ASGI http.disconnect not handled for streaming responses

[Sam Toyer]

The fix to #33738 (ticket) correctly handles HTTP disconnects before a view exits (i.e. returns a response object), but does not handle disconnects between the time when the view returns and the time when the response actually finishes. This is a particular problem for long-lived StreamingHttpResponses (e.g. for SSE).

The issue can be solved by extending the original cancellation/disconnect-handling logic to also cover AsgiHandler.send_response(). Example tests and a fix are in this commit: ​https://github.com/qxcv/django/commit/ed2f9ad03a3abbc636fd5fa590d69ba36571c158

I'm happy to file a PR if this is wanted upstream.

[Mariusz Felisiak]

This seems to be a bug in the new feature #33738 (related to the #33735).

[Carlton Gibson]

Hey Sam, thanks for the report. Yes, please do open a PR! (It being the weekend, I'd like to have a look at this properly once the PR is open.)

It would be nice to check that the cancelled error was handled explicitly, ​e.g. in PR for #33738 so similar in the streaming response iterator, and maybe add an example similar to the ​async views disconnect example to the ​StreamingHttpResponse docs.

[Carlton Gibson]

Hey Sam, no pressure but can I ask, do you have capacity to submit a PR here? (This is a Release Blocker for Django 5.0, so needs to be resolved before the feature freeze at the beginning of September) Thanks!

[Sam Toyer]

Yes, I can do this. There was an error in my original attempt at fixing it, so I've been holding off on submitting a PR until I'm confident the new fix works okay. I should have time to add some more tests and submit the PR tomorrow, though.

[Carlton Gibson]

Great. Thanks. 🎁

[Sam Toyer]

Filed a PR: ​https://github.com/django/django/pull/17147

One subtlety: I originally made a smaller modification to the handler that put view logic and response logic in the same coroutine, but otherwise followed the same flow. I found this broke in practice because sometimes both coroutines would return at once. This caused the previous pending.pop()/done.pop() logic to error out because one of those two sets would be empty. I think that this problem is particularly likely to happen in situations where some coroutines are long-lived. I haven't added a test for this edge case yet, though.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 64cea1e:

Fixed #34752 -- Fixed handling ASGI http.disconnect for streaming responses.