Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion superset/commands/report/execute.py
Original file line number Diff line number Diff line change
Expand Up @@ -664,8 +664,35 @@ def send_error(self, name: str, message: str) -> None:
header_data,
self._execution_id,
)

# Try to capture screenshot even on failure - best effort
screenshot_data = []
try:
screenshot_data = self._get_screenshots()
logger.info(
"Successfully captured screenshot for error notification: %s",
self._execution_id,
)
except (
ReportScheduleScreenshotTimeout,
ReportScheduleScreenshotFailedError,
) as ex:
logger.warning(
"Could not capture screenshot for error notification: %s", str(ex)
)
except Exception as ex: # pylint: disable=broad-except
logger.warning(
"Unexpected error while capturing screenshot for error "
"notification: %s",
str(ex),
)

notification_content = NotificationContent(
name=name, text=message, header_data=header_data, url=url
name=name,
text=message,
header_data=header_data,
url=url,
screenshots=screenshot_data,
)

# filter recipients to recipients who are also owners
Expand Down
25 changes: 22 additions & 3 deletions superset/reports/notifications/email.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,22 +97,41 @@ 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
}
for msgid in images.keys():
img_tag_str += (
f'<div class="image"><img width="1000" src="cid:{msgid}"></div>'
)
Comment on lines 162 to 165

This comment was marked as resolved.


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

Expand Down Expand Up @@ -147,7 +166,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">
Expand Down
78 changes: 78 additions & 0 deletions tests/unit_tests/reports/notifications/email_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading