search

jazzband / django-debug-toolbar

A configurable set of panels that display various debug information about the current request/response.
https://django-debug-toolbar.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
7.97k stars 1.03k forks source link

fix Exception while resolving variable 'toolbar' in template #1906

Closed foundkey closed 2 months ago

foundkey commented 2 months ago

Description

When sending a request using ajax, toolbar.js will send a request at the same time: GET /__debug__/history_sidebar/?store_id=<id> HTTP/1.1

This request will throw the following exception when rendering the template. It seems because toolbar is not passed in panel_context.

Fixes # (#1905 )

  1. add toolbar into panel_context.

Checklist:

matthiask commented 2 months ago

I think this makes sense, but I'm not 100% sure if you're just fixing an issue with a missing variable or if the functionality of loading panel contents with RENDER_PANELS = True and older requests was broken previously.

If it's the former I'm good with the PR, but if it's the later we'd really want a test to make sure we're not regressing here.

foundkey commented 2 months ago

I think this makes sense, but I'm not 100% sure if you're just fixing an issue with a missing variable or if the functionality of loading panel contents with RENDER_PANELS = True and older requests was broken previously.

If it's the former I'm good with the PR, but if it's the later we'd really want a test to make sure we're not regressing here.

My local test was fine. After modification, no exception logs can be seen in the same scenario. However, I only tested the scenarios I encountered, and I did not conduct a comprehensive test.

foundkey commented 2 months ago

Thanks for identifying a solution! Would you be able to include a test that confirms that this is fixed? It's hard to accept a change without being able to see what it's fixing.

Sorry, this test doesn't look easy to add.

According to the Django Template rendering process, the exception django.template.base.VariableDoesNotExist is handled gracefully. This exception is not thrown and is replaced with a default value, which is an empty string if not configured.

template_resolve

This results in an empty string being obtained at the exception and going to the false branch, which is in line with the expected logic.

debug-toolbar-exception

According to debug-toolbar's code: if panel_content.html is rendered under a non-ajax request, there will be a toolbar variable in the context, which is normal. If panel_content.html is rendered under an ajax request, the toolbar variable is missing from the context, eventually leading to the false branch. In ajax request, toolbar.should_render_panels value is always false.

It seems that this exception does not trigger any problems. Only in debug level logs, you will see relevant error logs. To sum up, it won't hurt if this anomaly is not fixed.

foundkey commented 2 months ago

After turning on Debug level logging, VariableDoesNotExist errors appear frequently during use. But it does not affect the use. I think my PR is redundant. It can be closed.