Template.render() should verify context instance

[cephelo@…]

Currently, Template.render() doesn't assert that "context" really is a Context object. This leads to all sorts of fun problems when debugging code that happens to be sending a dict() instead of Context(). While most tags work fine with a dict instead, there are a few that don't, notably {% for %} and {% filter %} (possibly a few others). While I'm not a huge fan of forcing data types, you get really wonky error messages.

The change itself is trivial, but I'm not sure how to handle a regression test for this, as the tests currently force-cast the dicts into Contexts.

[Chris Beaven]

I agree, but I'll push to a design decision from one of the core.

[Thomas Steinacher <tom@…>]

I think that it should definitely do a check. Otherwise you get strange errors when using autoescaping: 'dict' object has no attribute 'autoescape'.

[Tony Becker]

Would it make sense to do the check AFTER an error has occurred, and report at that time that "Hey, you are sending a dict instead of a context?" Would there be any performance benefit versus doing the check every single time? It seems to me that type checking every time is a little unpythonic, and the main goal is to get better error messages.

[James Bennett]

Personally I'd rather have the eventual error message be friendlier to say something like "did you pass a dictionary instead of a Context object?"; doing type checks or assertions to verify that we've actually been handed a Context is optimization for a case where things wouldn't work anyway (and would introduce some negligible overhead into the process).

[munhitsu]

Replying to ubernostrum:

Personally I'd rather have the eventual error message be friendlier to say something like "did you pass a dictionary instead of a Context object?"; doing type checks or assertions to verify that we've actually been handed a Context is optimization for a case where things wouldn't work anyway (and would introduce some negligible overhead into the process).

Rising this error message will require a type check so I don't see difference

Replying to tbecker:

Would it make sense to do the check AFTER an error has occurred, and report at that time that "Hey, you are sending a dict instead of a context?" Would there be any performance benefit versus doing the check every single time? It seems to me that type checking every time is a little unpythonic, and the main goal is to get better error messages.

Every time can mean only on each template render execution. Not on every Node render. So we have already an optimisation here.

[Chris Beaven]

I'm going to say that we don't need this as critically now. The error messages are at least raised in the render method now, since we do context.render_context.push() in there.

And like ubernostrum stated 3 years ago, doing type checks introduces overhead and no matter how negligible, it's still overhead which we don't need.