This is an automated email from the ASF dual-hosted git repository. aminghadersohi pushed a commit to branch mcp-rls-plugins-99978 in repository https://gitbox.apache.org/repos/asf/superset.git
commit e33d0d4f441e2023afb3cd5a6d0d04e5bcb88c7f Author: Elizabeth Thompson <[email protected]> AuthorDate: Thu May 21 09:19:29 2026 -0700 fix(reports): guard null dashboard height in Playwright screenshots (#40179) --- superset/utils/webdriver.py | 13 ++++- tests/unit_tests/utils/webdriver_test.py | 87 ++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/superset/utils/webdriver.py b/superset/utils/webdriver.py index a7f79e3b415..1ace36e349b 100644 --- a/superset/utils/webdriver.py +++ b/superset/utils/webdriver.py @@ -324,7 +324,10 @@ class WebDriverPlaywright(WebDriverProxy): 'document.querySelectorAll(".chart-container").length' ) dashboard_height = page.evaluate( - f'document.querySelector(".{element_name}").scrollHeight || 0' + f"""() => {{ + const target = document.querySelector(\".{element_name}\"); + return target ? target.scrollHeight : 0; + }}""" ) chart_threshold = app.config.get( "SCREENSHOT_TILED_CHART_THRESHOLD", 20 @@ -336,6 +339,14 @@ class WebDriverPlaywright(WebDriverProxy): "SCREENSHOT_TILED_VIEWPORT_HEIGHT", viewport_height ) + if dashboard_height == 0: + logger.warning( + "Could not determine dashboard height for element %s " + "at url %s; falling back to standard screenshot behavior", + element_name, + url, + ) + # Use tiled screenshots for large dashboards use_tiled = ( chart_count >= chart_threshold diff --git a/tests/unit_tests/utils/webdriver_test.py b/tests/unit_tests/utils/webdriver_test.py index b2e26269474..a27220908cc 100644 --- a/tests/unit_tests/utils/webdriver_test.py +++ b/tests/unit_tests/utils/webdriver_test.py @@ -811,3 +811,90 @@ class TestWebDriverPlaywrightErrorHandling: mock_logger.exception.assert_any_call( "Timed out requesting url %s", "http://example.com"; ) + + @patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True) + @patch("superset.utils.webdriver.sync_playwright") + @patch("superset.utils.webdriver.logger") + def test_missing_element_for_dashboard_height_falls_back_without_crashing( + self, mock_logger, mock_sync_playwright + ): + """Missing dashboard element should not crash height evaluation.""" + mock_user = MagicMock() + mock_user.username = "test_user" + + mock_playwright_instance = MagicMock() + mock_browser = MagicMock() + mock_context = MagicMock() + mock_page = MagicMock() + mock_element = MagicMock() + mock_chart_container = MagicMock() + + mock_sync_playwright.return_value.__enter__.return_value = ( + mock_playwright_instance + ) + mock_playwright_instance.chromium.launch.return_value = mock_browser + mock_browser.new_context.return_value = mock_context + mock_context.new_page.return_value = mock_page + + def locator_side_effect(selector): + if selector == ".dashboard": + return mock_element + if selector == ".chart-container": + locator = MagicMock() + locator.all.return_value = [mock_chart_container] + return locator + if selector == ".loading": + locator = MagicMock() + locator.all.return_value = [] + return locator + return MagicMock() + + mock_page.locator.side_effect = locator_side_effect + mock_element.wait_for.return_value = None + mock_element.screenshot.return_value = b"fake_screenshot" + mock_chart_container.wait_for.return_value = None + mock_page.wait_for_timeout.return_value = None + + def evaluate_side_effect(script): + if script == 'document.querySelectorAll(".chart-container").length': + return 1 + if "const target = document.querySelector" in script: + return 0 + return None + + mock_page.evaluate.side_effect = evaluate_side_effect + + with patch("superset.utils.webdriver.app") as mock_app: + mock_app.config = { + "WEBDRIVER_OPTION_ARGS": [], + "WEBDRIVER_WINDOW": {"pixel_density": 1}, + "SCREENSHOT_PLAYWRIGHT_DEFAULT_TIMEOUT": 30000, + "SCREENSHOT_PLAYWRIGHT_WAIT_EVENT": "networkidle", + "SCREENSHOT_SELENIUM_HEADSTART": 5, + "SCREENSHOT_SELENIUM_ANIMATION_WAIT": 1, + "SCREENSHOT_LOCATE_WAIT": 10, + "SCREENSHOT_LOAD_WAIT": 10, + "SCREENSHOT_WAIT_FOR_ERROR_MODAL_VISIBLE": 10, + "SCREENSHOT_WAIT_FOR_ERROR_MODAL_INVISIBLE": 10, + "SCREENSHOT_REPLACE_UNEXPECTED_ERRORS": False, + "SCREENSHOT_TILED_ENABLED": True, + "SCREENSHOT_TILED_CHART_THRESHOLD": 20, + "SCREENSHOT_TILED_HEIGHT_THRESHOLD": 5000, + "SCREENSHOT_TILED_VIEWPORT_HEIGHT": 600, + } + + with patch.object(WebDriverPlaywright, "auth") as mock_auth: + mock_auth.return_value = mock_context + + driver = WebDriverPlaywright("chrome") + result = driver.get_screenshot( + "http://example.com";, "dashboard", mock_user + ) + + assert result == b"fake_screenshot" + mock_logger.warning.assert_any_call( + "Could not determine dashboard height for element %s at url %s; " + "falling back to standard screenshot behavior", + "dashboard", + "http://example.com";, + )
