fix(reports): pre-commit tab permalinks before state machine transaction

When a dashboard report targets specific tabs, BaseReportState calls CreateDashboardPermalinkCommand inside the outer @transaction() on AsyncExecuteReportScheduleCommand.run(). The @transaction() nesting guard (g.in_transaction) causes the command to only flush the permalink INSERT, leaving the row invisible to Playwright's separate DB connection. Flask returns a 404 from dashboard_permalink, <body class="standalone"> never appears, and the screenshot times out after 600 s. Fix: remove @transaction() from AsyncExecuteReportScheduleCommand.run() and add a pre-warm call to get_dashboard_urls() before the state machine starts. Executed outside any transaction, the call lets CreateDashboardPermalinkCommand commit normally via its own @transaction(). The state machine's subsequent call to get_dashboard_urls() finds the already-committed row through get_entry() (same deterministic UUID) and returns it without a second INSERT. ReportScheduleStateMachine.run() retains its own @transaction() and remains the outermost transaction owner. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-16 13:09:20 +00:00 · 2026-06-16 00:15:23 +00:00
3 changed files with 23 additions and 79 deletions
--- a/superset/commands/report/execute.py
+++ b/superset/commands/report/execute.py
@@ -1157,7 +1157,6 @@ class AsyncExecuteReportScheduleCommand(BaseCommand):
        self._scheduled_dttm = scheduled_dttm
        self._execution_id = UUID(task_id)

-    @transaction()
    def run(self) -> None:
        try:
            self.validate()
@@ -1170,6 +1169,21 @@ class AsyncExecuteReportScheduleCommand(BaseCommand):
            )
            user = security_manager.find_user(username)

+            with override_user(user):
+                # Pre-commit any permalink rows before the state machine's
+                # @transaction() opens. When called inside a transaction,
+                # CreateDashboardPermalinkCommand only flushes (not commits),
+                # leaving the row invisible to Playwright's separate DB
+                # connection. Running get_dashboard_urls() here — outside any
+                # transaction — lets the command commit normally. The state
+                # machine's inner call to get_dashboard_urls() hits get_entry()
+                # for the same deterministic UUID and returns the
+                # already-committed row without a second INSERT.
+                if self._model.dashboard_id:
+                    BaseReportState(
+                        self._model, self._scheduled_dttm, self._execution_id
+                    ).get_dashboard_urls()
+
            start_time = datetime.utcnow()
            with override_user(user):
                ReportScheduleStateMachine(
--- a/superset/utils/webdriver.py
+++ b/superset/utils/webdriver.py
@@ -375,12 +375,15 @@ class WebDriverPlaywright(WebDriverProxy):
                            animation_wait=selenium_animation_wait,
                        )
                        if img is None:
-                            logger.error(
-                                "Tiled screenshot failed at url %s; "
-                                "not falling back to avoid sending a blank PDF",
-                                url,
+                            logger.warning(
+                                (
+                                    "Tiled screenshot failed, "
+                                    "falling back to standard screenshot"
+                                )
+                            )
+                            img = WebDriverPlaywright._get_screenshot(
+                                page, element, element_name
                            )
-                            return None
                    else:
                        # Standard screenshot captures the full element including
                        # below-the-fold content, so wait for all spinners globally.
--- a/tests/unit_tests/utils/webdriver_test.py
+++ b/tests/unit_tests/utils/webdriver_test.py
@@ -898,76 +898,3 @@ 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",
-        )