Issue 3084 - [cms] Show full tracebacks for exceptions passing template code

cms/converters.py

Index: cms/converters.py
===================================================================
--- a/cms/converters.py
+++ b/cms/converters.py
@@ -120,11 +120,15 @@
     self._seen_defaults  = {}
 
     # Read in any parameters specified at the beginning of the file
-    lines = params[key].splitlines(True)
-    while lines and re.search(r"^\s*[\w\-]+\s*=", lines[0]):
-      name, value = lines.pop(0).split("=", 1)
+    data, filename = params[key]
+    lines = data.splitlines(True)
+    for i, line in enumerate(lines):
+      if not re.search(r"^\s*[\w\-]+\s*=", line):
+        break
+      name, value = line.split("=", 1)
       params[name.strip()] = value.strip()
-    params[key] = "".join(lines)
+      lines[i] = "\n"
+    params[key] = ("".join(lines), filename)
 
   def localize_string(self, page, name, default, comment, localedata, escapes):
     def escape(s):
@@ -277,7 +281,7 @@
       return result
 
 class RawConverter(Converter):
-  def get_html(self, source):
+  def get_html(self, (source, filename)):

[kzar, 2015/09/17 08:41:17]

Any reason we don't just have source and filename as separate parameters here?
(Below too.)

[Sebastian Noack, 2015/09/17 08:49:59]

Indeed, we could do the unpacking in the calling code like
|self.get_html(*self._params[self._key])|. I think this would actual be nicer.
As this patch already landed, feel free to change it yourself.

[Sebastian Noack, 2015/09/17 10:01:23]

This bothered me. So I uploaded another patch set.

     result = self.insert_localized_strings(source, html_escapes)
     result = self.process_links(result)
     return result
@@ -292,7 +296,7 @@
     re.escape(jinja2.escape(Converter.include_end_regex))
   )
 
-  def get_html(self, source):
+  def get_html(self, (source, filename)):
     def remove_unnecessary_entities(match):
       char = unichr(int(match.group(1)))
       if char in html_escapes:
@@ -318,17 +322,18 @@
     result = self.process_links(result)
     return result
 
+class SourceTemplateLoader(jinja2.BaseLoader):
+  def __init__(self, source):
+    self.source = source
+
+  def get_source(self, environment, template):
+    try:
+      result = self.source.read_file(template + ".tmpl")
+    except Exception:
+      raise jinja2.TemplateNotFound(template)
+    return result + (None,)
+
 class TemplateConverter(Converter):
-  class _SourceLoader(jinja2.BaseLoader):
-    def __init__(self, source):
-      self.source = source
-
-    def get_source(self, environment, template):
-      try:
-        return self.source.read_file(template + ".tmpl"), None, None
-      except Exception:
-        raise jinja2.TemplateNotFound(template)
-
   def __init__(self, *args, **kwargs):
     Converter.__init__(self, *args, **kwargs)
 
@@ -350,16 +355,28 @@
           continue
 
         path = "%s/%s" % (dirname, filename)
+        namespace = self._params["source"].exec_file(path)
+
         name = os.path.basename(root)
-        dictionary[name] = self._params["source"].import_symbol(path, name)
+        try:
+          dictionary[name] = namespace[name]
+        except KeyError:
+          raise Exception("Expected symbol %r not found in %r" % (name, path))
 
-    self._env = jinja2.Environment(loader=self._SourceLoader(self._params["source"]), autoescape=True)
+    self._env = jinja2.Environment(loader=SourceTemplateLoader(self._params["source"]), autoescape=True)
     self._env.filters.update(filters)
     self._env.globals.update(globals)
 
-  def get_html(self, source):
-    template = self._env.from_string(source)
-    module = template.make_module(self._params)
+  def get_html(self, (source, filename)):
+    env = self._env
+    code = env.compile(source, None, filename)
+    template = jinja2.Template.from_code(env, code, env.globals)

[kzar, 2015/09/17 08:41:17]

I was trying to look up jinja2.Template.from_code to get more context about
what's happening here but I had no luck. Where can I find it?

[Sebastian Noack, 2015/09/17 08:49:59]

Indeed, it's not documented, at least not in the online docs. I wasn't ware of
that. But honestly, I think it's better to rely on these one undocumented
method, rather than having incomplete tracebacks. And to answer your question,
you find here in the source code:
https://github.com/mitsuhiko/jinja2/blob/2.8/jinja2/environment.py#L929

[Wladimir Palant, 2015/09/17 21:30:14]

It's documented under
http://code.nabla.net/doc/jinja2/api/jinja2/environment/jinja2.environment.Te...
but apparently that's not the official documentation.

+
+    try:
+      module = template.make_module(self._params)
+    except Exception:
+      env.handle_exception()
+
     for key, value in module.__dict__.iteritems():
       if not key.startswith("_"):
         self._params[key] = value