diff --git a/docs/diagram.md b/docs/diagram.md index a6512a75..6c49772c 100644 --- a/docs/diagram.md +++ b/docs/diagram.md @@ -486,6 +486,24 @@ JupyterLab cells — no extra code needed: ![Approval machine on JupyterLab](images/lab_approval_machine_accepted.png) +## When Graphviz fails + +Every path that runs `dot` (the `svg` format, `_repr_svg_()` in Jupyter, the command line +and the Sphinx directive) raises +{class}`~statemachine.contrib.diagram.GraphvizError` when the program is missing from +`PATH`, cannot run or exits with an error. The message carries the format that was requested and +what Graphviz wrote to stderr, such as +`Error: : syntax error in line 25 near ','`. The Sphinx directive reports the same +text as a build warning. + +Formats requested directly on the `pydot.Dot` object (`write_png()`, `create_svg()`, ...) +are run by pydot, which reports only the exit code. + +```{eval-rst} +.. autoexception:: statemachine.contrib.diagram.GraphvizError +``` + + ## Online generation (QuickChart) If you prefer not to install Graphviz locally, you can generate diagrams diff --git a/docs/images/readme_trafficlightmachine.png b/docs/images/readme_trafficlightmachine.png index 2f65f411..2df8eb5f 100644 Binary files a/docs/images/readme_trafficlightmachine.png and b/docs/images/readme_trafficlightmachine.png differ diff --git a/docs/releases/3.2.2.md b/docs/releases/3.2.2.md index 501eccdc..201b594f 100644 --- a/docs/releases/3.2.2.md +++ b/docs/releases/3.2.2.md @@ -333,6 +333,20 @@ queued behind one that reaches a final state is no longer processed. [#668](https://github.com/fgmacedo/python-statemachine/pull/668). +### Graphviz errors reported only the exit code + +A diagram that `dot` refused to render (the `svg` format, `_repr_svg_()` in Jupyter, the +command line or the Sphinx directive) failed with `"dot" with args [...] returned code: 1`, +while the reason, such as `Error: : syntax error in line 25 near ','`, was printed to +stdout by pydot, apart from the exception and from the Sphinx warning. These paths now run `dot` directly and raise +`GraphvizError` with the format that was requested and what Graphviz wrote to stderr. The +Sphinx directive reports the same text in its warning, and a missing `dot` says so, with a +link to the Graphviz download page. + +Reported by [@barnettwilliam](https://github.com/barnettwilliam). + +[#649](https://github.com/fgmacedo/python-statemachine/issues/649). + ## Documentation in 3.2.2 ### Platform events are not meant to be sent by user code diff --git a/statemachine/contrib/diagram/__init__.py b/statemachine/contrib/diagram/__init__.py index a5f5c368..4b37b8b0 100644 --- a/statemachine/contrib/diagram/__init__.py +++ b/statemachine/contrib/diagram/__init__.py @@ -5,6 +5,8 @@ from .extract import extract from .formatter import formatter as formatter +from .graphviz import GraphvizError as GraphvizError +from .graphviz import run_graphviz if TYPE_CHECKING: from .renderers.dot import DotRendererConfig @@ -207,13 +209,19 @@ def write_image(qualname, out, events=None, fmt=None): else: with open(out, "w") as f: f.write(text) + elif out == "-": + sys.stdout.buffer.write(_render_image(machine, "svg")) else: - graph = DotGraphMachine(machine).get_graph() - if out == "-": - sys.stdout.buffer.write(graph.create_svg()) # type: ignore[attr-defined] - else: - out_extension = out.rsplit(".", 1)[1] - graph.write(out, format=out_extension) + image = _render_image(machine, out.rsplit(".", 1)[1]) + with open(out, "wb") as f: + f.write(image) + + +def _render_image(machine, fmt: str) -> bytes: + dot_source: str = DotGraphMachine(machine).get_graph().to_string() + if fmt == "raw": + return dot_source.encode("utf-8") + return run_graphviz(dot_source, fmt) def main(argv=None): diff --git a/statemachine/contrib/diagram/formatter.py b/statemachine/contrib/diagram/formatter.py index 2027be31..f95e2abc 100644 --- a/statemachine/contrib/diagram/formatter.py +++ b/statemachine/contrib/diagram/formatter.py @@ -19,6 +19,8 @@ def _render_plantuml(machine): from collections.abc import Callable from typing import TYPE_CHECKING +from .graphviz import run_graphviz + if TYPE_CHECKING: from typing import TypeAlias @@ -108,8 +110,8 @@ def _render_dot(machine_or_class: "MachineRef") -> str: def _render_svg(machine_or_class: "MachineRef") -> str: from statemachine.contrib.diagram import DotGraphMachine - svg_bytes: bytes = DotGraphMachine(machine_or_class).get_graph().create_svg() # type: ignore[attr-defined] - return svg_bytes.decode("utf-8") + dot_source = DotGraphMachine(machine_or_class).get_graph().to_string() + return run_graphviz(dot_source, "svg").decode("utf-8") @formatter.register_format("mermaid") diff --git a/statemachine/contrib/diagram/graphviz.py b/statemachine/contrib/diagram/graphviz.py new file mode 100644 index 00000000..971607dd --- /dev/null +++ b/statemachine/contrib/diagram/graphviz.py @@ -0,0 +1,52 @@ +"""Run the Graphviz ``dot`` program and report its failures.""" + +import subprocess + +from statemachine.exceptions import StateMachineError + +GRAPHVIZ_DOWNLOAD_URL = "https://graphviz.org/download/" + + +class GraphvizError(StateMachineError): + """Raised when the Graphviz ``dot`` program cannot run or fails to render a graph.""" + + +def run_graphviz(dot_source: str, fmt: str, prog: str = "dot") -> bytes: + """Render ``dot_source`` with Graphviz and return the output bytes. + + Args: + dot_source: Graph in the DOT language. + fmt: Graphviz output format, passed as ``-T`` (``"svg"``, ``"png"``, ...). + prog: Graphviz layout program to run, looked up on ``PATH``. + + Raises: + GraphvizError: If ``prog`` cannot be run or exits with a non-zero code. + The message carries what Graphviz wrote to stderr. + """ + command = [prog, f"-T{fmt}"] + try: + completed = subprocess.run( + command, + input=dot_source.encode("utf-8"), + capture_output=True, + creationflags=getattr(subprocess, "CREATE_NO_WINDOW", 0), + ) + except FileNotFoundError as err: + raise GraphvizError( + f"{prog!r} not found in PATH. Install Graphviz from {GRAPHVIZ_DOWNLOAD_URL} " + "to generate diagrams." + ) from err + except OSError as err: + raise GraphvizError(f"{prog!r} could not be run: {err}") from err + + if completed.returncode != 0: + if completed.returncode < 0: + message = f"{' '.join(command)} was terminated by signal {-completed.returncode}." + else: + message = f"{' '.join(command)} failed with exit code {completed.returncode}." + stderr = completed.stderr.decode("utf-8", errors="replace").strip() + if stderr: + message = f"{message}\n{stderr}" + raise GraphvizError(message) + + return completed.stdout diff --git a/tests/scrape_images.py b/tests/scrape_images.py index 2547b536..16ca8d88 100644 --- a/tests/scrape_images.py +++ b/tests/scrape_images.py @@ -1,7 +1,7 @@ import os import re -from statemachine.contrib.diagram import DotGraphMachine +from statemachine.contrib.diagram import formatter from statemachine.factory import StateMachineMetaclass from .helpers import import_module_by_path @@ -31,7 +31,7 @@ def _get_module(self, src_file): def generate_image(self, sm_class, original_path): image_path = self.re_replace_png_extension.sub(".svg", original_path) - svg = DotGraphMachine(sm_class).get_graph().create_svg().decode() + svg = formatter.render(sm_class, "svg") with open(image_path, "w") as f: f.write(svg) return image_path diff --git a/tests/test_contrib_diagram.py b/tests/test_contrib_diagram.py index eb63df4a..52ddd737 100644 --- a/tests/test_contrib_diagram.py +++ b/tests/test_contrib_diagram.py @@ -1896,3 +1896,97 @@ def test_mermaid_render_failure_returns_warning(self, tmp_path): directive.state_machine.reporter.warning.assert_called_once() call_args = directive.state_machine.reporter.warning.call_args assert "failed to generate mermaid" in call_args[0][0] + + +class TestGraphvizErrors: + """Issue #649: a failing ``dot`` must report what Graphviz wrote to stderr.""" + + _QUALNAME = "tests.examples.traffic_light_machine.TrafficLightMachine" + _MALFORMED_DOT = "digraph { a -> ; }" + + @pytest.fixture() + def malformed_dot(self): + with mock.patch("pydot.Dot.to_string", return_value=self._MALFORMED_DOT): + yield + + def test_run_graphviz_reports_stderr_and_format(self): + from statemachine.contrib.diagram import GraphvizError + from statemachine.contrib.diagram.graphviz import run_graphviz + + with pytest.raises(GraphvizError) as excinfo: + run_graphviz(self._MALFORMED_DOT, "svg") + + message = str(excinfo.value) + assert "dot -Tsvg" in message + assert "exit code 1" in message + assert "syntax error" in message + assert "near ';'" in message + + def test_run_graphviz_succeeds(self): + from statemachine.contrib.diagram.graphviz import run_graphviz + + svg = run_graphviz("digraph { a -> b }", "svg") + assert svg.startswith(b" b }", "png") + assert png.startswith(b"\x89PNG") + + @pytest.mark.usefixtures("malformed_dot") + def test_format_svg_surfaces_stderr(self): + from statemachine.contrib.diagram import GraphvizError + + from tests.examples.traffic_light_machine import TrafficLightMachine + + sm = TrafficLightMachine() + with pytest.raises(GraphvizError, match="syntax error"): + format(sm, "svg") + + @pytest.mark.usefixtures("malformed_dot") + def test_repr_svg_surfaces_stderr(self): + from statemachine.contrib.diagram import GraphvizError + + from tests.examples.traffic_light_machine import TrafficLightMachine + + sm = TrafficLightMachine() + with pytest.raises(GraphvizError, match="syntax error"): + sm._repr_svg_() + + @pytest.mark.usefixtures("malformed_dot") + def test_write_image_to_file_surfaces_stderr(self, tmp_path): + from statemachine.contrib.diagram import GraphvizError + + out = tmp_path / "sm.png" + with pytest.raises(GraphvizError, match=r"(?s)dot -Tpng failed.*syntax error"): + main([self._QUALNAME, str(out)]) + + assert not out.exists() + + @pytest.mark.usefixtures("malformed_dot") + def test_write_image_to_stdout_surfaces_stderr(self, capsysbinary): + from statemachine.contrib.diagram import GraphvizError + + with pytest.raises(GraphvizError, match=r"(?s)dot -Tsvg failed.*syntax error"): + main([self._QUALNAME, "-"]) + + assert capsysbinary.readouterr().out == b"" + + @pytest.mark.usefixtures("malformed_dot") + def test_sphinx_warning_surfaces_stderr(self, tmp_path): + from statemachine.contrib.diagram.sphinx_ext import StateMachineDiagram + + directive = StateMachineDiagram.__new__(StateMachineDiagram) + directive.options = {} + directive.lineno = 1 + directive.state_machine = mock.MagicMock() + directive.state = mock.MagicMock() + directive.state.document.settings.env.app.outdir = str(tmp_path) + directive.content_offset = 0 + directive.arguments = [self._QUALNAME] + + result = directive.run() + + assert len(result) == 1 + directive.state_machine.reporter.warning.assert_called_once() + message = directive.state_machine.reporter.warning.call_args[0][0] + assert "failed to generate diagram" in message + assert "dot -Tsvg failed" in message + assert "syntax error" in message diff --git a/tests/test_contrib_diagram_graphviz.py b/tests/test_contrib_diagram_graphviz.py new file mode 100644 index 00000000..ae35c5ac --- /dev/null +++ b/tests/test_contrib_diagram_graphviz.py @@ -0,0 +1,72 @@ +"""Issue #649: ``run_graphviz`` errors that need no Graphviz installed.""" + +import subprocess +import sys + +import pytest +from statemachine.contrib.diagram import main + + +class TestGraphvizWithoutDot: + _QUALNAME = "tests.examples.traffic_light_machine.TrafficLightMachine" + + def test_run_graphviz_without_stderr_output(self, monkeypatch): + from statemachine.contrib.diagram import GraphvizError + from statemachine.contrib.diagram.graphviz import run_graphviz + + monkeypatch.setattr( + "subprocess.run", + lambda *args, **kwargs: subprocess.CompletedProcess(args, 3, b"", b""), + ) + + with pytest.raises(GraphvizError, match=r"dot -Tpng failed with exit code 3\.$"): + run_graphviz("digraph {}", "png") + + def test_run_graphviz_terminated_by_signal(self, monkeypatch): + from statemachine.contrib.diagram import GraphvizError + from statemachine.contrib.diagram.graphviz import run_graphviz + + monkeypatch.setattr( + "subprocess.run", + lambda *args, **kwargs: subprocess.CompletedProcess(args, -9, b"", b"killed"), + ) + + with pytest.raises(GraphvizError, match=r"dot -Tsvg was terminated by signal 9\.\nkilled"): + run_graphviz("digraph {}", "svg") + + def test_run_graphviz_without_dot_in_path(self, monkeypatch, tmp_path): + from statemachine.contrib.diagram import GraphvizError + from statemachine.contrib.diagram.graphviz import run_graphviz + + monkeypatch.setenv("PATH", str(tmp_path)) + + with pytest.raises(GraphvizError, match="'dot' not found in PATH") as excinfo: + run_graphviz("digraph {}", "svg") + + assert "graphviz.org/download" in str(excinfo.value) + assert isinstance(excinfo.value.__cause__, FileNotFoundError) + + @pytest.mark.skipif(sys.platform == "win32", reason="file mode bits are POSIX only") + def test_run_graphviz_with_dot_that_cannot_run(self, monkeypatch, tmp_path): + from statemachine.contrib.diagram import GraphvizError + from statemachine.contrib.diagram.graphviz import run_graphviz + + (tmp_path / "dot").write_text("") + monkeypatch.setenv("PATH", str(tmp_path)) + + with pytest.raises(GraphvizError, match="'dot' could not be run: .*Permission denied"): + run_graphviz("digraph {}", "svg") + + def test_graphviz_error_is_a_statemachine_error(self): + from statemachine.contrib.diagram import GraphvizError + from statemachine.exceptions import StateMachineError + + assert issubclass(GraphvizError, StateMachineError) + + def test_write_image_raw_writes_dot_source_without_running_dot(self, tmp_path, monkeypatch): + monkeypatch.setenv("PATH", str(tmp_path)) + out = tmp_path / "sm.raw" + + main([self._QUALNAME, str(out)]) + + assert out.read_text(encoding="utf-8").startswith("digraph TrafficLightMachine {")