plots: return errors in json format (#9146)

* plots: return errors in json format

* plots --json: move plots data into data key

* rename  revision key to rev

* move imports to top-level

* add tests

dvc/commands/plots.py

@@ -1,8 +1,9 @@
 import argparse
 import logging
 import os
+from typing import TYPE_CHECKING, Dict, List
 
-from funcy import first
+from funcy import compact, first
 
 from dvc.cli import completion
 from dvc.cli.command import CmdBase
@@ -11,14 +12,43 @@ from dvc.exceptions import DvcException
 from dvc.ui import ui
 from dvc.utils import format_link
 
+if TYPE_CHECKING:
+    from dvc.render.match import RendererWithErrors
+
+
 logger = logging.getLogger(__name__)
 
 
-def _show_json(renderers, split=False):
+def _show_json(renderers_with_errors: List["RendererWithErrors"], split=False):
     from dvc.render.convert import to_json
+    from dvc.utils.serialize import encode_exception
 
-    result = {renderer.name: to_json(renderer, split) for renderer in renderers}
+    errors: List[Dict] = []
-    ui.write_json(result, highlight=False)
+    data = {}
+
+    for renderer, src_errors, def_errors in renderers_with_errors:
+        name = renderer.name
+        data[name] = to_json(renderer, split)
+        errors.extend(
+            {
+                "name": name,
+                "rev": rev,
+                "source": source,
+                **encode_exception(e),
+            }
+            for rev, per_rev_src_errors in src_errors.items()
+            for source, e in per_rev_src_errors.items()
+        )
+        errors.extend(
+            {
+                "name": name,
+                "rev": rev,
+                **encode_exception(e),
+            }
+            for rev, e in def_errors.items()
+        )
+
+    ui.write_json(compact({"errors": errors, "data": data}), highlight=False)
 
 
 def _adjust_vega_renderers(renderers):
@@ -122,15 +152,16 @@ class CmdPlots(CmdBase):
 
             renderers_out = out if self.args.json else os.path.join(out, "static")
 
-            renderers = match_defs_renderers(
+            renderers_with_errors = match_defs_renderers(
                 data=plots_data,
                 out=renderers_out,
                 templates_dir=self.repo.plots.templates_dir,
             )
             if self.args.json:
-                _show_json(renderers, self.args.split)
+                _show_json(renderers_with_errors, self.args.split)
                 return 0
 
+            renderers = [r.renderer for r in renderers_with_errors]
             _adjust_vega_renderers(renderers)
             if self.args.show_vega:
                 renderer = first(filter(lambda r: r.TYPE == "vega", renderers))

dvc/render/match.py

@@ -1,10 +1,10 @@
 import os
 from collections import defaultdict
-from typing import TYPE_CHECKING, Dict, List, Optional
+from typing import TYPE_CHECKING, DefaultDict, Dict, List, NamedTuple, Optional
 
 import dpath
 import dpath.options
-from funcy import last
+from funcy import get_in, last
 
 from dvc.repo.plots import _normpath, infer_data_sources
 from dvc.utils.plots import get_plot_id
@@ -13,6 +13,8 @@ from .convert import _get_converter
 
 if TYPE_CHECKING:
     from dvc.types import StrPath
+    from dvc_render.base import Renderer
+
 
 dpath.options.ALLOW_EMPTY_STRING_KEYS = True
 
@@ -61,21 +63,31 @@ class PlotsData:
         return result
 
 
-def match_defs_renderers(
+class RendererWithErrors(NamedTuple):
+    renderer: "Renderer"
+    source_errors: Dict[str, Dict[str, Exception]]
+    definition_errors: Dict[str, Exception]
+
+
+def match_defs_renderers(  # noqa: C901, PLR0912
     data,
     out=None,
     templates_dir: Optional["StrPath"] = None,
-):
+) -> List[RendererWithErrors]:
     from dvc_render import ImageRenderer, VegaRenderer
 
     plots_data = PlotsData(data)
     renderers = []
     renderer_cls = None
+
     for plot_id, group in plots_data.group_definitions().items():
         plot_datapoints: List[Dict] = []
         props = _squash_plots_properties(group)
         final_props: Dict = {}
 
+        def_errors: Dict[str, Exception] = {}
+        src_errors: DefaultDict[str, Dict[str, Exception]] = defaultdict(dict)
+
         if out is not None:
             props["out"] = out
         if templates_dir is not None:
@@ -94,13 +106,24 @@ def match_defs_renderers(
 
             converter = _get_converter(renderer_cls, inner_id, props, definitions_data)
 
-            dps, rev_props = converter.flat_datapoints(rev)
+            for src in plot_sources:
+                if error := get_in(data, [rev, "sources", "data", src, "error"]):
+                    src_errors[rev][src] = error
+
+            try:
+                dps, rev_props = converter.flat_datapoints(rev)
+            except Exception as e:  # noqa: BLE001, pylint: disable=broad-except
+                def_errors[rev] = e
+                continue
+
             if not final_props and rev_props:
                 final_props = rev_props
             plot_datapoints.extend(dps)
 
         if "title" not in final_props:
             final_props["title"] = renderer_id
+
         if renderer_cls is not None:
-            renderers.append(renderer_cls(plot_datapoints, renderer_id, **final_props))
+            renderer = renderer_cls(plot_datapoints, renderer_id, **final_props)
+            renderers.append(RendererWithErrors(renderer, dict(src_errors), def_errors))
     return renderers

dvc/repo/plots/__init__.py

@@ -20,7 +20,7 @@ from typing import (
 
 import dpath
 import dpath.options
-from funcy import distinct, first, project, reraise
+from funcy import first, ldistinct, project, reraise
 
 from dvc.exceptions import DvcException
 from dvc.utils import error_handler, errored_revisions, onerror_collect
@@ -356,7 +356,7 @@ def infer_data_sources(plot_id, config=None):
     if isinstance(x, dict):
         sources.append(first(x.keys()))
 
-    return distinct(source for source in sources)
+    return ldistinct(source for source in sources)
 
 
 def _matches(targets, config_file, plot_id):

tests/func/plots/test_show.py

@@ -418,5 +418,8 @@ def test_show_plots_defined_with_native_os_path(tmp_dir, dvc, scm, capsys):
     assert main(["plots", "show", "--json"]) == 0
     out, _ = capsys.readouterr()
     json_out = json.loads(out)
-    assert json_out[f"dvc.yaml::{top_level_plot}"]
+    assert "errors" not in json_out
-    assert json_out[stage_plot]
+
+    json_data = json_out["data"]
+    assert json_data[f"dvc.yaml::{top_level_plot}"]
+    assert json_data[stage_plot]

tests/integration/plots/test_plots.py

@@ -182,7 +182,13 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_
     html_path, json_result, split_json_result = call(capsys)
     html_result = extract_vega_specs(html_path, ["linear.json", "confusion.json"])
 
-    assert json_result["linear.json"][0]["content"]["data"][
+    assert "errors" not in json_result
+    assert "errors" not in split_json_result
+
+    json_data = json_result["data"]
+    split_json_data = split_json_result["data"]
+
+    assert json_data["linear.json"][0]["content"]["data"][
         "values"
     ] == _update_datapoints(
         linear_v1,
@@ -200,7 +206,7 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_
             REVISION_FIELD: "workspace",
         },
     )
-    assert json_result["confusion.json"][0]["content"]["data"][
+    assert json_data["confusion.json"][0]["content"]["data"][
         "values"
     ] == _update_datapoints(
         confusion_v1,
@@ -218,34 +224,40 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_
             REVISION_FIELD: "workspace",
         },
     )
-    verify_image(tmp_dir, "workspace", "image.png", image_v1, html_path, json_result)
+    verify_image(tmp_dir, "workspace", "image.png", image_v1, html_path, json_data)
 
     for plot in ["linear.json", "confusion.json"]:
         verify_vega(
             "workspace",
             html_result[plot],
-            json_result[plot],
+            json_data[plot],
-            split_json_result[plot],
+            split_json_data[plot],
         )
 
-    verify_vega_props("confusion.json", json_result, **confusion_props)
+    verify_vega_props("confusion.json", json_data, **confusion_props)
 
     image_v2, linear_v2, confusion_v2, confusion_props = next(repo_state)
 
     html_path, json_result, split_json_result = call(capsys, subcommand="diff")
     html_result = extract_vega_specs(html_path, ["linear.json", "confusion.json"])
 
-    verify_image(tmp_dir, "workspace", "image.png", image_v2, html_path, json_result)
+    assert "errors" not in json_result
-    verify_image(tmp_dir, "HEAD", "image.png", image_v1, html_path, json_result)
+    assert "errors" not in split_json_result
+
+    json_data = json_result["data"]
+    split_json_data = split_json_result["data"]
+
+    verify_image(tmp_dir, "workspace", "image.png", image_v2, html_path, json_data)
+    verify_image(tmp_dir, "HEAD", "image.png", image_v1, html_path, json_data)
 
     for plot in ["linear.json", "confusion.json"]:
         verify_vega(
             ["HEAD", "workspace"],
             html_result[plot],
-            json_result[plot],
+            json_data[plot],
-            split_json_result[plot],
+            split_json_data[plot],
         )
-    verify_vega_props("confusion.json", json_result, **confusion_props)
+    verify_vega_props("confusion.json", json_data, **confusion_props)
     path = tmp_dir / "subdir"
     path.mkdir()
     with path.chdir():
@@ -254,7 +266,13 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_
             html_path,
             ["../linear.json", "../confusion.json"],
         )
-        assert json_result["../linear.json"][0]["content"]["data"][
+
+        assert "errors" not in json_result
+        assert "errors" not in split_json_result
+
+        json_data = json_result["data"]
+        split_json_data = split_json_result["data"]
+        assert json_data["../linear.json"][0]["content"]["data"][
             "values"
         ] == _update_datapoints(
             linear_v2,
@@ -286,7 +304,7 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_
                 REVISION_FIELD: "HEAD",
             },
         )
-        assert json_result["../confusion.json"][0]["content"]["data"][
+        assert json_data["../confusion.json"][0]["content"]["data"][
             "values"
         ] == _update_datapoints(
             confusion_v2,
@@ -326,8 +344,8 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_
             verify_vega(
                 ["HEAD", "workspace"],
                 html_result[plot],
-                json_result[plot],
+                json_data[plot],
-                split_json_result[plot],
+                split_json_data[plot],
             )
         verify_image(
             path,
@@ -335,7 +353,7 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_
             "../image.png",
             image_v2,
             html_path,
-            json_result,
+            json_data,
         )
         verify_image(
             path,
@@ -343,7 +361,7 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_
             "../image.png",
             image_v1,
             html_path,
-            json_result,
+            json_data,
         )
 
 
@@ -361,13 +379,30 @@ def test_repo_with_removed_plots(tmp_dir, capsys, repo_with_plots):
 
     for s in {"show", "diff"}:
         _, json_result, split_json_result = call(capsys, subcommand=s)
-        for p in {
+        errors = [
-            "linear.json",
+            {
-            "confusion.json",
+                "name": p,
-            "image.png",
+                "source": p,
-        }:
+                "rev": "workspace",
-            assert json_result[p] == []
+                "type": "FileNotFoundError",
-            assert split_json_result[p] == []
+                "msg": "",
+            }
+            for p in [
+                "linear.json",
+                "confusion.json",
+                "image.png",
+            ]
+        ]
+        expected_result = {
+            "errors": errors,
+            "data": {
+                "image.png": [],
+                "confusion.json": [],
+                "linear.json": [],
+            },
+        }
+        assert json_result == expected_result
+        assert split_json_result == expected_result
 
 
 def test_config_output_dir(tmp_dir, dvc, capsys):

tests/unit/command/test_plots.py

@@ -7,6 +7,7 @@ import pytest
 
 from dvc.cli import parse_args
 from dvc.commands.plots import CmdPlotsDiff, CmdPlotsShow, CmdPlotsTemplates
+from dvc.render.match import RendererWithErrors
 
 
 @pytest.fixture
@@ -268,8 +269,11 @@ def test_should_call_render(tmp_dir, mocker, capsys, plots_data, output):
 
     output = output or "dvc_plots"
     index_path = tmp_dir / output / "index.html"
-    renderers = mocker.MagicMock()
+    renderer = mocker.MagicMock()
-    mocker.patch("dvc.render.match.match_defs_renderers", return_value=renderers)
+    mocker.patch(
+        "dvc.render.match.match_defs_renderers",
+        return_value=[RendererWithErrors(renderer, {}, {})],
+    )
     render_mock = mocker.patch("dvc_render.render_html", return_value=index_path)
 
     assert cmd.run() == 0
@@ -278,7 +282,7 @@ def test_should_call_render(tmp_dir, mocker, capsys, plots_data, output):
     assert index_path.as_uri() in out
 
     render_mock.assert_called_once_with(
-        renderers=renderers,
+        renderers=[renderer],
         output_file=Path(tmp_dir / output / "index.html"),
         html_template=None,
     )
@@ -354,12 +358,13 @@ def test_show_json(split, mocker, capsys):
     import dvc.commands.plots
 
     renderer = mocker.MagicMock()
+    renderer_obj = RendererWithErrors(renderer, {}, {})
     renderer.name = "rname"
     to_json_mock = mocker.patch(
         "dvc.render.convert.to_json", return_value={"renderer": "json"}
     )
 
-    dvc.commands.plots._show_json([renderer], split)
+    dvc.commands.plots._show_json([renderer_obj], split)
 
     to_json_mock.assert_called_once_with(renderer, split)
 

tests/unit/render/test_match.py

@@ -1,4 +1,7 @@
+from funcy import set_in
+
 from dvc.render import VERSION_FIELD
+from dvc.render.converter.vega import VegaConverter
 from dvc.render.match import PlotsData, _squash_plots_properties, match_defs_renderers
 
 
@@ -35,7 +38,7 @@ def test_group_definitions():
     }
 
 
-def test_match_renderers(mocker):
+def test_match_renderers(M):
     data = {
         "v1": {
             "definitions": {
@@ -77,9 +80,9 @@ def test_match_renderers(mocker):
         },
     }
 
-    renderers = match_defs_renderers(data)
+    (renderer_with_errors,) = match_defs_renderers(data)
-    assert len(renderers) == 1
+    renderer = renderer_with_errors[0]
-    assert renderers[0].datapoints == [
+    assert renderer.datapoints == [
         {
             VERSION_FIELD: {
                 "revision": "v1",
@@ -99,13 +102,31 @@ def test_match_renderers(mocker):
             "y": 2,
         },
     ]
-    assert renderers[0].properties == {
+    assert renderer.properties == {
         "title": "config_file_1::plot_id_1",
         "x": "x",
         "y": "y",
         "x_label": "x",
         "y_label": "y",
     }
+    assert renderer_with_errors.source_errors == {
+        "revision_with_no_data": {"file.json": M.instance_of(FileNotFoundError)}
+    }
+    assert not renderer_with_errors.definition_errors
+
+
+def test_flat_datapoints_errors_are_caught(M, mocker):
+    d = {}
+    d = set_in(
+        d,
+        ["v1", "definitions", "data", "dvc.yaml", "data", "plot_id_1"],
+        {"x": "x", "y": {"file.json": "y"}},
+    )
+    d = set_in(d, ["v1", "sources", "data", "file.json", "data"], [{"x": 1, "y": 1}])
+    mocker.patch.object(VegaConverter, "flat_datapoints", side_effect=ValueError)
+    (renderer_with_errors,) = match_defs_renderers(d)
+    assert not renderer_with_errors.source_errors
+    assert renderer_with_errors.definition_errors == {"v1": M.instance_of(ValueError)}
 
 
 def test_squash_plots_properties():