Better Jinja2 error messages (#100)

* Blackify task.py

* Improve Jinja2 error handling

* Jinja errors from base.template

* Pass Jinja errors in creation of Jobs

* Update CHANGES

CHANGES.txt

@@ -14,6 +14,7 @@ Features
 - #96: Drop legacy task names
 - #97: Better context class. This allows dotted keys!
 - #99: Add support for Python 3.12 (and drop 3.6, 3.7)
+- #100: Better error messages for Jinja2 errors
 
 
 Fixes

src/scriptengine/jinja.py

@@ -94,11 +94,11 @@ def render_with_context(string_arg):
         try:
             # Render string in parameter environment using context
             return _param_env.from_string(string_arg).render(context)
-        except jinja2.TemplateSyntaxError:
+        except jinja2.TemplateError as e:
             raise ScriptEngineParseJinjaError(
-                "Syntax error while rendering template string "
-                f'"{string_arg}"'
-                f'{" in boolean context" if boolean else ""}'
+                f"Jinja2 {type(e).__name__} while parsing '{string_arg}'"
+                f"{' (in boolean context): ' if boolean else ': '}"
+                f"{e}"
             )
 
     if isinstance(arg, str):

src/scriptengine/jobs.py

@@ -75,29 +75,26 @@ def todo(self, todo):
     def when(self, context):
         try:
             return self._when is None or j2render(self._when, context, boolean=True)
-        except ScriptEngineParseJinjaError:
+        except ScriptEngineParseJinjaError as e:
             self.log_error(
-                "Error while parsing (Jinja2) invalid when clause "
-                f'"{self._when}" with context "{context}"'
+                f"Jinja2 error in *when* clause '{self._when}' (full error: {e})"
             )
             raise ScriptEngineJobParseError
 
     def loop_spec(self, context):
         try:
             iter = j2render(self._loop, context)
-        except ScriptEngineParseJinjaError:
+        except ScriptEngineParseJinjaError as e:
             self.log_error(
-                "Error while parsing (Jinja2) invalid loop expression "
-                f'"{self._loop}" with context "{context}"'
+                f"Jinja2 error in *loop* spec '{self._loop}' (full error: {e})"
             )
             raise ScriptEngineJobParseError
         if isinstance(iter, str):
             try:
                 iter = ast.literal_eval(iter or "None")
-            except (SyntaxError, ValueError):
+            except (SyntaxError, ValueError) as e:
                 self.log_error(
-                    "Error while evaluating (AST) invalid loop expression "
-                    f'"{iter}" with context "{context}"'
+                    f"AST evaluation error in *loop* spec '{iter}' (full error: {e})"
                 )
                 raise ScriptEngineJobParseError
         if isinstance(iter, dict):

src/scriptengine/tasks/base/template.py

@@ -1,13 +1,15 @@
 """Template task for ScriptEngine."""
 
-from pathlib import Path
 import os
 import stat
+from pathlib import Path
+
 import jinja2
 
-from scriptengine.tasks.core import Task, timed_runner
-from scriptengine.jinja import render as j2render
+from scriptengine.exceptions import ScriptEngineTaskRunError
 from scriptengine.jinja import filters as j2filters
+from scriptengine.jinja import render as j2render
+from scriptengine.tasks.core import Task, timed_runner
 
 
 class Template(Task):
@@ -54,7 +56,12 @@ def run(self, context):
         for name, function in j2filters().items():
             j2env.filters[name] = function
 
-        output = j2render(j2env.get_template(src).render(context), context)
+        try:
+            output = j2render(j2env.get_template(src).render(context), context)
+        except jinja2.TemplateError as e:
+            self.log_error(f"Jinja2 error {type(e).__name__}: {e}")
+            raise ScriptEngineTaskRunError
+
         with dst.open(mode="w") as f:
             f.write(output + "\n")
 

src/scriptengine/tasks/core/task.py

@@ -5,26 +5,27 @@
 
 import logging
 import uuid
+
 import yaml
 
 import scriptengine.jinja
-from scriptengine.yaml.noparse_strings import (
-    NoParseJinjaString,
-    NoParseYamlString,
-)
 import scriptengine.yaml
-
-from scriptengine.exceptions import ScriptEngineTaskArgumentInvalidError, \
-                                    ScriptEngineTaskArgumentMissingError
-
+from scriptengine.exceptions import (
+    ScriptEngineParseJinjaError,
+    ScriptEngineTaskArgumentInvalidError,
+    ScriptEngineTaskArgumentMissingError,
+)
+from scriptengine.yaml.noparse_strings import NoParseJinjaString, NoParseYamlString
 
 _SENTINEL = object()
 
 
 class Task:
-
     _reg_name = None
-    _invalid_arguments = ('run', 'id', )
+    _invalid_arguments = (
+        "run",
+        "id",
+    )
 
     @classmethod
     def check_arguments(cls, arguments):
@@ -34,50 +35,48 @@ def check_arguments(cls, arguments):
         ScriptEngineTaskArgumentMissingError.
         """
         for name in arguments:
-            if name in getattr(cls, '_invalid_arguments', ()):
-                logging.getLogger('se.task').error(
+            if name in getattr(cls, "_invalid_arguments", ()):
+                logging.getLogger("se.task").error(
                     (
                         f'Invalid argument "{name}" found while '
                         f'trying to create "{cls.__name__}" task'
                     ),
-                    extra={'id': 'no id', 'type': cls.__name__},
+                    extra={"id": "no id", "type": cls.__name__},
                 )
                 raise ScriptEngineTaskArgumentInvalidError
 
-        for name in getattr(cls, '_required_arguments', ()):
+        for name in getattr(cls, "_required_arguments", ()):
             if name not in arguments:
-                logging.getLogger('se.task').error(
+                logging.getLogger("se.task").error(
                     (
                         f'Missing required argument "{name}" while '
                         f'trying to create "{cls.__name__}" task'
                     ),
-                    extra={'id': 'no id', 'type': cls.__name__},
+                    extra={"id": "no id", "type": cls.__name__},
                 )
                 raise ScriptEngineTaskArgumentMissingError
 
     def __init__(self, arguments=None):
-
         self._identifier = uuid.uuid4()
 
         if arguments is not None:
             Task.check_arguments(arguments)
             for name, value in arguments.items():
                 if hasattr(self, name):
-                    self.log_error(
-                        f'Invalid (reserved name) task argument: {name}'
-                    )
+                    self.log_error(f"Invalid (reserved name) task argument: {name}")
                     raise ScriptEngineTaskArgumentInvalidError
                 setattr(self, name, value)
-        self.log_debug(f'Created task: {self}')
+        self.log_debug(f"Created task: {self}")
 
     @classmethod
     def register_name(cls, name):
         cls._reg_name = name
 
     @property
     def reg_name(self):
-        return self._reg_name or \
-               f'{self.__class__.__module__}:{self.__class__.__name__}'
+        return (
+            self._reg_name or f"{self.__class__.__module__}:{self.__class__.__name__}"
+        )
 
     @property
     def id(self):
@@ -88,32 +87,33 @@ def shortid(self):
         return self._identifier.hex[:10]
 
     def __repr__(self):
-        params = {key: val for key, val in self.__dict__.items()
-                  if not (isinstance(key, str) and key.startswith('_'))}
+        params = {
+            key: val
+            for key, val in self.__dict__.items()
+            if not (isinstance(key, str) and key.startswith("_"))
+        }
         params_list = f'{", ".join([f"{k}={params[k]}" for k in params])}'
-        return f'{self.__class__.__name__}({params_list})'
+        return f"{self.__class__.__name__}({params_list})"
 
     def run(self, context):
-        raise NotImplementedError(
-            'Base class function Task.run() must not be called'
-        )
+        raise NotImplementedError("Base class function Task.run() must not be called")
 
-    def getarg(self, name, context={}, *,
-               parse_jinja=True, parse_yaml=True, default=_SENTINEL):
+    def getarg(
+        self, name, context={}, *, parse_jinja=True, parse_yaml=True, default=_SENTINEL
+    ):
         """Returns the value of argument 'name'.
-           The argument value is parsed with Jinja2 and the given context,
-           unless 'parse_jinja' is False. The result is parsed once more with
-           the YAML parser in order to get a correctly typed result. If
-           parse_yaml is not True, the extra YAML parsing is skipped.
-           Parsing with Jinja/YAML is also skipped, if the argument value is a
-           string that starts with '_noparse_', '_noparsejinja_',
-           '_noparseyaml_', respectively.
-           If argument 'name' does not exist, the function raises an
-           AttributeError, unless a 'default' value is given.
-           """
+        The argument value is parsed with Jinja2 and the given context,
+        unless 'parse_jinja' is False. The result is parsed once more with
+        the YAML parser in order to get a correctly typed result. If
+        parse_yaml is not True, the extra YAML parsing is skipped.
+        Parsing with Jinja/YAML is also skipped, if the argument value is a
+        string that starts with '_noparse_', '_noparsejinja_',
+        '_noparseyaml_', respectively.
+        If argument 'name' does not exist, the function raises an
+        AttributeError, unless a 'default' value is given.
+        """
 
         def parse(arg_):
-
             # Recursively parse list items
             if isinstance(arg_, list):
                 return [parse(item) for item in arg_]
@@ -123,24 +123,24 @@ def parse(arg_):
                 return dict((key, parse(val)) for key, val in arg_.items())
 
             if isinstance(arg_, str):
-                if parse_jinja and \
-                   not isinstance(arg_, NoParseJinjaString):
-                    # Make sure that a NoParseString is still a NoParseString
-                    # after this!
-                    arg_ = type(arg_)(scriptengine.jinja.render(arg_, context))
-
-                if parse_yaml and \
-                   not isinstance(arg_, NoParseYamlString):
+                if parse_jinja and not isinstance(arg_, NoParseJinjaString):
+                    try:
+                        # Make sure that a NoParseString is still a NoParseString
+                        # after this!
+                        arg_ = type(arg_)(scriptengine.jinja.render(arg_, context))
+                    except ScriptEngineParseJinjaError as e:
+                        self.log_error(e)
+                        raise ScriptEngineTaskArgumentInvalidError
+
+                if parse_yaml and not isinstance(arg_, NoParseYamlString):
                     try:
                         return yaml.full_load(arg_)
                     except (
                         yaml.scanner.ScannerError,
                         yaml.parser.ParserError,
                         yaml.constructor.ConstructorError,
                     ):
-                        self.log_debug(
-                            f'Reparsing argument "{arg_}" with YAML failed'
-                        )
+                        self.log_debug(f'Reparsing argument "{arg_}" with YAML failed')
 
                 # Return plain strings, not NoParse*Strings
                 return str(arg_)
@@ -152,20 +152,14 @@ def parse(arg_):
             arg = getattr(self, name)
         except AttributeError:
             if default is _SENTINEL:
-                self.log_error(
-                    f'Trying to access missing task argument: {name}'
-                )
+                self.log_error(f"Trying to access missing task argument: {name}")
                 raise ScriptEngineTaskArgumentMissingError
             arg = default
         return parse(arg)
 
     def _log(self, level, msg):
-        logger = logging.getLogger('se.task')
-        logger.log(
-            level,
-            msg,
-            extra={'type': self.reg_name, 'id': self.shortid}
-        )
+        logger = logging.getLogger("se.task")
+        logger.log(level, msg, extra={"type": self.reg_name, "id": self.shortid})
 
     def log_debug(self, msg):
         self._log(logging.DEBUG, msg)