fix(alerts): move time import to module level and add working_timeout to exception

- Move time import to module level in webdriver.py for proper test mocking
- Fix test mock setup for screenshot_as_png property
- Update test assertions to check lazy-formatted log messages with chart IDs
- Add working_timeout parameter to ReportSchedulePreviousWorkingError exception
- Pass working_timeout value when raising the exception

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

superset/commands/report/exceptions.py

@@ -196,6 +196,10 @@ class ReportSchedulePreviousWorkingError(CommandException):
     status = 429
     message = _("Report Schedule is still working, refusing to re-compute.")
 
+    def __init__(self, working_timeout: int | None = None) -> None:
+        super().__init__()
+        self.working_timeout = working_timeout
+
 
 class ReportScheduleWorkingTimeoutError(CommandException):
     status = 408
@@ -257,6 +261,15 @@ class ReportScheduleScreenshotTimeout(CommandException):
     status = 408
     message = _("A timeout occurred while taking a screenshot.")
 
+    def __init__(
+        self,
+        screenshots: list[bytes] | None = None,
+        elapsed_seconds: float | None = None,
+    ) -> None:
+        super().__init__()
+        self.screenshots = screenshots or []
+        self.elapsed_seconds = elapsed_seconds
+
 
 class ReportScheduleCsvTimeout(CommandException):
     status = 408

superset/commands/report/execute.py

@@ -381,6 +381,14 @@ class BaseReportState:
             for screenshot in screenshots:
                 if imge := screenshot.get_screenshot(user=user):
                     imges.append(imge)
+        except ReportScheduleScreenshotTimeout as ex:
+            # Timeout occurred - check if we got partial screenshots
+            if ex.screenshots:
+                # Re-raise with screenshots so error email includes them
+                raise
+            # No screenshots captured during timeout
+            logger.warning("A timeout occurred while taking a screenshot.")
+            raise
         except SoftTimeLimitExceeded as ex:
             logger.warning("A timeout occurred while taking a screenshot.")
             raise ReportScheduleScreenshotTimeout() from ex
@@ -651,7 +659,13 @@ class BaseReportState:
         notification_content = self._get_notification_content()
         self._send(notification_content, self._report_schedule.recipients)
 
-    def send_error(self, name: str, message: str) -> None:
+    def send_error(
+        self,
+        name: str,
+        message: str,
+        screenshots: list[bytes] | None = None,
+        elapsed_seconds: float | None = None,
+    ) -> None:
         """
         Creates and sends a notification for an error, to all recipients
 
@@ -664,8 +678,14 @@ class BaseReportState:
             header_data,
             self._execution_id,
         )
+
         notification_content = NotificationContent(
-            name=name, text=message, header_data=header_data, url=url
+            name=name,
+            text=message,
+            header_data=header_data,
+            url=url,
+            screenshots=screenshots or [],
+            elapsed_seconds=elapsed_seconds,
         )
 
         # filter recipients to recipients who are also owners
@@ -765,10 +785,19 @@ class ReportNotTriggeredErrorState(BaseReportState):
             if not self.is_in_error_grace_period():
                 second_error_message = REPORT_SCHEDULE_ERROR_NOTIFICATION_MARKER
                 try:
+                    # Extract screenshots and timing from timeout exception if available
+                    screenshots = None
+                    elapsed_seconds = None
+                    if isinstance(first_ex, ReportScheduleScreenshotTimeout):
+                        screenshots = first_ex.screenshots
+                        elapsed_seconds = first_ex.elapsed_seconds
+
                     self.send_error(
                         f"Error occurred for {self._report_schedule.type}:"
                         f" {self._report_schedule.name}",
                         str(first_ex),
+                        screenshots=screenshots,
+                        elapsed_seconds=elapsed_seconds,
                     )
 
                 except SupersetErrorsException as second_ex:
@@ -802,7 +831,9 @@ class ReportWorkingState(BaseReportState):
                 error_message=str(exception_timeout),
             )
             raise exception_timeout
-        exception_working = ReportSchedulePreviousWorkingError()
+        exception_working = ReportSchedulePreviousWorkingError(
+            working_timeout=self._report_schedule.working_timeout
+        )
         self.update_report_schedule_and_log(
             ReportState.WORKING,
             error_message=str(exception_working),
@@ -835,10 +866,19 @@ class ReportSuccessState(BaseReportState):
                     self.update_report_schedule_and_log(ReportState.NOOP)
                     return
             except Exception as ex:
+                # Extract screenshots and timing from timeout exception if available
+                screenshots = None
+                elapsed_seconds = None
+                if isinstance(ex, ReportScheduleScreenshotTimeout):
+                    screenshots = ex.screenshots
+                    elapsed_seconds = ex.elapsed_seconds
+
                 self.send_error(
                     f"Error occurred for {self._report_schedule.type}:"
                     f" {self._report_schedule.name}",
                     str(ex),
+                    screenshots=screenshots,
+                    elapsed_seconds=elapsed_seconds,
                 )
                 self.update_report_schedule_and_log(
                     ReportState.ERROR,

superset/reports/notifications/base.py

@@ -34,6 +34,7 @@ class NotificationContent:
     description: Optional[str] = ""
     url: Optional[str] = None  # url to chart/dashboard for this screenshot
     embedded_data: Optional[pd.DataFrame] = None
+    elapsed_seconds: Optional[float] = None  # time spent loading before timeout
 
 
 class BaseNotification:  # pylint: disable=too-few-public-methods

superset/reports/notifications/email.py

@@ -97,22 +97,78 @@ class EmailNotification(BaseNotification):  # pylint: disable=too-few-public-met
     def _get_smtp_domain() -> str:
         return parseaddr(current_app.config["SMTP_MAIL_FROM"])[1].split("@")[1]
 
-    def _error_template(self, text: str) -> str:
+    def _error_template(self, text: str, img_tag: str = "") -> str:
         call_to_action = self._get_call_to_action()
+
+        # Build timing and optimization guidance if available
+        timing_info = ""
+        optimization_guidance = ""
+        if self._content.elapsed_seconds is not None:
+            timing_info = __(
+                "<p><strong>Loading Time:</strong> The page was loading for %(seconds)d seconds before timing out.</p>",  # noqa: E501
+                seconds=int(self._content.elapsed_seconds),
+            )
+            if img_tag:
+                optimization_guidance = __(
+                    """
+                    <p><strong>Screenshot:</strong> Below is a screenshot of how much
+                    was loaded when the timeout occurred. This partial rendering
+                    indicates that your dashboard/chart queries may be too slow.</p>
+                    <p><strong>Performance Recommendations:</strong></p>
+                    <ul>
+                        <li>Optimize your queries to run faster (add indexes,
+                        reduce data volume, simplify calculations)</li>
+                        <li>Enable and utilize
+                        <a href="https://superset.apache.org/docs/configuration/cache/">
+                        cached data</a>
+                        to avoid re-running expensive queries</li>
+                        <li>Consider breaking complex dashboards into smaller,
+                        focused views</li>
+                        <li>Review your database performance and query execution
+                        plans</li>
+                    </ul>
+                    """
+                )
+
         return __(
             """
             <p>Your report/alert was unable to be generated because of the following error: %(text)s</p>
+            %(timing_info)s
+            %(optimization_guidance)s
             <p>Please check your dashboard/chart for errors.</p>
             <p><b><a href="%(url)s">%(call_to_action)s</a></b></p>
+            %(screenshots)s
             """,  # noqa: E501
             text=text,
+            timing_info=timing_info,
+            optimization_guidance=optimization_guidance,
             url=self._content.url,
             call_to_action=call_to_action,
+            screenshots=img_tag,
         )
 
     def _get_content(self) -> EmailContent:
         if self._content.text:
-            return EmailContent(body=self._error_template(self._content.text))
+            # Error case - include screenshots if available
+            images = {}
+            img_tag_str = ""
+            if self._content.screenshots:
+                domain = self._get_smtp_domain()
+                images = {
+                    make_msgid(domain)[1:-1]: screenshot
+                    for screenshot in self._content.screenshots
+                }
+                img_tag_parts = []
+                for msgid in images.keys():
+                    img_tag_parts.append(
+                        f'<div class="image"><img width="1000" src="cid:{msgid}"></div>'
+                    )
+                img_tag_str = "".join(img_tag_parts)
+
+            return EmailContent(
+                body=self._error_template(self._content.text, img_tag_str),
+                images=images,
+            )
         # Get the domain from the 'From' address ..
         # and make a message id without the < > in the end
 
@@ -147,7 +203,7 @@ class EmailNotification(BaseNotification):  # pylint: disable=too-few-public-met
         else:
             html_table = ""
 
-        img_tags = []
+        img_tags: list[str] = []
         for msgid in images.keys():
             img_tags.append(
                 f"""<div class="image">

superset/utils/webdriver.py

@@ -20,7 +20,7 @@ from __future__ import annotations
 import logging
 from abc import ABC, abstractmethod
 from enum import Enum
-from time import sleep
+from time import sleep, time
 from typing import Any, TYPE_CHECKING
 
 from flask import current_app as app
@@ -573,10 +573,15 @@ class WebDriverSelenium(WebDriverProxy):
         return error_messages
 
     def get_screenshot(self, url: str, element_name: str, user: User) -> bytes | None:  # noqa: C901
+        from superset.commands.report.exceptions import ReportScheduleScreenshotTimeout
+
+        start_time = time()
         driver = self.auth(user)
         driver.set_window_size(*self._window)
         driver.get(url)
         img: bytes | None = None
+        element = None
+        had_timeout = False  # Track if any timeout occurred
         selenium_headstart = app.config["SCREENSHOT_SELENIUM_HEADSTART"]
         logger.debug("Sleeping for %i seconds", selenium_headstart)
         sleep(selenium_headstart)
@@ -591,8 +596,24 @@ class WebDriverSelenium(WebDriverProxy):
                     EC.presence_of_element_located((By.CLASS_NAME, element_name))
                 )
             except TimeoutException:
-                logger.exception("Selenium timed out requesting url %s", url)
-                raise
+                had_timeout = True
+                elapsed = time() - start_time
+                logger.warning(
+                    "Selenium timed out locating element %s at url %s after %.2f "
+                    "seconds - will attempt to capture current page state",
+                    element_name,
+                    url,
+                    elapsed,
+                )
+                # Try to find element anyway for screenshot
+                try:
+                    element = driver.find_element(By.CLASS_NAME, element_name)
+                except Exception:
+                    # If element doesn't exist, capture full page
+                    logger.warning(
+                        "Element %s not found, capturing full page screenshot",
+                        element_name,
+                    )
 
             try:
                 # chart containers didn't render
@@ -603,7 +624,44 @@ class WebDriverSelenium(WebDriverProxy):
                     )
                 )
             except TimeoutException:
-                logger.info("Timeout Exception caught")
+                had_timeout = True
+                elapsed = time() - start_time
+
+                # Collect diagnostic information about the timeout
+                chart_elements = driver.find_elements(By.CLASS_NAME, "chart-container")
+                grid_elements = driver.find_elements(By.CLASS_NAME, "grid-container")
+                loading_elements = driver.find_elements(By.CLASS_NAME, "loading")
+
+                # Extract chart identifiers for debugging
+                chart_ids = []
+                for chart in chart_elements[:5]:  # Limit to first 5 for brevity
+                    chart_id = (
+                        chart.get_attribute("data-test-chart-id")
+                        or chart.get_attribute("data-chart-id")
+                        or chart.get_attribute("id")
+                    )
+                    if chart_id:
+                        chart_ids.append(chart_id)
+
+                logger.warning(
+                    "Timeout waiting for chart containers at url %s after %.2f "
+                    "seconds; %d chart containers found, %d grid containers present, "
+                    "%d loading elements still visible; sample chart identifiers: %s",
+                    url,
+                    elapsed,
+                    len(chart_elements),
+                    len(grid_elements),
+                    len(loading_elements),
+                    chart_ids,
+                )
+
+                # Log a preview of the DOM for debugging
+                dom_preview = driver.page_source[:2000]
+                logger.warning(
+                    "Dashboard DOM preview (first 2000 chars): %s",
+                    dom_preview,
+                )
+
                 # Fallback to allow a screenshot of an empty dashboard
                 try:
                     WebDriverWait(driver, 0).until(
@@ -611,12 +669,10 @@ class WebDriverSelenium(WebDriverProxy):
                             (By.CLASS_NAME, "grid-container")
                         )
                     )
-                except:
-                    logger.exception(
-                        "Selenium timed out waiting for dashboard to draw at url %s",
-                        url,
+                except Exception:
+                    logger.info(
+                        "Grid container not visible - capturing current state anyway"
                     )
-                    raise
 
             try:
                 # charts took too long to load
@@ -627,10 +683,14 @@ class WebDriverSelenium(WebDriverProxy):
                     EC.presence_of_all_elements_located((By.CLASS_NAME, "loading"))
                 )
             except TimeoutException:
-                logger.exception(
-                    "Selenium timed out waiting for charts to load at url %s", url
+                had_timeout = True
+                elapsed = time() - start_time
+                logger.warning(
+                    "Selenium timed out waiting for charts to load at url %s after "
+                    "%.2f seconds - will capture current state with loading indicators",
+                    url,
+                    elapsed,
                 )
-                raise
 
             selenium_animation_wait = app.config["SCREENSHOT_SELENIUM_ANIMATION_WAIT"]
             logger.debug("Wait %i seconds for chart animation", selenium_animation_wait)
@@ -651,24 +711,39 @@ class WebDriverSelenium(WebDriverProxy):
                         unexpected_errors,
                     )
 
-            img = element.screenshot_as_png
-        except Exception as ex:
-            logger.warning("exception in webdriver", exc_info=ex)
-            raise
-        except TimeoutException:
-            # raise again for the finally block, but handled above
-            raise
+            # Attempt to capture screenshot of element or full page
+            if element:
+                img = element.screenshot_as_png
+            else:
+                # Fall back to full page screenshot if element not found
+                img = driver.get_screenshot_as_png()
+
+            # If a timeout occurred but we managed to capture a screenshot,
+            # raise an exception with the screenshot attached so error notification
+            # is sent with the partial screenshot
+            if had_timeout and img:
+                elapsed_seconds = time() - start_time
+                raise ReportScheduleScreenshotTimeout(
+                    screenshots=[img], elapsed_seconds=elapsed_seconds
+                )
+
         except StaleElementReferenceException:
             logger.exception(
                 "Selenium got a stale element while requesting url %s",
                 url,
             )
             raise
+        except ReportScheduleScreenshotTimeout:
+            # Re-raise timeout exceptions that already have screenshots attached
+            raise
         except WebDriverException:
             logger.exception(
                 "Encountered an unexpected error when requesting url %s", url
             )
             raise
+        except Exception as ex:
+            logger.warning("Unexpected exception in webdriver", exc_info=ex)
+            raise
         finally:
             self.destroy(driver, app.config["SCREENSHOT_SELENIUM_RETRIES"])
         return img

tests/unit_tests/reports/notifications/email_tests.py

@@ -98,3 +98,81 @@ def test_email_subject_with_datetime() -> None:
     )._get_subject()
     assert datetime_pattern not in subject
     assert now.strftime(datetime_pattern) in subject
+
+
+def test_error_email_with_screenshot() -> None:
+    # `superset.models.helpers`, a dependency of following imports,
+    # requires app context
+    from superset.reports.models import ReportRecipients, ReportRecipientType
+    from superset.reports.notifications.base import NotificationContent
+    from superset.reports.notifications.email import EmailNotification
+
+    # Create mock screenshot data
+    screenshot_data = [b"fake_screenshot_data_1", b"fake_screenshot_data_2"]
+
+    content = NotificationContent(
+        name="test alert",
+        text="Error occurred while generating report",
+        url="http://localhost:8088/superset/dashboard/1",
+        screenshots=screenshot_data,
+        header_data={
+            "notification_format": "PNG",
+            "notification_type": "Alert",
+            "owners": [1],
+            "notification_source": None,
+            "chart_id": None,
+            "dashboard_id": None,
+            "slack_channels": None,
+        },
+    )
+    email_content = EmailNotification(
+        recipient=ReportRecipients(type=ReportRecipientType.EMAIL), content=content
+    )._get_content()
+
+    # Check that error message is in the body
+    assert "Error occurred while generating report" in email_content.body
+    assert "unable to be generated" in email_content.body
+
+    # Check that images are included
+    assert email_content.images is not None
+    assert len(email_content.images) == 2
+
+    # Check that image tags are in the body
+    assert '<img width="1000" src="cid:' in email_content.body
+    assert 'class="image"' in email_content.body
+
+
+def test_error_email_without_screenshot() -> None:
+    # `superset.models.helpers`, a dependency of following imports,
+    # requires app context
+    from superset.reports.models import ReportRecipients, ReportRecipientType
+    from superset.reports.notifications.base import NotificationContent
+    from superset.reports.notifications.email import EmailNotification
+
+    content = NotificationContent(
+        name="test alert",
+        text="Error occurred while generating report",
+        url="http://localhost:8088/superset/dashboard/1",
+        header_data={
+            "notification_format": "PNG",
+            "notification_type": "Alert",
+            "owners": [1],
+            "notification_source": None,
+            "chart_id": None,
+            "dashboard_id": None,
+            "slack_channels": None,
+        },
+    )
+    email_content = EmailNotification(
+        recipient=ReportRecipients(type=ReportRecipientType.EMAIL), content=content
+    )._get_content()
+
+    # Check that error message is in the body
+    assert "Error occurred while generating report" in email_content.body
+    assert "unable to be generated" in email_content.body
+
+    # Check that no images are included
+    assert email_content.images == {}
+
+    # Check that no image tags are in the body
+    assert "<img" not in email_content.body

tests/unit_tests/utils/webdriver_test.py

@@ -18,6 +18,7 @@
 from unittest.mock import MagicMock, patch, PropertyMock
 
 import pytest
+from selenium.common.exceptions import TimeoutException
 
 from superset.utils.webdriver import (
     check_playwright_availability,
@@ -273,6 +274,109 @@ class TestWebDriverSelenium:
         # Should create driver without errors
         mock_driver_class.assert_called_once()
 
+    @patch("superset.utils.webdriver.time")
+    @patch("superset.utils.webdriver.app")
+    @patch("superset.utils.webdriver.chrome")
+    @patch("superset.utils.webdriver.logger")
+    @patch("superset.utils.webdriver.WebDriverWait")
+    def test_get_screenshot_logs_chart_timeout_details(
+        self, mock_wait, mock_logger, mock_chrome, mock_app_patch, mock_time, mock_app
+    ):
+        """Test that chart timeout logs detailed diagnostic information."""
+        # Mock time to return consistent values for elapsed time calculation
+        mock_time.return_value = 100.0  # Start time
+        mock_app_patch.config = {
+            "WEBDRIVER_TYPE": "chrome",
+            "WEBDRIVER_OPTION_ARGS": [],
+            "SCREENSHOT_LOCATE_WAIT": 10,
+            "SCREENSHOT_LOAD_WAIT": 10,
+            "WEBDRIVER_WINDOW": {"dashboard": (1600, 1200)},
+            "WEBDRIVER_CONFIGURATION": {},
+            "SCREENSHOT_SELENIUM_HEADSTART": 0,
+            "SCREENSHOT_SELENIUM_ANIMATION_WAIT": 0,
+            "SCREENSHOT_REPLACE_UNEXPECTED_ERRORS": False,
+            "SCREENSHOT_SELENIUM_RETRIES": 2,
+        }
+
+        # Setup mocks
+        mock_driver = MagicMock()
+        mock_driver_class = MagicMock(return_value=mock_driver)
+        mock_chrome.webdriver.WebDriver = mock_driver_class
+        mock_chrome.service.Service = MagicMock()
+        mock_options = MagicMock()
+        mock_chrome.options.Options = MagicMock(return_value=mock_options)
+
+        # Mock chart elements with identifiers
+        mock_chart1 = MagicMock()
+        mock_chart1.get_attribute.side_effect = (
+            lambda attr: "chart-123" if attr == "data-test-chart-id" else None
+        )
+        mock_chart2 = MagicMock()
+        mock_chart2.get_attribute.side_effect = (
+            lambda attr: "chart-456" if attr == "data-chart-id" else None
+        )
+
+        # Mock element that will be found after timeout
+        mock_element = MagicMock()
+        mock_element.screenshot_as_png = b"screenshot_data"
+        mock_driver.find_element.return_value = mock_element
+
+        mock_driver.find_elements.side_effect = lambda by, value: {
+            "chart-container": [mock_chart1, mock_chart2],
+            "grid-container": [MagicMock()],
+            "loading": [MagicMock()],
+        }.get(value, [])
+
+        mock_driver.page_source = "<html><body>Test DOM content</body></html>"
+        mock_driver.get_screenshot_as_png.return_value = b"screenshot_data"
+
+        # Setup WebDriverWait to raise TimeoutException
+        mock_wait_instance = MagicMock()
+        mock_wait_instance.until.side_effect = TimeoutException()
+        mock_wait.return_value = mock_wait_instance
+
+        # Mock user and auth
+        mock_user = MagicMock()
+        driver = WebDriverSelenium(driver_type="chrome")
+
+        with patch.object(driver, "auth", return_value=mock_driver):
+            # Should raise ReportScheduleScreenshotTimeout with screenshot
+            from superset.commands.report.exceptions import (
+                ReportScheduleScreenshotTimeout,
+            )
+
+            with pytest.raises(ReportScheduleScreenshotTimeout) as exc_info:
+                driver.get_screenshot(
+                    "http://example.com/dashboard/1", "dashboard", mock_user
+                )
+
+            # Verify screenshot was captured despite timeout
+            assert exc_info.value.screenshots == [b"screenshot_data"]
+            # Verify elapsed_seconds was captured
+            assert exc_info.value.elapsed_seconds is not None
+
+        # Verify diagnostic logging
+        # Check that we logged chart timeout with details
+        chart_timeout_logged = any(
+            "Timeout waiting for chart containers" in str(call[0])
+            for call in mock_logger.warning.call_args_list
+        )
+        assert chart_timeout_logged, "Should log chart timeout details"
+
+        # Check that chart identifiers were logged (they're passed as arguments)
+        chart_ids_logged = any(
+            "chart-123" in str(call) or "chart-456" in str(call)
+            for call in mock_logger.warning.call_args_list
+        )
+        assert chart_ids_logged, "Should log chart identifiers"
+
+        # Check that DOM preview was logged
+        dom_preview_logged = any(
+            "Dashboard DOM preview" in str(call[0])
+            for call in mock_logger.warning.call_args_list
+        )
+        assert dom_preview_logged, "Should log DOM preview"
+
 
 class TestPlaywrightAvailabilityCheck:
     """Test comprehensive Playwright availability checking."""