[Django] #34062: object HttpResponseNotAllowed can't be used in 'await' expression

[Django]

#34062: object HttpResponseNotAllowed can't be used in 'await' expression
-------------------------------------------+------------------------
Reporter: Antoine Lorence | Owner: nobody
Type: Bug | Status: new
Component: Generic views | Version: 4.1
Severity: Normal | Keywords: async
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------------+------------------------
When defining a simple View subclass with only an async "post" method, GET
requests to this view cause the following exception:

{{{
[29/Sep/2022 07:50:48] "GET /demo HTTP/1.1" 500 81134
Method Not Allowed (GET): /demo
Internal Server Error: /demo
Traceback (most recent call last):
File "/home/alorence/.cache/pypoetry/virtualenvs/dj-bug-demo-FlhD0jMY-
py3.10/lib/python3.10/site-packages/django/core/handlers/exception.py",
line 55, in inner
response = get_response(request)
File "/home/alorence/.cache/pypoetry/virtualenvs/dj-bug-demo-FlhD0jMY-
py3.10/lib/python3.10/site-packages/django/core/handlers/base.py", line
197, in _get_response
response = wrapped_callback(request, *callback_args,
**callback_kwargs)
File "/home/alorence/.cache/pypoetry/virtualenvs/dj-bug-demo-FlhD0jMY-
py3.10/lib/python3.10/site-packages/asgiref/sync.py", line 218, in
__call__
return call_result.result()
File "/usr/lib/python3.10/concurrent/futures/_base.py", line 451, in
result
return self.__get_result()
File "/usr/lib/python3.10/concurrent/futures/_base.py", line 403, in
__get_result
raise self._exception
File "/home/alorence/.cache/pypoetry/virtualenvs/dj-bug-demo-FlhD0jMY-
py3.10/lib/python3.10/site-packages/asgiref/sync.py", line 284, in
main_wrap
result = await self.awaitable(*args, **kwargs)
TypeError: object HttpResponseNotAllowed can't be used in 'await'
expression
}}}

This can be easily reproduced with an empty project (no external
dependencies) started with Django 4.1.1 and python 3.10.6.

Basic view to reproduce the bug:

{{{
from django.views import View
from django.http import HttpResponse

class Demo(View):
"""This basic view supports only POST requests"""
async def post(self, request):
return HttpResponse("ok")
}}}

URL pattern to access it:
{{{
from django.urls import path

from views import Demo

urlpatterns = [
path("demo", Demo.as_view()),
]
}}}

Start the local dev server (manage.py runserver) and open
http://127.0.0.1:8000/demo in the browser.
Server crash with 500 error with the given traceback.

--
Ticket URL: <https://code.djangoproject.com/ticket/34062>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#34062: object HttpResponseNotAllowed can't be used in 'await' expression

---------------------------------+--------------------------------------

Reporter: Antoine Lorence | Owner: nobody
Type: Bug | Status: new
Component: Generic views | Version: 4.1

Severity: Normal | Resolution:

Keywords: async | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

---------------------------------+--------------------------------------
Changes (by Antoine Lorence):

* Attachment "Capture d’écran du 2022-09-29 09-54-36.png" added.

[Django]

#34062: object HttpResponseNotAllowed can't be used in 'await' expression
---------------------------------+------------------------------------

Reporter: Antoine Lorence | Owner: nobody
Type: Bug | Status: new
Component: Generic views | Version: 4.1

Severity: Release blocker | Resolution:
Keywords: async | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------

Changes (by Carlton Gibson):

* severity: Normal => Release blocker
* stage: Unreviewed => Accepted

Comment:

Yes, looks right.

`http_method_not_allowed()` needs to be adjusted to handle both sync and
async cases in the same way as `options()`

Do you have capacity to do a patch quickly? (Otherwise I'll take it on.)

Thanks for the report!

Regression in 9ffd4eae2ce7a7100c98f681e2b6ab818df384a4.

--
Ticket URL: <https://code.djangoproject.com/ticket/34062#comment:1>

[Django]

#34062: object HttpResponseNotAllowed can't be used in 'await' expression
---------------------------------+------------------------------------
Reporter: Antoine Lorence | Owner: nobody
Type: Bug | Status: new
Component: Generic views | Version: 4.1
Severity: Release blocker | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------

Comment (by Antoine Lorence):

Thank you very much for your confirmation.

I've never contributed to Django codebase, but the fix for this issue
seems obvious. I think this is a good occasion to contribute for the first
tme.
I will follow contributions guide and try to provide a regression test and
a patch for that issue.

--
Ticket URL: <https://code.djangoproject.com/ticket/34062#comment:2>

[Django]

#34062: object HttpResponseNotAllowed can't be used in 'await' expression

-------------------------------------+-------------------------------------
Reporter: Antoine Lorence | Owner: Antoine
| Lorence
Type: Bug | Status: assigned

Component: Generic views | Version: 4.1
Severity: Release blocker | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Antoine Lorence):

* owner: nobody => Antoine Lorence
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/34062#comment:3>

[Django]

#34062: object HttpResponseNotAllowed can't be used in 'await' expression
-------------------------------------+-------------------------------------
Reporter: Antoine Lorence | Owner: Antoine
| Lorence
Type: Bug | Status: assigned
Component: Generic views | Version: 4.1
Severity: Release blocker | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Carlton Gibson):

Great, welcome aboard Antoine!

I pushed a draft here
https://github.com/django/django/compare/main...carltongibson:django:4.1.2/ticket-34062
that you can use for inspiration.

If you open a PR on GitHub, I'm happy to advise.

Normally there's no rush here, but we have releases due for the beginning
of next week, and as a regression this needs to go in. As such, if you hit
any barriers please reach out so I can help out. Thanks 🏅

--
Ticket URL: <https://code.djangoproject.com/ticket/34062#comment:4>

[Django]

#34062: object HttpResponseNotAllowed can't be used in 'await' expression
-------------------------------------+-------------------------------------
Reporter: Antoine Lorence | Owner: Antoine
| Lorence
Type: Bug | Status: assigned
Component: Generic views | Version: 4.1
Severity: Release blocker | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Antoine Lorence):

Wow, your draft is almost exactly what I already written in the codebase.

I ended with the exact same modification in View.http_method_not_allowed()
(to the letter).
I also written the almost same test in "async/tests" (I didn't use
RequestFactory, your version is better then).

I was currently looking into "asgi/tests" to check if I can add a full
request-response lifecycle test in such case, but this appear to be more
complicated. Do you think this is feasible / required ?

--
Ticket URL: <https://code.djangoproject.com/ticket/34062#comment:5>

[Django]

#34062: object HttpResponseNotAllowed can't be used in 'await' expression
-------------------------------------+-------------------------------------
Reporter: Antoine Lorence | Owner: Antoine
| Lorence
Type: Bug | Status: assigned
Component: Generic views | Version: 4.1
Severity: Release blocker | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Carlton Gibson):

It's feasible, but I don't think it's required.

(We already test the full dispatch in many places elsewhere. What we're
looking for here is the `http_method_not_allowed()` is correctly adapted
when the view is async (i.e. has async handlers).

Make sense?

--
Ticket URL: <https://code.djangoproject.com/ticket/34062#comment:6>

[Django]

#34062: object HttpResponseNotAllowed can't be used in 'await' expression
-------------------------------------+-------------------------------------
Reporter: Antoine Lorence | Owner: Antoine
| Lorence
Type: Bug | Status: assigned
Component: Generic views | Version: 4.1
Severity: Release blocker | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Antoine Lorence):

That totally makes sense. What is the next step now ?

Your draft is perfect, more complete than mine (I didn't write into
release notes).
I think you can merge your branch django:4.1.2/ticket-34062.

--
Ticket URL: <https://code.djangoproject.com/ticket/34062#comment:7>

[Django]

#34062: object HttpResponseNotAllowed can't be used in 'await' expression
-------------------------------------+-------------------------------------
Reporter: Antoine Lorence | Owner: Antoine
| Lorence
Type: Bug | Status: assigned
Component: Generic views | Version: 4.1
Severity: Release blocker | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Carlton Gibson):

I'd like to give you the chance to contribute if you'd like to do that![0]

You've made the report. You've investigated.
If you'd like to make the PR, please do!

You can pull any parts from my branch. If you'd like you can add `Co-
authored-by: Carlton Gibson <carlton...@noumenal.es>` at the bottom of
the commit message (but that's not necessary 🙂)

[0]: This is entirely selfish on my part — once you've done 1 PR, you're
ever so much more likely to do another 🙂

--
Ticket URL: <https://code.djangoproject.com/ticket/34062#comment:8>

[Django]

#34062: object HttpResponseNotAllowed can't be used in 'await' expression
-------------------------------------+-------------------------------------
Reporter: Antoine Lorence | Owner: Antoine
| Lorence
Type: Bug | Status: assigned
Component: Generic views | Version: 4.1
Severity: Release blocker | Resolution:
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Antoine Lorence):

Thank for your support. The PR is now open:
https://github.com/django/django/pull/16136

--
Ticket URL: <https://code.djangoproject.com/ticket/34062#comment:9>

[Django]

#34062: object HttpResponseNotAllowed can't be used in 'await' expression
-------------------------------------+-------------------------------------
Reporter: Antoine Lorence | Owner: Antoine
| Lorence
Type: Bug | Status: assigned
Component: Generic views | Version: 4.1
Severity: Release blocker | Resolution:
Keywords: async | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Antoine Lorence):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/34062#comment:10>

[Django]

#34062: object HttpResponseNotAllowed can't be used in 'await' expression
-------------------------------------+-------------------------------------
Reporter: Antoine Lorence | Owner: Antoine
| Lorence
Type: Bug | Status: assigned
Component: Generic views | Version: 4.1
Severity: Release blocker | Resolution:

Keywords: async | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Carlton Gibson):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/34062#comment:11>

[Django]

#34062: object HttpResponseNotAllowed can't be used in 'await' expression
-------------------------------------+-------------------------------------
Reporter: Antoine Lorence | Owner: Antoine
| Lorence

Type: Bug | Status: closed

Component: Generic views | Version: 4.1

Severity: Release blocker | Resolution: fixed

Keywords: async | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Carlton Gibson <carlton@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"9b0c9821ed4dd9920cc7c5e7b657720d91a89bdc" 9b0c9821]:
{{{
#!CommitTicketReference repository=""
revision="9b0c9821ed4dd9920cc7c5e7b657720d91a89bdc"
Fixed #34062 -- Updated View.http_method_not_allowed() to support async.

As with the options() methods, wrap the response in a coroutine if
the view is async.

Co-authored-by: Carlton Gibson <carlton...@noumenal.es>
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/34062#comment:12>

[Django]

#34062: object HttpResponseNotAllowed can't be used in 'await' expression
-------------------------------------+-------------------------------------
Reporter: Antoine Lorence | Owner: Antoine
| Lorence
Type: Bug | Status: closed
Component: Generic views | Version: 4.1
Severity: Release blocker | Resolution: fixed
Keywords: async | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Carlton Gibson <carlton.gibson@…>):

In [changeset:"ecf6506f44cd09021927a821eb14e3f7f726ab14" ecf6506f]:
{{{
#!CommitTicketReference repository=""
revision="ecf6506f44cd09021927a821eb14e3f7f726ab14"
[4.1.x] Fixed #34062 -- Updated View.http_method_not_allowed() to support
async.

As with the options() methods, wrap the response in a coroutine if
the view is async.

Co-authored-by: Carlton Gibson <carlton...@noumenal.es>

Backport of 9b0c9821ed4dd9920cc7c5e7b657720d91a89bdc from main
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/34062#comment:13>