fix(reports): catch TypeError for non-string extra.dashboard.anchor in get_dashboard_urls

[eschutho]

SUMMARY

BaseReportState.get_dashboard_urls (in superset/commands/report/execute.py) parses extra.dashboard.anchor with json.loads under a bare except json.JSONDecodeError. Because extra is an untyped marshmallow Dict field (superset/reports/schemas.py), anchor can be any JSON value — not just a string.

Problem: When anchor is a non-string JSON value (int, list, dict, bool), json.loads(anchor) raises a raw TypeError, which is not a subclass of json.JSONDecodeError (JSONDecodeError is a ValueError; TypeError is unrelated). The existing except json.JSONDecodeError does not catch it, so the TypeError propagates uncaught. get_dashboard_urls() runs on the live report/alert execution path — a background Celery job — reached both from BaseReportState._get_screenshots (dashboard screenshot path) and directly from AsyncExecuteReportScheduleCommand.run (permalink pre-commit step). So any report whose stored extra.dashboard.anchor is a non-string value crashes that job's screenshot/permalink step.

Fix: Widen the guard to except (TypeError, json.JSONDecodeError), mirroring the sibling query_context parsing already in this same file (AsyncExecuteReportScheduleCommand, except (TypeError, json.JSONDecodeError) as ex:). This is a pure additive except-tuple widening — no new exception class, no behavior change for any input that already worked (string anchors, valid/invalid JSON-list anchors). Only the previously-crashing non-string-anchor case now falls through to the existing graceful "fall back to single tab" path, exactly like the sibling site's established behavior.

This is sibling-site drift: the same file has the correct broader guard one method over. The same bug class at validation time was addressed in #44404 (base.py::_validate_report_extra), whose body explicitly flagged this execute.py anchor-handling site as a known follow-up gap, out of scope for that PR. This PR closes that follow-up. See also #42401 for the original sibling-site-drift precedent pattern.

TESTING INSTRUCTIONS

Added a regression case to the existing parametrized test_get_dashboard_urls_with_multiple_tabs in tests/unit_tests/commands/report/execute_test.py with a non-string anchor (42), asserting it does not raise TypeError and instead falls back gracefully to a single-tab permalink.

• Pre-fix: the new case fails with TypeError: Input string must be text, not bytes (raw TypeError propagates).
• Post-fix: all 177 tests in the file pass.

pytest tests/unit_tests/commands/report/execute_test.py

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

🤖 Generated with Claude Code

[bito-code-review]

Code Review Agent Run #72fe52

Actionable Suggestions - 0

Additional Suggestions - 1

• superset/commands/report/execute.py - 1

  • Over-broad except masks errors · Line 621-621
    Broadening the except to `TypeError` also wraps the `self._get_tabs_urls(...)` call (which invokes `_get_tab_url` → `CreateDashboardPermalinkCommand.run`). A genuine `TypeError` from that permalink code is now silently swallowed and falls back to a single-tab permalink, masking the real failure. Consider scoping the try/except to just `json.loads(anchor)`.

Review Details

• Files reviewed - 2 · Commit Range: cafb981..cafb981

  • superset/commands/report/execute.py
  • tests/unit_tests/commands/report/execute_test.py
• Files skipped - 0
• Tools

  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful
  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers an incremental AI Review.

• /review full - Manually triggers a full AI Review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.65%. Comparing base (43fee87) to head (cafb981).
⚠️ Report is 145 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #44450      +/-   ##
==========================================
- Coverage   80.65%   80.65%   -0.01%     
==========================================
  Files        2942     2942              
  Lines      175692   175692              
  Branches    40788    40788              
==========================================
- Hits       141705   141703       -2     
- Misses      31325    31327       +2     
  Partials     2662     2662              

Flag	Coverage Δ
hive	37.15% <0.00%> (ø)
mysql	56.38% <100.00%> (ø)
postgres	56.39% <100.00%> (-0.01%)	⬇️
presto	39.04% <0.00%> (ø)
python	84.96% <100.00%> (-0.01%)	⬇️
sqlite	56.10% <100.00%> (ø)
unit	76.77% <100.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[codeant-ai-for-open-source]

Suggestion: The widened handler also catches TypeError from _get_tabs_urls, so permalink-generation failures are misclassified as invalid anchors and trigger the single-tab fallback.

Assessment: 🟠 Major · 🔁 Occurrence: Sometimes · 🏷️ Error handling

Prompt for AI Agent 🤖

This is a comment left during a code review.

**Path:** superset/commands/report/execute.py
**Line:** 621:621
**Comment:**
	*Error Handling: The widened handler also catches `TypeError` from `_get_tabs_urls`, so permalink-generation failures are misclassified as invalid anchors and trigger the single-tab fallback.

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

👍 | 👎

[bito-code-review]

The flagged issue is correct. Widening the exception handler to catch TypeError alongside json.JSONDecodeError causes the code to treat any TypeError (including those from _get_tabs_urls) as an invalid anchor, triggering the single-tab fallback incorrectly. To resolve this, you should catch TypeError only when it is specifically expected from the JSON decoding process, or ensure _get_tabs_urls does not raise TypeError in cases that should be treated as errors rather than fallback triggers.

Would you like me to fetch all other comments on this PR to validate and implement fixes for them as well?

superset/commands/report/execute.py

except (TypeError, json.JSONDecodeError):
                    logger.debug("Anchor value is not a list, Fall back to single tab")