Merge pull request sphinx-contrib#99 from Bizordec/master

handle directive exceptions

sphinxcontrib/datatemplates/directive.py

@@ -5,6 +5,7 @@
 from collections import defaultdict
 
 import defusedxml.ElementTree as ET
+import jinja2
 import yaml
 from docutils import nodes
 from docutils.parsers import rst
@@ -152,8 +153,14 @@ def run(self):
             source = self.options['source']
         elif self.arguments:
             source = self.arguments[0]
-        else:
+        elif self.loader in {loaders.load_import_module, loaders.load_nodata}:
             source = ""
+        else:
+            error = self.state_machine.reporter.error(
+                'Source file is required',
+                nodes.literal_block(self.block_text, self.block_text),
+                line=self.lineno)
+            return [error]
 
         relative_resolved_path, absolute_resolved_path = env.relfn2path(source)
 
@@ -171,6 +178,13 @@ def run(self):
             template = '\n'.join(self.content)
             render_function = _templates(builder).render_string
 
+        if not template:
+            error = self.state_machine.reporter.error(
+                "Template is empty",
+                nodes.literal_block(self.block_text, self.block_text),
+                line=self.lineno)
+            return [error]
+
         loader_options = {
             "source": source,
             "relative_resolved_path": relative_resolved_path,
@@ -181,12 +195,38 @@ def run(self):
                 "-", "_")  # make identifier-compatible if trivially possible
             loader_options.setdefault(k, v)  # do not overwrite
 
-        with self.loader(**loader_options) as data:
-            context = self._make_context(data, app.config, env)
-            rendered_template = render_function(
-                template,
-                context,
-            )
+        try:
+            with self.loader(**loader_options) as data:
+                context = self._make_context(data, app.config, env)
+                rendered_template = render_function(
+                    template,
+                    context,
+                )
+        except FileNotFoundError:
+            error = self.state_machine.reporter.error(
+                f"Source file '{relative_resolved_path}' not found",
+                nodes.literal_block(self.block_text, self.block_text),
+                line=self.lineno)
+            return [error]
+        except loaders.LoaderError as err:
+            error = self.state_machine.reporter.error(
+                f"Error in source '{relative_resolved_path}': {err}",
+                nodes.literal_block(self.block_text, self.block_text),
+                line=self.lineno)
+            return [error]
+        except jinja2.exceptions.TemplateNotFound:
+            error = self.state_machine.reporter.error(
+                f"Template file '{template}' not found",
+                nodes.literal_block(self.block_text, self.block_text),
+                line=self.lineno)
+            return [error]
+        except jinja2.exceptions.TemplateSyntaxError as err:
+            error = self.state_machine.reporter.error(
+                f"Error in template file '{template}' line {err.lineno}: "
+                f"{err.message}",
+                nodes.literal_block(self.block_text, self.block_text),
+                line=self.lineno)
+            return [error]
 
         result = ViewList()
         for line in rendered_template.splitlines():

sphinxcontrib/datatemplates/loaders.py

@@ -11,6 +11,10 @@
 registered_loaders = []
 
 
+class LoaderError(Exception):
+    pass
+
+
 class LoaderEntry:
     def __init__(self, loader, name, match_source):
         self.loader = loader
@@ -113,7 +117,10 @@ def load_csv(source,
 @contextlib.contextmanager
 def load_json(source, absolute_resolved_path, encoding='utf-8-sig', **options):
     with open(absolute_resolved_path, 'r', encoding=encoding) as f:
-        yield json.load(f)
+        try:
+            yield json.load(f)
+        except json.decoder.JSONDecodeError as error:
+            raise LoaderError(str(error)) from error
 
 
 @file_extension_loader("yaml", ['.yml', '.yaml'])
@@ -124,26 +131,41 @@ def load_yaml(source,
               multiple_documents=False,
               **options):
     with open(absolute_resolved_path, 'r', encoding=encoding) as f:
-        if multiple_documents:
-            yield list(
-                yaml.safe_load_all(f)
-            )  # force loading all documents now so the file can be closed
-        else:
-            yield yaml.safe_load(f)
+        try:
+            if multiple_documents:
+                yield list(
+                    yaml.safe_load_all(f)
+                )  # force loading all documents now so the file can be closed
+            else:
+                yield yaml.safe_load(f)
+        except yaml.error.MarkedYAMLError as error:
+            if error.context_mark.name == absolute_resolved_path:
+                error.context_mark.name = source
+            error.problem_mark.name = source
+            raise LoaderError(str(error)) from error
 
 
 @lenient_mimetype_loader('xml', 'xml')
 @contextlib.contextmanager
 def load_xml(source, absolute_resolved_path, **options):
-    yield ET.parse(absolute_resolved_path).getroot()
+    try:
+        yield ET.parse(absolute_resolved_path).getroot()
+    except ET.ParseError as error:
+        raise LoaderError(str(error)) from error
 
 
 @file_extension_loader("dbm", ['.dbm'])
 def load_dbm(source, absolute_resolved_path, **options):
-    return dbm.open(absolute_resolved_path, "r")
+    try:
+        return dbm.open(absolute_resolved_path, "r")
+    except dbm.error[0] as error:
+        raise LoaderError(str(error)) from error
 
 
 @data_source_loader("import-module")
 @contextlib.contextmanager
 def load_import_module(source, **options):
-    yield importlib.import_module(source)
+    try:
+        yield importlib.import_module(source)
+    except ModuleNotFoundError as error:
+        raise LoaderError(str(error)) from error

tests/conftest.py

@@ -1 +1,23 @@
+import pytest
+
+from pathlib import Path
+from sphinx import version_info as sphinx_version_info
+
 pytest_plugins = "sphinx.testing.fixtures"
+
+
+@pytest.fixture(scope="session")
+def rootdir():
+    """
+    Fixture for 'pytest.mark.sphinx'.
+
+    Test data is stored in the 'testdata/test-<testroot>'.
+    """
+    if sphinx_version_info >= (7, 2):
+        abs_path = Path(__file__).parent.absolute()
+    else:
+        from sphinx.testing.path import path
+
+        abs_path = path(__file__).parent.abspath()
+
+    return abs_path / "testdata"

tests/test_custom_template_bridge.py

@@ -1,35 +1,18 @@
 from __future__ import annotations
+import pytest
 
-import os
-import shutil
-import sys
 from pathlib import Path
 from typing import TYPE_CHECKING
 
 from bs4 import BeautifulSoup
-from sphinx import version_info as sphinx_version_info
-from sphinx.testing.util import SphinxTestApp
 
 if TYPE_CHECKING:
-    from typing import Callable
+    from sphinx.testing.util import SphinxTestApp
 
-if sphinx_version_info >= (7, 2):
-    _path = Path
-else:
-    from sphinx.testing.path import path
 
-    _path = path
-
-
-def test_custom_template_bridge(
-    tmp_path: Path,
-    make_app: Callable[..., SphinxTestApp],
-):
-    dataset_path = Path(os.path.dirname(__file__), "custom_template_bridge")
-    shutil.copytree(dataset_path, tmp_path, dirs_exist_ok=True)
-
-    app = make_app(srcdir=_path(tmp_path), warning=sys.stdout)
-    app.build()
+@pytest.mark.sphinx("html", testroot="custom-template-bridge")
+def test_custom_template_bridge(app: SphinxTestApp):
+    app.builder.build_all()
 
     assert app._warncount == 0, "\n".join(app.messagelog)
 

tests/test_errors.py

@@ -0,0 +1,125 @@
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+import pytest
+
+if TYPE_CHECKING:
+    from io import StringIO
+    from sphinx.testing.util import SphinxTestApp
+
+
+@pytest.mark.sphinx("html", testroot="empty-source")
+def test_empty_source(app: SphinxTestApp, warning: StringIO):
+    app.builder.build_all()
+    expected_error_str = (
+        f"{app.srcdir / 'index.rst'}:1: ERROR: Source file is required"
+    )
+    assert expected_error_str in warning.getvalue()
+
+
+@pytest.mark.sphinx("html", testroot="nonexistent-source")
+def test_nonexisting_source(app: SphinxTestApp, warning: StringIO):
+    app.builder.build_all()
+    expected_error_str = (
+        f"{app.srcdir / 'index.rst'}:1: "
+        "ERROR: Source file 'sample1.json' not found"
+    )
+    assert expected_error_str in warning.getvalue()
+
+
+@pytest.mark.sphinx("html", testroot="empty-template")
+def test_empty_template(app: SphinxTestApp, warning: StringIO):
+    app.builder.build_all()
+    expected_error_str = (
+        f"{app.srcdir / 'index.rst'}:1: ERROR: Template is empty"
+    )
+    assert expected_error_str in warning.getvalue()
+
+
+@pytest.mark.sphinx("html", testroot="nonexistent-template")
+def test_nonexistent_template(app: SphinxTestApp, warning: StringIO):
+    app.builder.build_all()
+    expected_error_str = (
+        f"{app.srcdir / 'index.rst'}:1: "
+        "ERROR: Template file 'sample1.tmpl' not found"
+    )
+    assert expected_error_str in warning.getvalue()
+
+
+@pytest.mark.sphinx("html", testroot="nonexistent-template-filter")
+def test_nonexistent_template_filter(app: SphinxTestApp, warning: StringIO):
+    app.builder.build_all()
+    expected_error_str = (
+        f"{app.srcdir / 'index.rst'}:1: "
+        "ERROR: Error in template file 'sample.tmpl' line 1: "
+        "No filter named 'some_filter'."
+    )
+    assert expected_error_str in warning.getvalue()
+
+
+@pytest.mark.sphinx("html", testroot="incorrect-template-syntax")
+def test_incorrect_template_syntax(app: SphinxTestApp, warning: StringIO):
+    app.builder.build_all()
+    expected_error_str = (
+        f"{app.srcdir / 'index.rst'}:1: "
+        "ERROR: Error in template file 'sample.tmpl' line 1: "
+        "unexpected '}'"
+    )
+    assert expected_error_str in warning.getvalue()
+
+
+@pytest.mark.sphinx("html", testroot="incorrect-json-syntax")
+def test_incorrect_json_syntax(app: SphinxTestApp, warning: StringIO):
+    app.builder.build_all()
+    expected_error_str = (
+        f"{app.srcdir / 'index.rst'}:1: "
+        "ERROR: Error in source 'sample.json': "
+        "Invalid control character at: line 2 column 28 (char 29)"
+    )
+    assert expected_error_str in warning.getvalue()
+
+
+@pytest.mark.sphinx("html", testroot="incorrect-yaml-syntax")
+def test_incorrect_yaml_syntax(app: SphinxTestApp, warning: StringIO):
+    app.builder.build_all()
+    expected_error_str = (
+        f"{app.srcdir / 'index.rst'}:1: "
+        "ERROR: Error in source 'sample.yaml': "
+        "while parsing a block collection\n"
+        '  in "sample.yaml", line 11, column 3\n'
+        "expected <block end>, but found '?'\n"
+        '  in "sample.yaml", line 12, column 3'
+    )
+    assert expected_error_str in warning.getvalue()
+
+
+@pytest.mark.sphinx("html", testroot="incorrect-xml-syntax")
+def test_incorrect_xml_syntax(app: SphinxTestApp, warning: StringIO):
+    app.builder.build_all()
+    expected_error_str = (
+        f"{app.srcdir / 'index.rst'}:1: "
+        "ERROR: Error in source 'sample.xml': "
+        "not well-formed (invalid token): line 2, column 4"
+    )
+    assert expected_error_str in warning.getvalue()
+
+
+@pytest.mark.sphinx("html", testroot="incorrect-import-module")
+def test_incorrect_import_module(app: SphinxTestApp, warning: StringIO):
+    app.builder.build_all()
+    expected_error_str = (
+        f"{app.srcdir / 'index.rst'}:1: "
+        "ERROR: Error in source 'some_module': No module named 'some_module'"
+    )
+    assert expected_error_str in warning.getvalue()
+
+
+@pytest.mark.sphinx("html", testroot="incorrect-dbm")
+def test_incorrect_dbm(app: SphinxTestApp, warning: StringIO):
+    app.builder.build_all()
+    expected_error_str = (
+        f"{app.srcdir / 'index.rst'}:1: "
+        "ERROR: Error in source 'sampledbm': "
+        "db type could not be determined"
+    )
+    assert expected_error_str in warning.getvalue()

tests/testdata/test-empty-source/conf.py

@@ -0,0 +1,2 @@
+extensions = ["sphinxcontrib.datatemplates"]
+templates_path = ["templates"]

tests/testdata/test-empty-source/index.rst

@@ -0,0 +1,2 @@
+.. datatemplate:json::
+    :template: sample.tmpl

tests/testdata/test-empty-source/sample.json

@@ -0,0 +1,3 @@
+{
+    "some_key": "some_value"
+}

tests/testdata/test-empty-source/templates/sample.tmpl

@@ -0,0 +1 @@
+{{data.some_key}}

tests/testdata/test-empty-template/conf.py

@@ -0,0 +1,2 @@
+extensions = ["sphinxcontrib.datatemplates"]
+templates_path = ["templates"]

tests/testdata/test-empty-template/index.rst

@@ -0,0 +1 @@
+.. datatemplate:json:: sample.json

tests/testdata/test-empty-template/sample.json

@@ -0,0 +1,3 @@
+{
+    "some_key": "some_value"
+}

tests/testdata/test-empty-template/templates/sample.tmpl

@@ -0,0 +1 @@
+{{data.some_key}}

tests/testdata/test-incorrect-dbm/conf.py

@@ -0,0 +1,2 @@
+extensions = ["sphinxcontrib.datatemplates"]
+templates_path = ["templates"]

tests/testdata/test-incorrect-dbm/index.rst

@@ -0,0 +1,2 @@
+.. datatemplate:dbm:: sampledbm
+    :template: sample.tmpl

tests/testdata/test-incorrect-dbm/sampledbm

@@ -0,0 +1 @@
+sample text

tests/testdata/test-incorrect-dbm/templates/sample.tmpl

@@ -0,0 +1 @@
+{{data}}

tests/testdata/test-incorrect-import-module/conf.py

@@ -0,0 +1,2 @@
+extensions = ["sphinxcontrib.datatemplates"]
+templates_path = ["templates"]

tests/testdata/test-incorrect-import-module/index.rst

@@ -0,0 +1,2 @@
+.. datatemplate:import-module:: some_module
+    :template: sample.tmpl

tests/testdata/test-incorrect-import-module/templates/sample.tmpl

@@ -0,0 +1 @@
+{{data}}

tests/testdata/test-incorrect-json-syntax/conf.py

@@ -0,0 +1,2 @@
+extensions = ["sphinxcontrib.datatemplates"]
+templates_path = ["templates"]

tests/testdata/test-incorrect-json-syntax/index.rst

@@ -0,0 +1,2 @@
+.. datatemplate:json:: sample.json
+    :template: sample.tmpl

tests/testdata/test-incorrect-json-syntax/sample.json

@@ -0,0 +1,3 @@
+{
+    "some_key": "some_value
+}