codeant-ai-for-open-source[bot] commented on code in PR #42273:
URL: https://github.com/apache/superset/pull/42273#discussion_r3692428320
##########
superset/utils/webdriver.py:
##########
@@ -563,18 +563,22 @@ def get_screenshot( # pylint: disable=too-many-locals,
too-many-statements # n
log_context=log_context,
)
if not img:
+ # _get_screenshot() has no wait/readiness logic at
+ # all, so falling back to it here would risk
+ # silently delivering a screenshot of spinners or
+ # a blank dashboard. Fail the report loudly
+ # instead of guessing at a "safer" fallback.
logger.warning(
- (
- "Tiled screenshot failed, "
- "falling back to standard screenshot"
- )
+ "Tiled screenshot failed for url %s and no "
+ "safe fallback exists; failing the report",
+ url,
)
- img = WebDriverPlaywright._get_screenshot(
- page, element, element_name
+ raise PlaywrightTimeout(
+ f"Tiled screenshot failed for url {url}"
)
Review Comment:
**Suggestion:** The tiled helper returns `None` for non-timeout failures
such as element lookup, JavaScript evaluation, screenshot capture, or
tile-combination errors, but this branch converts every such failure into
`PlaywrightTimeout`. That misclassifies system failures as readiness timeouts
and discards the exception from the caller's control flow. Preserve the
original exception (or introduce a dedicated tiled-capture failure exception
and handle it explicitly) instead of synthesizing a timeout for all falsy
results. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ System capture errors receive timeout status 408.
- ⚠️ Original exception details are not propagated.
- ⚠️ Report failure classification becomes operationally misleading.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=aeb9caf8ba3242309a351feb8b8aa868&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=aeb9caf8ba3242309a351feb8b8aa868&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/webdriver.py
**Line:** 565:578
**Comment:**
*Api Mismatch: The tiled helper returns `None` for non-timeout failures
such as element lookup, JavaScript evaluation, screenshot capture, or
tile-combination errors, but this branch converts every such failure into
`PlaywrightTimeout`. That misclassifies system failures as readiness timeouts
and discards the exception from the caller's control flow. Preserve the
original exception (or introduce a dedicated tiled-capture failure exception
and handle it explicitly) instead of synthesizing a timeout for all falsy
results.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42273&comment_hash=1d4871a286ebf6a6cc771d35a58b36b632cd2ea4b89c3e23c59c9827efc99a84&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42273&comment_hash=1d4871a286ebf6a6cc771d35a58b36b632cd2ea4b89c3e23c59c9827efc99a84&reaction=dislike'>👎</a>
--
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]
