jazzband
Document that the toolbar does not support async functionality
We should include in our readme that the application doesn't not support async functionality and that effort is under development. If possible, it'd be great to detect that async functionality is being used and warn the user that the toolbar won't be effective. @michaelurban suggested this in https://github.com/jazzband/django-debug-toolbar/issues/1430#issuecomment-1697910837
Hello!
I would like to implement showing warning message if DebugToolbarMiddleware is called in async environment.
I suppose the easiest way to check if async is enabled is something like this:
from django.core.handlers.asgi import ASGIRequest
...
class DebugToolbarMiddleware:
...
def __call__(self, request):
# Decide whether the toolbar is active for this request.
show_toolbar = get_show_toolbar()
if not show_toolbar(request) or DebugToolbar.is_toolbar_request(request):
return self.get_response(request)
if type(request) == ASGIRequest:
self._warn_djdt_asgi_uneffective()
...
What do you think about this approach? I guess there are some important thinks about compability, but general idea is the same.
What is the optimal way to warn developer? At this point I think about log a warning message, but may be it's something else.
@denisSurkov what do you think of using iscoroutinefunction(get_response) similar to https://docs.djangoproject.com/en/4.2/topics/http/middleware/#asynchronous-support?
@tim-schilling , yes, this is an option
I can do something like this:
from inspect import iscoroutinefunction
...
class DebugToolbarMiddleware:
def __init__(self, get_response):
self.get_response = get_response
if iscoroutinefunction(get_response):
self._warn_djdt_asgi_uneffective()
This seems cleaner I agree
I'm not completely sure that works, since Django automatically wraps get_response to be either a coroutine or not depending on whether the current middleware (and maybe other middlewares in the stack) support async or not.
The note here is especially relevant:
https://docs.djangoproject.com/en/5.0/topics/http/middleware/#async-middleware
If you declare a hybrid middleware that supports both synchronous and asynchronous calls, the kind of call you get may not match the underlying view. Django will optimize the middleware call stack to have as few sync/async transitions as possible.
So, as long as the DebugToolbarMiddleware doesn't officially support async we won't ever get a get_response. At least that's my understanding of where we are now.
@matthiask , you are right.
Currently DegubToolbarMiddleware is sync_only. There are two attributes django checking (and does fallback) for every middleware:
sync_capable = True
async_capable = False
and DebugToolbarMiddleware is sync_capable only.
So async get_response would not be an option.
For this feature I see two options now:
-
Adding flags (and also refactoring for new way middlewares work with process_view and etc, but it may break compability) and checking with
iscoroutinefunction; -
Checking type of request - we could do it one time, because once ASGIRequest came to middleware next requests would have the same type I guess.
I think checking the type of the request may be fine?
I'm thinking if there are other signals such as the presence of an event loop (https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.get_running_loop), but it seems to me that "just" doing a type check may be the most straightforward option for now. I didn't spend much time thinking this through just now so that's not really a well researched opinion.
That's okey
I'll try to make an MVP in a couple of days.
@tim-schilling , I still have question about how to warn a developer? Just printing a message or logging it somehow? If DJDT has way to log it, can you point the place?
Also I think making tests for it would be challenging
That's a good question. I think there are a number of issues that would benefit from a warnings/alerts panel (#1435, #1682). This could also potentially go in there too.
I think for now let's keep it easy and log it out with logger.info().
To test it you should be able to patch logger, then use logger.info.assert_called_once_with("the warning goes here")
@matthiask what do you think about logging vs some sort of panel to render these messages?
I'm not sure. We have removed the logging panel some time ago and I'm not sure we want to reintroduce it since we had so many hard to find bugs around logging.
Re. warnings: Maybe it would be nice to show it inside the panel, but I also think something like a multiline and really loud and obnoxious warning (something with borders and ASCII art 😅) would be a good fit for this issue. It's not similar to a warning or anything of the sort: For the time being the toolbar is of very limited use when running an async server.
I just checked and Django also has a similar thing, when doing stuff which is probable to cause problems down the road: https://github.com/django/django/blob/10c7c7320baf1c655fcb91202169d77725c9c4bd/setup.py#L34-L55
Hmm. I wasn't thinking something to show all calls to logger.log(). But instead there would be a series of checks/validations that the toolbar could perform and inform the user. Such as "Hey, this particular form has a file input, but the form doesn't have enctype="multipart/form-data"" set.
@tim-schilling ,
To test it you should be able to patch logger, then use logger.info.assert_called_once_with("the warning goes here")
Testing logging is not a problem. I'm worried about how to pass ASGIRequest. I can construct a request directly, but this seems like a hack. So I was thinking about running ASGIHandler with async_to_sync in tests.