[patch] Noisier bugs in context processors

[Jeremy Dunck <jdunck@…>]

I just spent an embarrassingly long time figuring out the cause of "'NoneType' object has no attribute 'has_key'" when rendering a template.

I had a buggy context processor which returned None under some conditions, which then made it onto the context stack.

It seems to me that Context.update could verify that it's getting an object that supports getitem since it counts on that (and give a helpful error if it's not the case), but I understand context needs to be high-performance, and generally update is called for context processors anyway.

Perhaps RequestContext.init could raise a TypeError.

Patch attached. Feel free to ignore if I'm the only one to be bitten by this. :-/

[Jeremy Dunck <jdunck@…>]

Another diff in case preference is to enforce in Context.update rather than RequestContext.init

[(none)]

Milestone Version 1.0 deleted

[Chris Beaven]

Looks just about ready for checkin - needs decision on best way to implement.

I feel the patch should be using a try/except, however

[James Bennett]

Unless I'm misunderstanding, this is invalid: a context processor *must* return a dictionary, even if it's empty, and the pattern for a processor which wants to return nothing is to return an empty dictionary (much as template nodes can render to an empty string if they don't want to output anything).

[Jeremy Dunck]

Re-opening for clarification. I'm not fighting the invalid decision, if my point is understood, but ubernostrum suggested that maybe it wasn't.

I agree that a context processor *must* return a dictionary. What I'm trying to address, here, is that it isn't immediately obvious when a bug exists in a context processor which causes it not to return the expected type.

If a context processor returns an object which can't be used as a dictionary, an error does not occur until much later, and then that error 1) kills template rendering rather than using TEMPLATE_STRING_IF_INVALID, and 2) doesn't hint that a context processor may have caused the problem.

...And I'm using hasattr...'getitem' rather than isinstance(..., dict) because Context itself doesn't inherit from dict, and smashing contexts together currently works, so I didn't want the change to start insisting on a dictionary.

Finally, I'm not using "try...except" because Context.update doesn't throw an exception when handed something other than a dictionary-- it happily appends None to the stack of *expected* dictionaries to be checked upon context lookup, i.e. Context.getitem.

Feel free to close as invalid before, but I hope I've made my rationale clearer.

[anonymous]

+1 on this one, it's a 2-liner and if it saves anybody an hour or two it should go in. I'd make the message more explicit though.

[Luke Plant]

Fixed in r8181 (which mistakenly says it fixed #3912 instead of #3192