[Django] #32049: Middleware behaviour inconsistent between runserver and daphne
Django
------------------------------------------------+------------------------
Reporter: Hugo Osvaldo Barrera | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: 3.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
All my views expect a particular middleware to have run (it detects the
current tenant based on the request's host).
I'm having an issue which ''only'' occurs on production, but I could not
reproduce in development mode (even with `DEBUG=False`) nor via unit
tests.
After just inspecting stack traces, it seems that when `handler404` is
run, behaviour varies between runserver and daphne.
Looking at the backtrace of my `handler404` function:
- When running `django-admin runserver 0.0.0.0:8000`, all my middlewares
are in my backtrace, meaning they've gotten executed and the function is
called in their context.
- When running `daphne myapp.asgi:application --bind 0.0.0.0 --port 8000
--http-timeout 90`, I put a tracepoint in my `handler404`, and the
middleware **are not** in the backtrace. It seems that the view is run
outside the context of these middlewares.
I haven't been able to find anything in the docs clarifying whether error
handlers should expect middleware to have run successfully, but whatever
the ''intended'' behaviour is, it should be consistent across the two.
--
Ticket URL: <https://code.djangoproject.com/ticket/32049>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Hugo Osvaldo | Owner: nobody
Barrera |
Type: Bug | Status: new
Component: HTTP handling | Version: 3.1
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Hugo Osvaldo Barrera):
This reproduces the issue. Hope it's clear enough.
https://github.com/WhyNotHugo/django-32049
--
Ticket URL: <https://code.djangoproject.com/ticket/32049#comment:2>
Django
Reporter: Hugo Osvaldo | Owner: nobody
Barrera |
Component: HTTP handling | Version: 3.1
Keywords: ASGI | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* cc: Andrew Godwin (added)
* keywords: => ASGI
* status: new => closed
* resolution: => invalid
Comment:
This is an interesting one. The issue is the way the ASGIHandler runs your
code in separate threads. (The bottom-line will be that what you're trying
to do isn't supported: you're going to have to find a thread-safe way of
doing this if you want to use ASGI here, now.)
Your middleware sets `connection.tenant` in Thread 1. Via the
`DefaultConnectionProxy` this fetches and sets the property on the
`"default"` connection for Thread 1.
Your view is then run on Thread 2. Attempting to get `connection.tenant`
the `DefaultConnectionProxy` fetches the `"default"` connection for
Thread 2.
Since DB connections are not shared between thread, this is not the same
connection, so the `tenant` attribute you set doesn't exist.
I'll cc Andrew since it's an interesting case, but it's expected
behaviour. ASGI/async implies/requires much more thought about what thread
you're going to be running on here, and hence how you pass this kind of
info around.
--
Ticket URL: <https://code.djangoproject.com/ticket/32049#comment:3>
Django
ASGI.
-------------------------------------+-------------------------------------
Reporter: Hugo Osvaldo | Owner: nobody
Barrera |
Type: Bug | Status: closed
Keywords: ASGI | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Carlton Gibson:
Old description:
> All my views expect a particular middleware to have run (it detects the
> current tenant based on the request's host).
>
> I'm having an issue which ''only'' occurs on production, but I could not
> reproduce in development mode (even with `DEBUG=False`) nor via unit
> tests.
>
> After just inspecting stack traces, it seems that when `handler404` is
> run, behaviour varies between runserver and daphne.
>
> Looking at the backtrace of my `handler404` function:
>
> - When running `django-admin runserver 0.0.0.0:8000`, all my middlewares
> are in my backtrace, meaning they've gotten executed and the function is
> called in their context.
> - When running `daphne myapp.asgi:application --bind 0.0.0.0 --port 8000
> --http-timeout 90`, I put a tracepoint in my `handler404`, and the
> middleware **are not** in the backtrace. It seems that the view is run
> outside the context of these middlewares.
>
> I haven't been able to find anything in the docs clarifying whether error
> handlers should expect middleware to have run successfully, but whatever
> the ''intended'' behaviour is, it should be consistent across the two.
New description:
(Was: Middleware behaviour inconsistent between runserver and daphne)
All my views expect a particular middleware to have run (it detects the
current tenant based on the request's host).
I'm having an issue which ''only'' occurs on production, but I could not
reproduce in development mode (even with `DEBUG=False`) nor via unit
tests.
After just inspecting stack traces, it seems that when `handler404` is
run, behaviour varies between runserver and daphne.
Looking at the backtrace of my `handler404` function:
- When running `django-admin runserver 0.0.0.0:8000`, all my middlewares
are in my backtrace, meaning they've gotten executed and the function is
called in their context.
- When running `daphne myapp.asgi:application --bind 0.0.0.0 --port 8000
--http-timeout 90`, I put a tracepoint in my `handler404`, and the
middleware **are not** in the backtrace. It seems that the view is run
outside the context of these middlewares.
I haven't been able to find anything in the docs clarifying whether error
handlers should expect middleware to have run successfully, but whatever
the ''intended'' behaviour is, it should be consistent across the two.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32049#comment:4>
Django
ASGI.
-------------------------------------+-------------------------------------
Reporter: Hugo Osvaldo | Owner: nobody
Barrera |
Type: Bug | Status: closed
Keywords: ASGI | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Hugo Osvaldo Barrera):
You mention this is expected behaviour, but which one? `runserver` does
one thing, but `daphne` does another.
If one of them is behaving as expected, then the other is misbehaving,
right? I don't think the intent is for them to run different, is it?
Side note:
I find it curious that for non-404 views, the same db connection is used
for both the middleware and the view. The exception seems to _only_ how
the 404 view is called.
> you're going to have to find a thread-safe way of doing this if you want
to use ASGI here, now
Any hints on something that's safe?
What I'm doing right now is setting a `tenant` based on the domain name,
and this is used in various places (e.g.: by my custom staticfiles'
`Storage`).
--
Ticket URL: <https://code.djangoproject.com/ticket/32049#comment:5>
Django
ASGI.
-------------------------------------+-------------------------------------
Reporter: Hugo Osvaldo | Owner: nobody
Barrera |
Type: Bug | Status: closed
Keywords: ASGI | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
`runserver` doesn't yet support ASGI, so it's running as a WSGI app in a
single thread.
--
Ticket URL: <https://code.djangoproject.com/ticket/32049#comment:6>
Django
ASGI.
-------------------------------------+-------------------------------------
Reporter: Hugo Osvaldo | Owner: nobody
Barrera |
Type: Bug | Status: closed
Keywords: ASGI | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Hugo Osvaldo Barrera):
Ah, right, makes sense. I'll be sure to do testing of this kind of
functionality with daphne too then.
Thanks a lot for your feedback/insight here.
It should be noted that, apparently, since both unittests and runserver
run synchronously, these bugs are very hard to pick up -- for me they
_only_ occur on the 404 page, and on production.
Do you have any hints on how to safely do what I'm trying to do? E.g.:
keep a pseudo-global state isolated to the context of a single request?
--
Ticket URL: <https://code.djangoproject.com/ticket/32049#comment:7>
Django
ASGI.
-------------------------------------+-------------------------------------
Reporter: Hugo Osvaldo | Owner: nobody
Barrera |
Type: Bug | Status: closed
Keywords: ASGI | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Andrew Godwin):
It's worth noting that I still consider this a bug since it's a type of
failure, but it's already been fixed in asgiref:
https://github.com/django/asgiref/commit/7becc9daca2628c46af1cb7e46b4c47c1ea27adf
As of the next release, that will enforce that all synchronous calls run
inside the same, single thread, slightly hurting performance but crucially
meaning that this issue would not have happened.
--
Ticket URL: <https://code.djangoproject.com/ticket/32049#comment:9>
Django
ASGI.
-------------------------------------+-------------------------------------
Reporter: Hugo Osvaldo | Owner: nobody
Barrera |
Type: Bug | Status: closed
Keywords: ASGI | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
Hey Andrew. I hadn't seen that go in. It's a good move I think but, do you
think it would be worth a post on the Django blog highlighting the change
in behaviour for people?
--
Ticket URL: <https://code.djangoproject.com/ticket/32049#comment:10>