Merge branch 'main' into cam/465/revert-serialization-from-442

Author: camdecoster

CHANGELOG.md

@@ -8,6 +8,7 @@
 - Remove the `orjson` dependency [[#478](https://github.com/plotly/Kaleido/pull/478)]
 
 ### Fixed
+- Add the browser path to message in `ChromeNotFoundError` when it was set with `path=` or `BROWSER_PATH` and points to nothing [[#476](https://github.com/plotly/Kaleido/pull/476)], with thanks to @Blizzeq for the contribution!
 - Restore serialization of figure specs through the `plotly` JSON encoder [[#478](https://github.com/plotly/Kaleido/pull/478)]
 
 ## v1.4.0

src/py/kaleido/kaleido.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import asyncio
+import os
 import warnings
 from collections import deque
 from collections.abc import AsyncIterable, Iterable
@@ -184,12 +185,19 @@ def __init__(  # noqa: PLR0913
         try:
             super().__init__(**kwargs)
         except ChromeNotFoundError:
-            raise ChromeNotFoundError(
+            msg = (
                 "Kaleido v1 and later requires Chrome to be installed. "
                 "To install Chrome, use the CLI command `kaleido_get_chrome`, "
                 "or from Python, use either `await kaleido.get_chrome()` "
-                "or `kaleido.get_chrome_sync()`.",
-            ) from None  # overwriting the error entirely. (diagnostics)
+                "or `kaleido.get_chrome_sync()`."
+            )
+            # a path the user set explicitly is the likelier cause
+            if browser_path := kwargs.get("path"):
+                msg = f"No browser found at path={str(browser_path)!r}. {msg}"
+            elif browser_path := os.environ.get("BROWSER_PATH"):
+                msg = f"No browser found at BROWSER_PATH={browser_path!r}. {msg}"
+            # overwriting the error entirely. (diagnostics)
+            raise ChromeNotFoundError(msg) from None
 
         # save this for open() because it requires close()
         self._saved_page_arg = page

src/py/tests/test_kaleido.py

@@ -6,10 +6,12 @@
 from unittest.mock import AsyncMock, patch
 
 import pytest
+from choreographer.browsers import chromium
 from hypothesis import HealthCheck, given, settings
 from hypothesis import strategies as st
 
 from kaleido import Kaleido
+from kaleido.errors import ChromeNotFoundError
 
 if TYPE_CHECKING:
     from collections.abc import AsyncGenerator, Generator
@@ -338,6 +340,22 @@ async def test_kaleido_instantiate_and_close():
     await k.close()
 
 
+async def test_kaleido_reports_bad_path():
+    """Test that a browser path passed by the user is named in the error."""
+    with pytest.raises(ChromeNotFoundError, match="path='/nonexistent/chrome'"):
+        Kaleido(path="/nonexistent/chrome")
+
+
+async def test_kaleido_reports_bad_browser_path_env(monkeypatch):
+    """Test that a BROWSER_PATH set by the user is named in the error."""
+    monkeypatch.setenv("BROWSER_PATH", "/nonexistent/chrome")
+    # a downloaded chrome takes precedence over BROWSER_PATH, so hide it
+    monkeypatch.setattr(chromium, "get_chrome_download_path", lambda **_: None)
+    monkeypatch.setattr(chromium, "get_old_chrome_download_path", lambda: None)
+    with pytest.raises(ChromeNotFoundError, match="BROWSER_PATH='/nonexistent/chrome'"):
+        Kaleido()
+
+
 async def test_all_methods_context(simple_figure_with_bytes, tmp_path):
     """Test write, write_from_object, and calc with context."""
     fig = simple_figure_with_bytes["fig"]