TypeError: 'NoneType' object is not iterable --- enumerating late links (layout.html)

[olemis+trac@…]

I'm submitting this bug report after performing this search . Nothing found .

Fact is that I got this error .

How to Reproduce

While doing a GET operation on /ticket/48, Trac issued an internal error.

(please provide additional details here)

Request parameters:

{'id': u'48'}

User agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:11.0) Gecko/20100101 Firefox/11.0

System Information

Trac	0.13dev-r11046
Bloodhound Trac	0.13dev-r1360726
Genshi	0.6
Mercurial	2.1
Pygments	1.2.2
pysqlite	2.4.1
Python	2.6.5 (r265:79063, Apr 16 2010, 13:57:41) [GCC 4.4.3]
setuptools	0.6c11
SQLite	3.6.22
jQuery	1.7.2

Enabled Plugins

BloodhoundDashboardPlugin	1.0.0
BloodhoundMultiProduct	0.2.0
BloodhoundTheme	1.0.1
IniAdmin	0.2
TracAccountManager	0.3.2
TracGrowlPlugin	0.2.1dev
TracMercurial	0.13.0.5dev
TracThemeEngine	2.0.1

Python Traceback

Traceback (most recent call last):
  File "/path/to/hg/trac/trac/web/main.py", line 480, in _dispatch_request
    dispatcher.dispatch(req)
  File "/path/to/hg/trac/trac/web/main.py", line 217, in dispatch
    content_type)
  File "/path/to/hg/trac/trac/web/chrome.py", line 1022, in render_template
    encoding='utf-8')
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/core.py", line 183, in render
    return encode(generator, method=method, encoding=encoding, out=out)
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/output.py", line 58, in encode
    for chunk in iterator:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/output.py", line 339, in __call__
    for kind, data, pos in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/output.py", line 826, in __call__
    for kind, data, pos in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/output.py", line 670, in __call__
    for kind, data, pos in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/output.py", line 771, in __call__
    for kind, data, pos in chain(stream, [(None, None, None)]):
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/output.py", line 586, in __call__
    for ev in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/core.py", line 288, in _ensure
    for event in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/core.py", line 288, in _ensure
    for event in stream:
  File "/path/to/hg/trac/trac/web/chrome.py", line 1161, in _strip_accesskeys
    for kind, data, pos in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/core.py", line 288, in _ensure
    for event in stream:
  File "/path/to/hg/trac/trac/web/chrome.py", line 1150, in _generate
    for kind, data, pos in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/core.py", line 288, in _ensure
    for event in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/core.py", line 288, in _ensure
    for event in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/filters/transform.py", line 686, in _unmark
    for mark, event in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/filters/transform.py", line 1175, in __call__
    for mark, (kind, data, pos) in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/filters/transform.py", line 714, in __call__
    for mark, event in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/filters/transform.py", line 779, in __call__
    for mark, event in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/filters/transform.py", line 726, in __call__
    mark, subevent = next()
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/filters/transform.py", line 682, in _mark
    for event in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/template/base.py", line 605, in _include
    for event in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/template/markup.py", line 378, in _match
    ctxt, start=idx + 1, **vars):
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/template/markup.py", line 378, in _match
    ctxt, start=idx + 1, **vars):
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/template/markup.py", line 327, in _match
    for event in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/template/base.py", line 545, in _flatten
    for kind, data, pos in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/core.py", line 288, in _ensure
    for event in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/path.py", line 588, in _generate
    subevent = next()
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/template/base.py", line 605, in _include
    for event in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/template/markup.py", line 316, in _strip
    event = next()
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/template/base.py", line 545, in _flatten
    for kind, data, pos in stream:
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/template/directives.py", line 359, in __call__
    iterable = _eval_expr(self.expr, ctxt, vars)
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/template/base.py", line 277, in _eval_expr
    retval = expr.evaluate(ctxt)
  File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/template/eval.py", line 178, in evaluate
    return eval(self.code, _globals, {'__data__': data})
  File "/path/to/hg/trac/trac/templates/layout.html", line 44, in <Expression u"iter(chrome.late_links.get('stylesheet'))">
    <py:for each="link in chrome.late_links.get('stylesheet')">
TypeError: 'NoneType' object is not iterable

I detected this while running Trac @ r11046 . Nonetheless I checked latest code​ and it seems there is still a problem in there . The following modification (against r11046) should fix it

trac/templates/layout.html

@@ -41 +41 @@
     ${select('*|text()|comment()')}
 
     <script type="text/javascript" py:if="chrome.late_links">
-      <py:for each="link in chrome.late_links.get('stylesheet')">
+      <py:for each="link in chrome.late_links.get('stylesheet', [])">
         jQuery.loadStyleSheet("${link.href}", "${link.type}");
       </py:for>
     </script>

[Remy Blank]

Yes, that's a bug.

[Christian Boos]

Rémy, should we integrate the patch or did you have something else in mind?

[Remy Blank]

I was going to do that, but looking at the code, I don't understand it (the code) anymore :)

In Chrome.render_template(), before rendering, we set req.chrome['links'] = {} and copy the reference to req.chrome['late_links']. How come the template still has data in chrome.links, then? Is the code in the template already evaluated in template.generate()?

The purpose of these empty dicts seems to be to catch links (and JavaScript) that are set after the <head> has been rendered (e.g. in stream filters), and to render them at the end. But if, as I assumed above, the code is evaluated in template.generate(), then chrome.late_links hasn't been set at that point, and it would therefore always be empty.

Clearly, there's something I don't understand here.

But one thing is clear: the patch above will hide the actual problem, which is that a plugin is setting a non-stylesheet link that wasn't rendered in <head>, and only stylesheet links are rendered late, so that link is effectively lost. We may want to apply it anyway for robustness, though.

Thoughts?

[olemis+trac@…]

Replying to rblank:

I was going to do that, but looking at the code, I don't understand it (the code) anymore :)

In Chrome.render_template(), before rendering, we set req.chrome['links'] = {} and copy the reference to req.chrome['late_links']. How come the template still has data in chrome.links, then? Is the code in the template already evaluated in template.generate()?

E.g. by adding something like this in some Genshi template

<?python

from trac.web.chrome import add_stylesheet, add_script

add_stylesheet(req, 'stylesheet.css')
add_script(req, 'script.js')

?>

The purpose of these empty dicts seems to be to catch links (and JavaScript) that are set after the <head> has been rendered (e.g. in stream filters), and to render them at the end. But if, as I assumed above, the code is evaluated in template.generate(), then chrome.late_links hasn't been set at that point, and it would therefore always be empty.

Clearly, there's something I don't understand here.

The bug was identified when a macro (actually a ​Bloodhound widget ;) added some such stylesheet , script , … and another request filter removed it afterwards . The request filter might enabled / disabled , so there is nothing wrong with doing things like this .

But one thing is clear: the patch above will hide the actual problem, which is that a plugin is setting a non-stylesheet link that wasn't rendered in <head>, and only stylesheet links are rendered late, so that link is effectively lost. We may want to apply it anyway for robustness, though.

IMO the simple patch I mentioned above fixes a semantic inconsistency consisting of the fact that the contract of get method of a dictionary states that it may return None . So using the result in a for statement is not a robust approach in general .

Thoughts?

my $0.02 ;)

[Christian Boos]

Replying to rblank:

In Chrome.render_template(), before rendering, we set req.chrome['links'] = {} and copy the reference to req.chrome['late_links'].

Yes, this means that any add_link() that happens during template rendering (e.g. a wiki_to_html() call which in turn calls a macro or a wiki processor) will insert into that new dict, and this will be reachable from the data chrome.late_links as well.

How come the template still has data in chrome.links, then? Is the code in the template already evaluated in template.generate()?

No, in generate() the code is simply parsed, but it is evaluated in render(), as seen in the traceback above.

The purpose of these empty dicts seems to be to catch links (and JavaScript) that are set after the <head> has been rendered (e.g. in stream filters), and to render them at the end.

Yes.

But if, as I assumed above, the code is evaluated in template.generate(), then chrome.late_links hasn't been set at that point, and it would therefore always be empty.

No, wrong assumption ;-) But I hope it's clear now.

But one thing is clear: the patch above will hide the actual problem, which is that a plugin is setting a non-stylesheet link that wasn't rendered in <head>, and only stylesheet links are rendered late, so that link is effectively lost. We may want to apply it anyway for robustness, though.

Well, what was unclear for me is how could ever the late_links be non-null but not contain the 'stylesheet' key, given the way we set the late_links in render_template(). However comment:5 hints about how this happened ("and another request filter removed it afterwards" … deletion of the whole 'stylesheet' entry, mayhaps?).

Thoughts?

Writing late_links.get('stylesheet', []) would perhaps wrongly convey the idea that the 'stylesheet' entry may be missing, which should normally never happen, so I think the code as it stands has at least the merit to be consistent.

But while looking at this, I re-read r6135, see last paragraph ;-)

[olemis+trac@…]

Replying to cboos:

Replying to rblank:

[…]

But one thing is clear: the patch above will hide the actual problem, which is that a plugin is setting a non-stylesheet link that wasn't rendered in <head>, and only stylesheet links are rendered late, so that link is effectively lost. We may want to apply it anyway for robustness, though.

Well, what was unclear for me is how could ever the late_links be non-null but not contain the 'stylesheet' key, given the way we set the late_links in render_template(). However comment:5 hints about how this happened ("and another request filter removed it afterwards" … deletion of the whole 'stylesheet' entry, mayhaps?).

I'll provide further details . I was trying to close [BH:ticket:226 a ticket] once I noticed this problem (there's a patch attached btw) . Bloodhound disables all common trac css in order to avoid many conflicts we've found related to interactions with ​Bootstrap styling . As a particular case it turns out that Bloodhound activity widget (i.e. the timeline in a jar ;) includes timeline.css necessary to make it look as expected when BloodhoundTheme plugin is not active . Before BH:ticket:226 that stylesheet was still applied . Nonetheless considering that request for enhancement if bloodhound theme is enabled then the corresponding theme component (i.e. request filter) had to remove stylesheet from the list , and thereby this error occurs .

[Christian Boos]

Replying to cboos:

No, in generate() the code is simply parsed, but it is evaluated in render(), as seen in the traceback above.

I couldn't sleep after writing this ;-) Of course, the Python code in the template is parsed much earlier, when the template is first loaded. generate() is mostly binding the data to the (genshi) Context and get the filter generators "stacked up".

[Christian Boos]

Replying to olemis+trac@…:

[…] the corresponding theme component (i.e. request filter) had to remove stylesheet from the list , and thereby this error occurs .

There's certainly a better way to do this removal, like (selectively?) clearing the dict contained at the 'stylesheet' key, rather than deleting the 'stylesheet' entry.

[olemis+trac@…]

Replying to cboos:

Replying to olemis+trac@…:

[…] the corresponding theme component (i.e. request filter) had to remove stylesheet from the list , and thereby this error occurs .

There's certainly a better way to do this removal, like (selectively?) clearing the dict contained at the 'stylesheet' key, rather than deleting the 'stylesheet' entry.

That's what we do already . Widgets are somehow like macros , in the sense that they can be embedded and rendered in many different contexts . So we remove some stylesheets if present , not all . We do not delete 'stylesheet' entry either . I'm pasting below the relevant code , extracted from ​theme.py for you to take a look at .

class BloodhoundTheme(ThemeBase):
    """Look and feel of Bloodhound issue tracker.
    """

    # ----------8<---------- code omitted ----------8<----------

    BLOODHOUND_KEEP_CSS = set(
        (
            'diff.css',
        )
    )

    # ----------8<---------- code omitted ----------8<----------

    def post_process_request(self, req, template, data, content_type):
        """Post process request filter.
        Removes all trac provided css if required"""
        def is_active_theme():
            is_active = False
            active_theme = ThemeEngineSystem(self.env).theme
            if active_theme is not None:
                this_theme_name = self.get_theme_names().next()
                is_active = active_theme['name'] == this_theme_name
            return is_active

        links = req.chrome.get('links',{})
        # replace favicon if appropriate
        if self.env.project_icon == 'common/trac.ico':
            bh_icon = 'theme/img/bh.ico'
            new_icon = {'href': req.href.chrome(bh_icon),
                        'type': get_mimetype(bh_icon)}
            if links.get('icon'):
                links.get('icon')[0].update(new_icon)
            if links.get('shortcut icon'):
                links.get('shortcut icon')[0].update(new_icon)
        
        is_active_theme = is_active_theme()
        if self.disable_all_trac_css and is_active_theme:
            if self.disable_all_trac_css:
                stylesheets = links.get('stylesheet',[])
                if stylesheets:
                    path = req.base_path + '/chrome/common/css/'
                    _iter = ([ss, ss.get('href', '')] for ss in stylesheets)
                    links['stylesheet'] = [ss for ss, href in _iter 
                            if not href.startswith(path) or
                            href.rsplit('/', 1)[-1] in self.BLOODHOUND_KEEP_CSS]

    # ----------8<---------- code omitted ----------8<----------

        return template, data, content_type

In ​timeline.py the widget invokes add_stylesheet(req, 'common/css/timeline.css') . All this happens by expanding ​custom ticket template looking like this :

    <!------------8<---------- code omitted ----------8<---------->
    <div id="content" class="ticket row">
      <div class="$colspan">
        <!------------8<---------- code omitted ----------8<---------->
      <div>
      <div py:if="bhdb" class="span4">
         <bh:widget urn="Timeline">
           <bh:args>
             <bh:arg name="filters">ticket,ticket_details,changeset</bh:arg>
           </bh:args>
         </bh:widget>
      </div>
    </div>
    <!------------8<---------- code omitted ----------8<---------->

bh:* tags are expanded by following the rules in ​widget_macros.html together with the whole dashboard infrastructure , which is a bit much harder to describe .

So maybe this is about the combination of many factors . Fact is that , in any case there's a semantic inconsistency due to the fact that the contract of dict.get method states that it may return None , which is neither a collection nor an iterable object .

PS: I cannot insert links to Bloodhound repository browser area because we have not connected ASF svn repos with the issue tracker … yet … Hope you don't mind . I think I've been providing enough information considering the size and characteristics of the patch . Feel free to request more details any time .

[Remy Blank]

Replying to cboos:

Replying to rblank:

But if, as I assumed above, the code is evaluated in template.generate(), then chrome.late_links hasn't been set at that point, and it would therefore always be empty.

No, wrong assumption ;-) But I hope it's clear now.

Well… not quite yet. If the code is evaluated in template.render(), then how come chrome.links ever contains anything? It's set to an empty list right before rendering.

Writing late_links.get('stylesheet', []) would perhaps wrongly convey the idea that the 'stylesheet' entry may be missing, which should normally never happen, so I think the code as it stands has at least the merit to be consistent.

I actually agree with Olemis' fix. In add_link(), we do a links.setdefault(rel, []).append(link) with rel = 'stylesheet', which tells me that chrome.links is either absent or a list. So let's handle this case gracefully.

Patch applied in [11463].

[Christian Boos]

Replying to rblank:

Replying to cboos:

Replying to rblank:

But if, as I assumed above, the code is evaluated in template.generate(), then chrome.late_links hasn't been set at that point, and it would therefore always be empty.

No, wrong assumption ;-) But I hope it's clear now.

Well… not quite yet. If the code is evaluated in template.render(), then how come chrome.links ever contains anything? It's set to an empty list right before rendering.

It's populated during rendering, if anything calls add_link (like the example I gave in the first paragraph of comment:6).

I actually agree with Olemis' fix. In add_link(), we do a links.setdefault(rel, []).append(link) with rel = 'stylesheet', which tells me that chrome.links is either absent or a list. So let's handle this case gracefully.

Ok, there might be other links than 'stylesheet' links (like 'icon' or 'shortcut icon' from Olemis' example in comment:10), in which case chrome.late_links could be non-empty without necessarily containing the 'stylesheet' entry.

[Remy Blank]

Replying to cboos:

It's populated during rendering, if anything calls add_link (like the example I gave in the first paragraph of comment:6).

Yes, and these will be rendered as late_links. But how do the links that were originally in chrome.links get rendered? If I look at the source of this page, I see they are in <head>, so the content of that tag must be rendered before we swap out chrome.links, no?

I guess I need to trace through that code to finally understand.

[Christian Boos]

Replying to rblank:

Replying to cboos:

It's populated during rendering, if anything calls add_link (like the example I gave in the first paragraph of comment:6).

Yes, and these will be rendered as late_links.

Ah, I thought you were wondering about these.

But how do the links that were originally in chrome.links get rendered?

In populate_data() we have d['chrome'].update(req.chrome), which is why later replacement of req.chrome content will not matter.

I guess I need to trace through that code to finally understand.

Still a good idea as you mightfind a nice way to simplify this h*ly mess ;-)

[Remy Blank]

Replying to cboos:

In populate_data() we have d['chrome'].update(req.chrome), which is why later replacement of req.chrome content will not matter.

Aaah… That's what I was missing. Thanks for the explanation. This would certainly benefit from a big fat comment in the code.