jrobichaud/django-structlog

Possible issue with async detection

totycro opened this issue · 6 comments

totycro commented

I'm not sure if I completely understand the situation and whether this is a configuration issue on my side, but I wanted to raise it since it could also be interesting for other users.

If I use the middleware like this, I think that the sync middleware is activated because the other middleware uses sync as well.

MIDDLEWARE = [
    "django_structlog.middlewares.request.RequestMiddleware",
    "django.middleware.clickjacking.XFrameOptionsMiddleware",
]

However if I reorder them like this:

MIDDLEWARE = [
    "django.middleware.clickjacking.XFrameOptionsMiddleware",
    "django_structlog.middlewares.request.RequestMiddleware",
]

I get the following error

data_1  | │ /usr/local/lib/python3.11/site-packages/django/middleware/clickjacking.py:27 │
data_1  | │ in process_response                                                          │
data_1  | │                                                                              │
data_1  | │   24 │                                                                       │
data_1  | │   25 │   def process_response(self, request, response):                      │
data_1  | │   26 │   │   # Don't set it if it's already in the response                  │
data_1  | │ ❱ 27 │   │   if response.get("X-Frame-Options") is not None:                 │
data_1  | │   28 │   │   │   return response                                             │
data_1  | │   29 │   │                                                                   │
data_1  | │   30 │   │   # Don't set it if they used @xframe_options_exempt              │
data_1  | │                                                                              │
data_1  | │ ╭───────────────────────────────── locals ─────────────────────────────────╮ │
data_1  | │ │  request = <ASGIRequest: GET '/probe'>                                   │ │
data_1  | │ │ response = <coroutine object AsyncRequestMiddleware.__call__ at          │ │
data_1  | │ │            0x7f20a4935d20>                                               │ │
data_1  | │ │     self = <XFrameOptionsMiddleware                                      │ │
data_1  | │ │            get_response=convert_exception_to_response.<locals>.inner>    │ │
data_1  | │ ╰──────────────────────────────────────────────────────────────────────────╯ │
data_1  | ╰──────────────────────────────────────────────────────────────────────────────╯
data_1  | AttributeError: 'coroutine' object has no attribute 'get'
data_1  | 
data_1  | [2023-02-07 14:33:55 +0000] [44] [ERROR] Exception in ASGI application
data_1  | Traceback (most recent call last):
data_1  |   File "/usr/local/lib/python3.11/site-packages/uvicorn/protocols/http/httptools_impl.py", line 419, in run_asgi
data_1  |     result = await app(  # type: ignore[func-returns-value]
data_1  |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
data_1  |   File "/usr/local/lib/python3.11/site-packages/uvicorn/middleware/proxy_headers.py", line 78, in __call__
data_1  |     return await self.app(scope, receive, send)
data_1  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
data_1  |   File "/usr/local/lib/python3.11/site-packages/django/core/handlers/asgi.py", line 155, in __call__
data_1  |     await self.handle(scope, receive, send)
data_1  |   File "/usr/local/lib/python3.11/site-packages/django/core/handlers/asgi.py", line 178, in handle
data_1  |     response = await self.get_response_async(request)
data_1  |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
data_1  |   File "/usr/local/lib/python3.11/site-packages/django/core/handlers/base.py", line 162, in get_response_async
data_1  |     response = await self._middleware_chain(request)
data_1  |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
data_1  | TypeError: object HttpResponse can't be used in 'await' expression

I think this is because RequestMiddleware detects the actual view to be async here, but then the other middleware expects the response not to be a coroutine.

For now I'm working around that by using django_structlog.middlewares.request.SyncRequestMiddleware, but do you think that this is an issue with other middlewares or with this middleware, or is there some kind of config to be set such that all middlewares use async?

jrobichaud commented

Are you using any async views in your project?

I was following django's documentation and they might not having considered some edge cases.

I will roll back the breaking change and document it separately as a BETA feature.

totycro commented

Yes, I'm using some async views, although I'm not sure if I've set up everything correctly yet. However I did not have this issue with the previous version of django-structlog.

  • 👍1
totycro commented

In case it's helpful, I've found a similar issue in another project: jazzband/django-hosts#119

jrobichaud commented

I think its a bug with XFrameOptionsMiddleware

The class inherit from MiddlewareMixin

class XFrameOptionsMiddleware(MiddlewareMixin):
    #...

Which is defined as:

class MiddlewareMixin:
    sync_capable = True
    async_capable = True
    #...

It works if I make and use this custom middleware:

class SyncXFrameOptionsMiddleware(XFrameOptionsMiddleware):
    sync_capable = True
    async_capable = False
smallfish06 commented

Also having same problem, using it with ASGI seems causing problem. if i change into WSGI, seems like working

In this case both using async view

jrobichaud commented

I rolled back the change in 4.1.1.

You can switch back again to RequestMiddleware.

  • 👍2