To keep or not to keep: logging of undefined template variables

[Tim Graham]

Ticket #18773 [0] added logging of undefined template variables in Django 1.9 [1], however, I've seen several reports of users finding this logging more confusing than helpful. For example, admin templates log errors about missing is_popup variables [2] which is how the template are designed (is_popup is only in the contexts of pop ups) and the TECHNICAL_404_TEMPLATE also logs errors without any obvious solution about how to prevent that [3].

I'm thinking it might be better to remove this noisy, generally unhelpful logging. What do you think?

[0] https://code.djangoproject.com/ticket/18773
[1] https://github.com/django/django/commit/dc5b01ad05e50ccde688c73c2ed3334a956076b0
[2] https://groups.google.com/d/topic/django-users/6Ve9dcv23sI/discussion
[3] https://code.djangoproject.com/ticket/26886

[Tim Martin]

I don't have any objection to removing this. However, the thought occurs that my patch to the handling of undefined variables could enhance this to make it more useful. I think we could log in the case where a conditional expression actually depends on an undefined variable. I haven't thought about it properly, but presumably the expression evaluation can even take account of short-circuiting, so that:

  {% if defined_value or some_undefined_value %}

correctly warns only in the case where defined_value is false. Of course, this won't help in the case of the admin template example you gave, where the conditional depends directly on the undefined value.

The obvious drawback is that I think my patch is going to be abandoned, so I think this is a purely theoretical possibility. But if anyone likes the sound of it I can think through it a little more carefully.

Tim

[Adam Johnson]

Doesn't sound like there's much impetus either way, and it's confusing people, so I'd say remove it. Or maybe make it optional, default off?

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/5cf29e82-19d7-4567-b537-21a77faf88a9%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Adam

[Luke Plant]

My general policy is that warnings should only be logged if there is a good way to silence them. A good way means:

1) fix the bug which is causing them (only applies if there is genuinely a bug or bad practice that should be fixed)

2) disable the warning in a fine tuned way i.e. you can indicate, without much work or hackiness, *this* instance generating the warning is actually fine.

Otherwise, these warnings are useless or worse - people often:

* just turn off all warnings of this type because they are too noisy (so they become useless)

* or end up turning off even more than necessary (hurting the project)

* or miss genuine issues because of the noise.

It sounds like these warnings are of the unhelpful kind.

For implementing option 2, the only thing I could think of is extending the template syntax with something that indicates you are expecting the value to be missing e.g.:

   {% if foo? %}

   {{ foo? }}

This would clearly be a big change though.

Luke

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.

Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/a428f373-ef0c-4575-8a84-69a4beda154c%40googlegroups.com.

[Melvyn Sopacua]

On Thursday 16 March 2017 12:03:07 Tim Graham wrote:

> Ticket #18773 [0] added logging of undefined template variables in

> Django 1.9 [1], however, I've seen several reports of users finding

> this logging more confusing than helpful.

 

With channels hitting 2.0 and the already large stack of moving parts surrounding Django you need some basic system administration skills and programming experience to work with the system. And there are quite a few examples to link to from the user's list that deal with those moving parts rather then Django itself. It is not an application that you download, install and run.

 

An introduction "What you need to know before starting Django" would help a lot in this respect and explaining the noisiness of some logging belongs in there.

 

Because it *is* useful if you defined that variable to True in your settings, and it's working in all projects but this one. It could be there's an extra piece of context middleware that uses the same name and deletes the variable from the context. It could be there's a Mixin missing in the view hierarchy. Or a typo you don't notice anymore after plowing through 20+ included template bits.

 

Noisy logging is exactly what you want when debugging. It should log things that may be working as designed, especially things that are ambiguous (like undefined and false).

 

Another thing is that logging is the ugly duckling of Django. It's not mentioned much if at all in the tutorial. It is not mentioned at all in "How to write reusable apps" and it shows in the eco system. It's like finding a diamond when an app actually has logging implemented.

 

But it also means that novice users touching the LOGGING configuration are exceptions and I don't think Django should cater to the exceptions.

 

--

Melvyn Sopacua

[Vlastimil Zíma]

-1 to the removal. I was annoyed by the logging at first, but then I started to clean individual logs. Half way through, I found several usages of long removed variables, one unused template (as a side effect) and I updated several views to always provide defined context variables.

All in all, I consider the warnings very useful for a cleaning, though I wouldn't be against an option to silence them. Which can already by accomplished by LOGGING, can't it?

Vlastik

Dne neděle 26. března 2017 11:14:23 UTC+2 Melvyn Sopacua napsal(a):

[Anthony King]

-1 for removing logs. Like Vlastimil, it's helped me spot a couple of stray bugs.

What I'd actually like to see is this becoming stricter, with the end goal of errors raising when using undefined variables.

For the verbosity, perhaps there's a middle ground? only log once per variable access per template context, and provide a formatter that will clean up the output?

I believe in debug mode, you have access to line numbers and character positions, so the final output could look something like this:

``

some_app/home.html:32:24: Undefined variable: missing_variable

``

I'm unsure how much effort this would take, but it would definitely make the logging a lot more user + developer friendly.

--

You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.

Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/80fafa13-ef3b-4b8c-9376-cc63b1776cb8%40googlegroups.com.

[Tim Graham]

We received a report that shows the large number of undefined variable warnings when rendering an admin changelist page [0].

I'm still not sure what the solution should be, but I created #28526 [1] to track this problem: finding a remedy to the problem of verbose, often unhelpful logging of undefined variables.

I don't think "the end goal of errors raising when using undefined variables" is feasible. My sense is that relying on the behavior of undefined variables is too entrenched in the Django template language to change it at this point. (If someone wanted to try to fix all the warnings in the admin templates, that might provide a useful data point). See the "Template handling of undefined variables" thread [2] for a longer discussion.

[0] https://code.djangoproject.com/ticket/28516
[1] https://code.djangoproject.com/ticket/28526
[2] https://groups.google.com/d/topic/django-developers/LT5ESP0w0gQ/discussion

To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.

Visit this group at https://groups.google.com/group/django-developers.

[Sjoerd Job Postmus]

As an anecdotal data-point: at the company I'm working at, we are running Django with a custom object as "string_if_invalid" that raises an exception on string-interpolation. This way missing template variables *do* get converted to an exception. I myself am very happy with this solution, as it forces us to make sure templates and views match up correctliy. The essence is that it's better to get an exception, than incorrect output (Yes, you should test beforehand, but sometimes there's a combinatorial explosion that makes that hard).

[Collin Anderson]

I used to do that too (string_if_invalid that raises an exception). What was nice about this, is you could explicitly wrap the variable in an if tag to silence the error if you were ok with it. (I'm not sure if this behavior is still true.)

{% if var_that_sometimes_exists %}{{ var_that_sometimes_exists }}{% endif %}

To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.

Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/f7636989-c87e-4e63-955c-94fb3a14bc78%40googlegroups.com.

[Vlastimil Zíma]

Apparently there is number of errors in admin templates. I suggest to fix the templates. I my experience, the most cases are missing if statements or missing context variables. These can be fixed very easily and produce cleaner templates. I consider this much better solution than just ignoring error messages.

As Anthony suggested, the main problem is more often the fuzziness of the messages, which do not often properly state template, line or expression which is incorrect. This makes it difficult to resolve them in some cases.

Vlastik

Dne čtvrtek 24. srpna 2017 17:21:38 UTC+2 Tim Graham napsal(a):

[Vlastimil Zíma]

If anyone is interested, I've cleaned the errors in admin templates:

Ticket: https://code.djangoproject.com/ticket/28529
PR: https://github.com/django/django/pull/8973

The fixes are quite simple. The biggest problem is sometimes to find out, in which template the bug actually appears.

Vlastik

Dne pátek 25. srpna 2017 9:28:30 UTC+2 Vlastimil Zíma napsal(a):

[Timothy McCurrach]

I was going to have a go at this ticket (https://code.djangoproject.com/ticket/28526) -  which links this thread.

Having read the various replies, it seems like there is no shortage of differing views how missing/invalid variables should be treated:

 - There should be no logging

 - There should be logging, but full tracebacks are too noisy / not beginner friendly

 - Full tracebacks are useful

 - There should be logging but only for the first time an invalid variable is encountered within a template

 - Some combination of the above with a setting (or settings) to alter behaviour

 - There should be a setting to make invalid variables raise errors

There is a danger of bloating the number of options/settings and still not catering for what everyone wants.

One way to allow all of these would be to provide a hook to handle raising errors/logging in the case of invalid variables. This would also allow all kinds of custom behaviour, that might be useful for particular cases, but which wouldn't warrant being features of django itself:

 - log / raise only on particular templates

 - provide different levels of logging depending on the type of the invalid variable

 - etc

I originally thought of replacing `string_if_invalid` with an option function `invalid_variable_handler` that points to a function, which would return a string for invalid variables, and could also handle logging/errors at the same time. Perhaps this is too many concerns for one function; although having said that, it's really just one concern - handling invalid/undefined variables. The default function could return `string_if_invalid` during the interim whilst we add a deprecation warning for `string_if_invalid`.

A less destructive option would be to leave `string_if_invalid` as it is, and just move the code that logs/raises errors to a public method `handle_variable_resolve_errors` of Engine(?). This would provide a public API for people to customise logging/raising of errors if they should so wish, by sub-classing `Engine` (and then add an engine attribute to DjangoTemplates??). 

Alternatively, `handle_variable_resolve_errors` could just be another option which would point to the above function, rather than having to subclass Engine.

The very first of these approaches would allow useful things like returning different `string_if_invalid` values for different templates, if debug is on etc. But seems a bit messier.

There are a few more variations on the same theme, in terms of actual implementation, but the basic idea is the same - move logging/raising errors to an overridable function. 

Any thoughts on this would be much appreciated

[Vlastimil Zíma]

+1 to overridable function. Subclassing the whole Engine would be somewhat cumbersome to just set up errors for invalid variable cases.

Dne neděle 24. ledna 2021 v 18:20:58 UTC+1 uživatel tim.mc...@gmail.com napsal: