jinja icon indicating copy to clipboard operation
jinja copied to clipboard

New verbose error message

Open Yourun-proger opened this issue 2 years ago • 6 comments

  • fixes #1661

Checklist:

  • [ ] Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • [ ] Add or update relevant docs, in the docs folder and in code.
  • [x] Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • [ ] Add .. versionchanged:: entries in any relevant code docs.
  • [x] Run pre-commit hooks and fix any issues.
  • [x] Run pytest and tox, no tests failed.

Yourun-proger avatar May 02 '22 12:05 Yourun-proger

I don't find this particularly useful to be honest. Especially in a larger Flask app with many blueprints (which IIRC. all contribute to the search path) this would make the error message extremely bloated with information that's not particularly useful in most cases...

ThiefMaster avatar May 02 '22 12:05 ThiefMaster

Probably. I saw now that in airflow has already come up with a patch. Maybe the truth is that everyone should change on their own, not at the library level. In short, I can close this.

Yourun-proger avatar May 02 '22 12:05 Yourun-proger

Well, a parameter like ignore or verbose is definitely stupid....

Yourun-proger avatar May 02 '22 13:05 Yourun-proger

A middle ground for this would be to include searchpath on the exception object as an attribute, but not shown in the error message. This should be good enough for most usages; they can simply do

try:
    template = get_template(filename)
except TemplateNotFound as e:
    raise MyCustomException(f"{filename} not found in {e.searchpath}")

instead of having to dig through the template loaders to find the context.

uranusjr avatar May 02 '22 21:05 uranusjr

@uranusjr, thanks for the reasonable offer. I think that it can satisfy everyone. If no one minds, I'll take care of it.

Yourun-proger avatar May 03 '22 12:05 Yourun-proger

It's me again. I made a patch that looks like this:

diff --git a/src/jinja2/exceptions.py b/src/jinja2/exceptions.py
index 082ebe8..6031bee 100644
--- a/src/jinja2/exceptions.py
+++ b/src/jinja2/exceptions.py
@@ -1,4 +1,5 @@
 import typing as t
+import os

 if t.TYPE_CHECKING:
     from .runtime import Undefined
@@ -18,6 +19,9 @@ class TemplateError(Exception):
 class TemplateNotFound(IOError, LookupError, TemplateError):
     """Raised if a template does not exist.

+    .. versionchanged:: 3.1.3
+        Added new parameter "search path"
+
     .. versionchanged:: 2.11
         If the given name is :class:`Undefined` and no message was
         provided, an :exc:`UndefinedError` is raised.
@@ -31,6 +35,7 @@ class TemplateNotFound(IOError, LookupError, TemplateError):
         self,
         name: t.Optional[t.Union[str, "Undefined"]],
         message: t.Optional[str] = None,
+        searchpath: t.Optional[t.Union[str, os.PathLike, t.Sequence[t.Union[str, os.PathLike]]]] = None,
     ) -> None:
         IOError.__init__(self, name)

@@ -45,6 +50,8 @@ class TemplateNotFound(IOError, LookupError, TemplateError):
         self.message = message
         self.name = name
         self.templates = [name]
+        # Transforms of searchpath are not required here.
+        self.searchpath = searchpath

     def __str__(self) -> str:
         return str(self.message)
@@ -55,6 +62,9 @@ class TemplatesNotFound(TemplateNotFound):
     are selected.  This is a subclass of :class:`TemplateNotFound`
     exception, so just catching the base exception will catch both.

+    .. versionchanged:: 3.1.3
+        Added new parameter "search path"
+
     .. versionchanged:: 2.11
         If a name in the list of names is :class:`Undefined`, a message
         about it being undefined is shown rather than the empty string.
@@ -66,6 +76,7 @@ class TemplatesNotFound(TemplateNotFound):
         self,
         names: t.Sequence[t.Union[str, "Undefined"]] = (),
         message: t.Optional[str] = None,
+        searchpath: t.Optional[t.Union[str, os.PathLike, t.Sequence[t.Union[str, os.PathLike]]]] = None,
     ) -> None:
         if message is None:
             from .runtime import Undefined

and

diff --git a/src/jinja2/loaders.py b/src/jinja2/loaders.py
index d2c39c1..e398c2d 100644
--- a/src/jinja2/loaders.py
+++ b/src/jinja2/loaders.py
@@ -215,9 +215,7 @@ class FileSystemLoader(BaseLoader):

             # Use normpath to convert Windows altsep to sep.
             return contents, os.path.normpath(filename), uptodate
-        raise TemplateNotFound(
-            f"{template} not found in the following search path(s): {self.searchpath}"
-        )
+        raise TemplateNotFound(template, searchpath=self.searchpath)

     def list_templates(self) -> t.List[str]:
         found = set()

In principle, now you can achieve the desired behavior and do something like:

try:
    template = my_file_system_loader.get_source(some_env, filename)
except TemplateNotFound as e:
    raise MyCustomException(f"{filename} not found in {e.searchpath}")

As you may have understood, I now need to add the keyword argument searchpath wherever TemplateNotFound and TemplatesNotFound are raised, which is feasible in principle, but seems like some kind of bad decision? What do you think about this? Can you advise me something?

Yourun-proger avatar May 03 '22 17:05 Yourun-proger