You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by el...@apache.org on 2026/06/12 23:33:44 UTC
(superset) branch fix/reports-echarts-animation-wait created (now d40415650f8)
This is an automated email from the ASF dual-hosted git repository.
eschutho pushed a change to branch fix/reports-echarts-animation-wait
in repository https://gitbox.apache.org/repos/asf/superset.git
at d40415650f8 fix(reports): fail report on tiled screenshot error instead of sending blank PDF
This branch includes the following new commits:
new d40415650f8 fix(reports): fail report on tiled screenshot error instead of sending blank PDF
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/01: fix(reports): fail report on tiled screenshot error instead of sending blank PDF
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
eschutho pushed a commit to branch fix/reports-echarts-animation-wait
in repository https://gitbox.apache.org/repos/asf/superset.git
commit d40415650f83a0751f0d43004d627cbcbe97701b
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Fri Jun 12 23:33:13 2026 +0000
fix(reports): fail report on tiled screenshot error instead of sending blank PDF
When take_tiled_screenshot returns None, the previous fallback immediately took
a standard element.screenshot() with no spinner check or animation wait, which
produced a blank image that silently became a blank PDF sent to recipients.
Remove the fallback: return None instead, which propagates up through
_get_screenshots() → ReportScheduleScreenshotFailedError → ERROR execution
state with owner notification.
Co-Authored-By: Claude Sonnet 4.6 <no...@anthropic.com>
---
superset/utils/webdriver.py | 13 +++---
tests/unit_tests/utils/webdriver_test.py | 73 ++++++++++++++++++++++++++++++++
2 files changed, 78 insertions(+), 8 deletions(-)
diff --git a/superset/utils/webdriver.py b/superset/utils/webdriver.py
index 2a48a90b2da..5db82e44dc4 100644
--- a/superset/utils/webdriver.py
+++ b/superset/utils/webdriver.py
@@ -372,15 +372,12 @@ class WebDriverPlaywright(WebDriverProxy):
animation_wait=selenium_animation_wait,
)
if img is None:
- logger.warning(
- (
- "Tiled screenshot failed, "
- "falling back to standard screenshot"
- )
- )
- img = WebDriverPlaywright._get_screenshot(
- page, element, element_name
+ logger.error(
+ "Tiled screenshot failed at url %s; "
+ "not falling back to avoid sending a blank PDF",
+ url,
)
+ return None
else:
# Standard screenshot captures the full element including
# below-the-fold content, so wait for all spinners globally.
diff --git a/tests/unit_tests/utils/webdriver_test.py b/tests/unit_tests/utils/webdriver_test.py
index a27220908cc..2f2ecc1ae3f 100644
--- a/tests/unit_tests/utils/webdriver_test.py
+++ b/tests/unit_tests/utils/webdriver_test.py
@@ -898,3 +898,76 @@ class TestWebDriverPlaywrightErrorHandling:
"dashboard",
"http://example.com",
)
+
+ @patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True)
+ @patch("superset.utils.webdriver.sync_playwright")
+ @patch("superset.utils.webdriver.logger")
+ @patch("superset.utils.webdriver.take_tiled_screenshot")
+ def test_tiled_screenshot_failure_returns_none_without_fallback(
+ self, mock_take_tiled, mock_logger, mock_sync_playwright
+ ):
+ """When take_tiled_screenshot fails, return None rather than fall back to a
+ potentially blank standard screenshot."""
+ 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_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
+ mock_page.locator.return_value = mock_element
+ mock_element.wait_for.return_value = None
+ mock_element.screenshot.return_value = b"should_not_be_called"
+
+ def evaluate_side_effect(script):
+ if "querySelectorAll" in script:
+ return 25 # chart_count >= threshold
+ if "const target" in script:
+ return 6000 # dashboard_height > height_threshold and > tile_height
+ return None
+
+ mock_page.evaluate.side_effect = evaluate_side_effect
+ mock_take_tiled.return_value = None # tiled screenshot fails
+
+ 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": 0,
+ "SCREENSHOT_SELENIUM_ANIMATION_WAIT": 0,
+ "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 is None
+ mock_element.screenshot.assert_not_called()
+ mock_logger.error.assert_any_call(
+ "Tiled screenshot failed at url %s; "
+ "not falling back to avoid sending a blank PDF",
+ "http://example.com",
+ )