feat(alerts): Include screenshots in alert/report failure emails

superset/commands/report/exceptions.py

@@ -257,6 +257,10 @@ class ReportScheduleScreenshotTimeout(CommandException):
     status = 408
     message = _("A timeout occurred while taking a screenshot.")
 
+    def __init__(self, screenshots: list[bytes] | None = None) -> None:
+        super().__init__()
+        self.screenshots = screenshots or []
+
 
 class ReportScheduleCsvTimeout(CommandException):
     status = 408

superset/commands/report/execute.py

@@ -381,6 +381,14 @@ def _get_screenshots(self) -> list[bytes]:
             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,9 @@ def send(self) -> None:
         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
+    ) -> None:
         """
         Creates and sends a notification for an error, to all recipients
 
@@ -664,8 +674,13 @@ def send_error(self, name: str, message: str) -> None:
             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 [],
         )
 
         # filter recipients to recipients who are also owners
@@ -765,10 +780,16 @@ def next(self) -> None:
             if not self.is_in_error_grace_period():
                 second_error_message = REPORT_SCHEDULE_ERROR_NOTIFICATION_MARKER
                 try:
+                    # Extract screenshots from timeout exception if available
+                    screenshots = None
+                    if isinstance(first_ex, ReportScheduleScreenshotTimeout):
+                        screenshots = first_ex.screenshots
+
                     self.send_error(
                         f"Error occurred for {self._report_schedule.type}:"
                         f" {self._report_schedule.name}",
                         str(first_ex),
+                        screenshots=screenshots,
                     )
 
                 except SupersetErrorsException as second_ex:
@@ -835,10 +856,16 @@ def next(self) -> None:
                     self.update_report_schedule_and_log(ReportState.NOOP)
                     return
             except Exception as ex:
+                # Extract screenshots from timeout exception if available
+                screenshots = None
+                if isinstance(ex, ReportScheduleScreenshotTimeout):
+                    screenshots = ex.screenshots
+
                 self.send_error(
                     f"Error occurred for {self._report_schedule.type}:"
                     f" {self._report_schedule.name}",
                     str(ex),
+                    screenshots=screenshots,
                 )
                 self.update_report_schedule_and_log(
                     ReportState.ERROR,

superset/reports/notifications/email.py

@@ -97,22 +97,43 @@ def _name(self) -> str:
     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()
         return __(
             """
             <p>Your report/alert was unable to be generated because of the following error: %(text)s</p>
             <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,
             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 +168,7 @@ def _get_content(self) -> EmailContent:
         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

@@ -573,10 +573,14 @@ def find_unexpected_errors(driver: WebDriver) -> list[str]:
         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
+
         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 +595,22 @@ def get_screenshot(self, url: str, element_name: str, user: User) -> bytes | Non
                     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
+                logger.warning(
+                    "Selenium timed out locating element %s at url %s - will "
+                    "attempt to capture current page state",
+                    element_name,
+                    url,
+                )
+                # 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,20 +621,53 @@ def get_screenshot(self, url: str, element_name: str, user: User) -> bytes | Non
                     )
                 )
             except TimeoutException:
-                logger.info("Timeout Exception caught")
+                had_timeout = True
+
+                # 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; "
+                    "%d chart containers found, %d grid containers present, "
+                    "%d loading elements still visible; sample chart identifiers: %s",
+                    url,
+                    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(
                         EC.visibility_of_all_elements_located(
                             (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 +678,12 @@ def get_screenshot(self, url: str, element_name: str, user: User) -> bytes | Non
                     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
+                logger.warning(
+                    "Selenium timed out waiting for charts to load at url %s - "
+                    "will capture current state with loading indicators",
+                    url,
                 )
-                raise
 
             selenium_animation_wait = app.config["SCREENSHOT_SELENIUM_ANIMATION_WAIT"]
             logger.debug("Wait %i seconds for chart animation", selenium_animation_wait)
@@ -651,24 +704,36 @@ def get_screenshot(self, url: str, element_name: str, user: User) -> bytes | Non
                         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:
+                raise ReportScheduleScreenshotTimeout(screenshots=[img])
+
         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