Compare commits

...

1 Commits

Author SHA1 Message Date
Elizabeth Thompson
7730afa27c 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 <noreply@anthropic.com>
2026-06-16 00:10:38 +00:00
2 changed files with 78 additions and 8 deletions

View File

@@ -375,15 +375,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.

View File

@@ -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",
)