eschutho opened a new pull request, #46064:
URL: https://github.com/apache/superset/pull/46064
### SUMMARY
A dashboard report scoped to a tab that contains only text/markdown (no
charts) failed after the wait limit with a raw Playwright error
(`Page.wait_for_function: Timeout ...`), and an "All Tabs" report on a
dashboard with such a tab could stay in *Working* for close to an hour and hold
a worker.
**Root cause**
- The report readiness checks in `screenshot_utils.py` / `webdriver.py`
(`REPORT_CHART_HOLDERS_READY_JS`, `REPORT_ALL_CHART_HOLDERS_READY_JS`, their
stable wrappers and `CHART_HOLDERS_MOUNTED_JS`, at six call sites across the
standard and tiled capture paths) require `holders.length > 0`. Markdown does
not render a chart holder, so a selected tab without charts can never become
ready.
- Report readiness waits are not bounded by `SCREENSHOT_LOAD_WAIT`; they
receive the whole remaining execution budget (about 56 minutes at the default
3600s).
- A capture timeout was wrapped as `Failed taking a screenshot <raw
Playwright text>` and emailed as is.
**Changes**
- New
`superset/utils/dashboard_layout.py::selected_tab_chart_count(position,
anchor)` returns how many charts the selected tab's pane will show (path-aware
for nested tabs: charts outside nested tabs count, off-path tab groups count
their first tab) and the id suffix of its tab pane. It returns `None`, never
`0`, for any layout it cannot trust (missing/malformed layout, stale `parents`,
duplicate or shared nodes, cycles, unknown node types, unknown anchor).
- `ReportExecutionContext` gets `empty_tab_pane_suffix` and
`with_empty_tab_pane()` (copy-based, so the shared capture-rejection and
validated-artifact state is preserved across tabs).
- `report_ready_js` / `report_mounted_js` accept zero chart holders **only**
when the selected tab is proven chart-free, its tab pane (and every ancestor
pane) is active, and it contains the dashboard content wrapper. The existing JS
constants are unchanged and still used when no suffix is set. All six call
sites use the factories.
- For a proven-empty tab the readiness waits are capped at
`SCREENSHOT_LOAD_WAIT`, and the whole-dashboard slice count no longer forces
tiled capture.
- **Behaviour change for all report captures:** the "no chart holder has
ever mounted" stage is now capped at `SCREENSHOT_LOAD_WAIT` on both the
standard and tiled paths (`ChartHoldersNeverMountedError`, a
`PlaywrightTimeout` subclass). A dashboard on which no chart holder mounts
therefore fails after about 60s instead of holding a worker for up to an hour.
Charts that did mount keep the full remaining budget. Thumbnails,
chart-container and `require_complete_capture` paths are untouched.
- `_get_screenshots` maps anchors to URLs (only for explicit anchors;
`activeTabs` or a mismatch means no change) and, when the empty-tab path or the
mount cap fires, raises a readable error naming the tab instead of the raw
Playwright text. Other capture timeouts keep the existing message.
- Not in scope: the no-tab-selected default tab and fully empty dashboards
(only the mount cap applies).
### TESTING INSTRUCTIONS
```
pytest tests/unit_tests/utils/test_dashboard_layout.py \
tests/unit_tests/utils/test_report_execution.py \
tests/unit_tests/utils/test_screenshot_utils.py \
tests/unit_tests/utils/webdriver_test.py \
tests/unit_tests/commands/report/execute_test.py
```
Covered: layout counting incl. nested tabs and the `None` cases; readiness
predicates in a Node fake DOM (active/inactive pane, unready holders, nested
ancestors, unchanged defaults); standard and tiled capture of a chart-free tab;
the mount cap only for report captures and not for slow-but-mounted charts;
shared capture state across tabs; anchor mapping, `activeTabs` /
length-mismatch guards and the readable error. Two existing `webdriver_test.py`
tests were adjusted for the new mount pre-wait (new log reason
`zero_holders_timeout`; the below-fold test lets the mount predicate pass so
its failure still comes from the readiness wait).
Manual: report on a tab that only contains a Markdown component, and an "All
Tabs" report on a dashboard with such a tab; both should complete.
Limitations: the tab-pane id format (`<TabsId>-panel-<key>`) was checked on
antd 6.3.5 / rc-tabs 15.7, not on the pinned antd, and nothing was run in a
real browser. If the format differs, the pane lookup fails and the capture ends
after `SCREENSHOT_LOAD_WAIT` with the readable error rather than hanging. One
pre-existing unrelated failure exists on master
(`test_date_parsing.py::test_epoch_format_invalid_values`).
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags: `ALERT_REPORT_TABS`
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
