You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2022/12/13 21:31:59 UTC
[superset] branch master updated: fix(report): Capture unexpected errors in report screenshots. Fixes #21653 (#21724)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new d1989a4766 fix(report): Capture unexpected errors in report screenshots. Fixes #21653 (#21724)
d1989a4766 is described below
commit d1989a4766ca624763fcefe50fa5a4c582e950f6
Author: Rui Zhao <10...@users.noreply.github.com>
AuthorDate: Tue Dec 13 13:31:36 2022 -0800
fix(report): Capture unexpected errors in report screenshots. Fixes #21653 (#21724)
Co-authored-by: Rui Zhao <zh...@dropbox.com>
---
superset/config.py | 6 +++
superset/utils/webdriver.py | 70 ++++++++++++++++++++++++++++-
tests/integration_tests/thumbnails_tests.py | 70 ++++++++++++++++++++++++++++-
3 files changed, 143 insertions(+), 3 deletions(-)
diff --git a/superset/config.py b/superset/config.py
index f163997c6e..a597678e59 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -595,6 +595,12 @@ SCREENSHOT_SELENIUM_RETRIES = 5
SCREENSHOT_SELENIUM_HEADSTART = 3
# Wait for the chart animation, in seconds
SCREENSHOT_SELENIUM_ANIMATION_WAIT = 5
+# Replace unexpected errors in screenshots with real error messages
+SCREENSHOT_REPLACE_UNEXPECTED_ERRORS = False
+# Max time to wait for error message modal to show up, in seconds
+SCREENSHOT_WAIT_FOR_ERROR_MODAL_VISIBLE = 5
+# Max time to wait for error message modal to close, in seconds
+SCREENSHOT_WAIT_FOR_ERROR_MODAL_INVISIBLE = 5
# ---------------------------------------------------
# Image and file configuration
diff --git a/superset/utils/webdriver.py b/superset/utils/webdriver.py
index 93e8957f21..991790339a 100644
--- a/superset/utils/webdriver.py
+++ b/superset/utils/webdriver.py
@@ -20,7 +20,7 @@ from __future__ import annotations
import logging
from enum import Enum
from time import sleep
-from typing import Any, Dict, Optional, Tuple, TYPE_CHECKING
+from typing import Any, Dict, List, Optional, Tuple, TYPE_CHECKING
from flask import current_app
from selenium.common.exceptions import (
@@ -51,6 +51,62 @@ class DashboardStandaloneMode(Enum):
REPORT = 3
+def find_unexpected_errors(driver: WebDriver) -> List[str]:
+ error_messages = []
+
+ try:
+ alert_divs = driver.find_elements(By.XPATH, "//div[@role = 'alert']")
+ logger.debug(
+ "%i alert elements have been found in the screenshot", len(alert_divs)
+ )
+
+ for alert_div in alert_divs:
+ # See More button
+ alert_div.find_element(By.XPATH, ".//*[@role = 'button']").click()
+
+ # wait for modal to show up
+ modal = WebDriverWait(
+ driver, current_app.config["SCREENSHOT_WAIT_FOR_ERROR_MODAL_VISIBLE"]
+ ).until(
+ EC.visibility_of_any_elements_located(
+ (By.CLASS_NAME, "ant-modal-content")
+ )
+ )[
+ 0
+ ]
+
+ err_msg_div = modal.find_element(By.CLASS_NAME, "ant-modal-body")
+
+ # collect error message
+ error_messages.append(err_msg_div.text)
+
+ # close modal after collecting error messages
+ modal.find_element(By.CLASS_NAME, "ant-modal-close").click()
+
+ # wait until the modal becomes invisible
+ WebDriverWait(
+ driver, current_app.config["SCREENSHOT_WAIT_FOR_ERROR_MODAL_INVISIBLE"]
+ ).until(EC.invisibility_of_element(modal))
+
+ # Use HTML so that error messages are shown in the same style (color)
+ error_as_html = err_msg_div.get_attribute("innerHTML").replace("'", "\\'")
+
+ try:
+ # Even if some errors can't be updated in the screenshot,
+ # keep all the errors in the server log and do not fail the loop
+ driver.execute_script(
+ f"arguments[0].innerHTML = '{error_as_html}'", alert_div
+ )
+ except WebDriverException:
+ logger.warning(
+ "Failed to update error messages using alert_div", exc_info=True
+ )
+ except WebDriverException:
+ logger.warning("Failed to capture unexpected errors", exc_info=True)
+
+ return error_messages
+
+
class WebDriverProxy:
def __init__(self, driver_type: str, window: Optional[WindowSize] = None):
self._driver_type = driver_type
@@ -141,7 +197,19 @@ class WebDriverProxy:
url,
user.username,
)
+
+ if current_app.config["SCREENSHOT_REPLACE_UNEXPECTED_ERRORS"]:
+ unexpected_errors = find_unexpected_errors(driver)
+ if unexpected_errors:
+ logger.warning(
+ "%i errors found in the screenshot. URL: %s. Errors are: %s",
+ len(unexpected_errors),
+ url,
+ unexpected_errors,
+ )
+
img = element.screenshot_as_png
+
except TimeoutException:
logger.warning("Selenium timed out requesting url %s", url, exc_info=True)
except StaleElementReferenceException:
diff --git a/tests/integration_tests/thumbnails_tests.py b/tests/integration_tests/thumbnails_tests.py
index 5eabc4da00..81557c7d89 100644
--- a/tests/integration_tests/thumbnails_tests.py
+++ b/tests/integration_tests/thumbnails_tests.py
@@ -19,8 +19,9 @@
import urllib.request
from io import BytesIO
from unittest import skipUnless
-from unittest.mock import ANY, call, patch
+from unittest.mock import ANY, call, MagicMock, patch
+import pytest
from flask_testing import LiveServerTestCase
from sqlalchemy.sql import func
@@ -30,7 +31,7 @@ from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.utils.screenshots import ChartScreenshot, DashboardScreenshot
from superset.utils.urls import get_url_host, get_url_path
-from superset.utils.webdriver import WebDriverProxy
+from superset.utils.webdriver import find_unexpected_errors, WebDriverProxy
from tests.integration_tests.conftest import with_feature_flags
from tests.integration_tests.test_app import app
@@ -62,6 +63,71 @@ class TestThumbnailsSeleniumLive(LiveServerTestCase):
self.assertEqual(response.getcode(), 202)
+class TestWebDriverScreenshotErrorDetector(SupersetTestCase):
+ @patch("superset.utils.webdriver.WebDriverWait")
+ @patch("superset.utils.webdriver.firefox")
+ @patch("superset.utils.webdriver.find_unexpected_errors")
+ def test_not_call_find_unexpected_errors_if_feature_disabled(
+ self, mock_find_unexpected_errors, mock_firefox, mock_webdriver_wait
+ ):
+ webdriver_proxy = WebDriverProxy("firefox")
+ user = security_manager.get_user_by_username(
+ app.config["THUMBNAIL_SELENIUM_USER"]
+ )
+ url = get_url_path("Superset.dashboard", dashboard_id_or_slug=1)
+ webdriver_proxy.get_screenshot(url, "grid-container", user=user)
+
+ assert not mock_find_unexpected_errors.called
+
+ @patch("superset.utils.webdriver.WebDriverWait")
+ @patch("superset.utils.webdriver.firefox")
+ @patch("superset.utils.webdriver.find_unexpected_errors")
+ def test_call_find_unexpected_errors_if_feature_enabled(
+ self, mock_find_unexpected_errors, mock_firefox, mock_webdriver_wait
+ ):
+ app.config["SCREENSHOT_REPLACE_UNEXPECTED_ERRORS"] = True
+ webdriver_proxy = WebDriverProxy("firefox")
+ user = security_manager.get_user_by_username(
+ app.config["THUMBNAIL_SELENIUM_USER"]
+ )
+ url = get_url_path("Superset.dashboard", dashboard_id_or_slug=1)
+ webdriver_proxy.get_screenshot(url, "grid-container", user=user)
+
+ assert mock_find_unexpected_errors.called
+
+ app.config["SCREENSHOT_REPLACE_UNEXPECTED_ERRORS"] = False
+
+ def test_find_unexpected_errors_no_alert(self):
+ webdriver = MagicMock()
+
+ webdriver.find_elements.return_value = []
+
+ unexpected_errors = find_unexpected_errors(driver=webdriver)
+ assert len(unexpected_errors) == 0
+
+ assert "alert" in webdriver.find_elements.call_args_list[0][0][1]
+
+ @patch("superset.utils.webdriver.WebDriverWait")
+ def test_find_unexpected_errors(self, mock_webdriver_wait):
+ webdriver = MagicMock()
+ alert_div = MagicMock()
+
+ webdriver.find_elements.return_value = [alert_div]
+ alert_div.find_elements.return_value = MagicMock()
+
+ unexpected_errors = find_unexpected_errors(driver=webdriver)
+ assert len(unexpected_errors) == 1
+
+ # attempt to find alerts
+ assert "alert" in webdriver.find_elements.call_args_list[0][0][1]
+ # attempt to click on "See more" buttons
+ assert "button" in alert_div.find_element.call_args_list[0][0][1]
+ # Wait for error modal to show up and to hide
+ assert 2 == len(mock_webdriver_wait.call_args_list)
+ # replace the text in alert div, eg, "unexpected errors"
+ assert alert_div == webdriver.execute_script.call_args_list[0][0][1]
+
+
class TestWebDriverProxy(SupersetTestCase):
@patch("superset.utils.webdriver.WebDriverWait")
@patch("superset.utils.webdriver.firefox")