From d2fcf7ec60aaa3fbaf8be7b52db95c0a7e7fb78f Mon Sep 17 00:00:00 2001 From: Jakub Krasuski Date: Fri, 18 Sep 2026 20:45:44 +0200 Subject: [PATCH] Name the browser path in ChromeNotFoundError When the user points Kaleido at a browser with path= or BROWSER_PATH and nothing is there, the error only told them to install Chrome. Now it names the path that was tried. --- CHANGELOG.md | 3 +++ src/py/kaleido/kaleido.py | 14 +++++++++++--- src/py/tests/test_kaleido.py | 18 ++++++++++++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd4fc87c..753bfa5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ ## Unreleased +### Fixed +- Name the browser path in `ChromeNotFoundError` when it was set with `path=` or `BROWSER_PATH` and points to nothing, instead of only suggesting to install Chrome [[issue #422](https://github.com/plotly/Kaleido/issues/422)] + ## v1.4.0 ### Fixed diff --git a/src/py/kaleido/kaleido.py b/src/py/kaleido/kaleido.py index 74d0789c..02d2b85c 100644 --- a/src/py/kaleido/kaleido.py +++ b/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 @@ -187,12 +188,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 diff --git a/src/py/tests/test_kaleido.py b/src/py/tests/test_kaleido.py index a2f213f6..c78abd69 100644 --- a/src/py/tests/test_kaleido.py +++ b/src/py/tests/test_kaleido.py @@ -6,6 +6,8 @@ from unittest.mock import AsyncMock, patch import pytest +from choreographer.browsers import chromium +from choreographer.errors import ChromeNotFoundError from hypothesis import HealthCheck, given, settings from hypothesis import strategies as st @@ -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"]