Skip to content

Comments

fix(screenshots): downgrade screenshot timeout logs from ERROR to WARNING#38130

Open
aminghadersohi wants to merge 1 commit intoapache:masterfrom
preset-io:fix/reduce-screenshot-error-logging
Open

fix(screenshots): downgrade screenshot timeout logs from ERROR to WARNING#38130
aminghadersohi wants to merge 1 commit intoapache:masterfrom
preset-io:fix/reduce-screenshot-error-logging

Conversation

@aminghadersohi
Copy link
Contributor

SUMMARY

Selenium timeout exceptions during screenshot generation are expected operational behavior (slow-loading dashboards, network latency, etc.) and not actual server errors. These were being logged at ERROR level via logger.exception(), creating significant noise in log aggregators like Datadog.

Changes:

  • webdriver.py: Change logger.exception() to logger.warning(..., exc_info=True) for all three timeout scenarios (page load, dashboard draw, chart load)
  • webdriver.py: Fix bare except: clause to use except Exception:
  • webdriver.py: Reorder outer except blocks so specific exceptions (TimeoutException, StaleElementReferenceException, WebDriverException) are caught before the generic Exception handler — previously these were unreachable dead code
  • dashboards/api.py: Downgrade logger.error() to logger.warning() in the thumbnail endpoint's catch-all handler that returns 404 (inconsistent to log ERROR for a 404 response)

BEFORE/AFTER SCREENSHOTS OR COVERAGE URL

N/A - logging level changes only, no UI impact.

TESTING INSTRUCTIONS

  1. Trigger a dashboard thumbnail generation for a dashboard that takes a long time to load
  2. Check that selenium timeout messages are logged at WARNING level instead of ERROR
  3. Verify that the ScreenshotImageNotAvailableException still returns 404 properly
  4. Unit tests: pytest tests/unit_tests/utils/webdriver_test.py (16/17 pass - the 1 failure is pre-existing in Playwright test, unrelated to these changes)

ADDITIONAL INFORMATION

  • These changes directly address Datadog [remove] exclusion filters for selenium timeout errors
  • The outer except block reordering fixes a bug where except Exception caught everything before the more specific handlers, making them dead code
  • logger.warning(..., exc_info=True) preserves the full stack trace for debugging while correctly classifying the severity

🤖 Generated with Claude Code

…NING

Selenium timeout exceptions during screenshot generation are expected
operational behavior (slow-loading dashboards, network latency, etc.)
and not actual server errors. These were being logged at ERROR level
via logger.exception(), creating noise in log aggregators like Datadog.

Changes:
- webdriver.py: Change logger.exception() to logger.warning() for all
  three timeout scenarios (page load, dashboard draw, chart load)
- webdriver.py: Fix bare except clause to use except Exception
- webdriver.py: Reorder outer except blocks so specific exceptions
  (TimeoutException, StaleElementReferenceException, WebDriverException)
  are caught before the generic Exception handler (previously unreachable)
- dashboards/api.py: Downgrade logger.error() to logger.warning() in
  the thumbnail endpoint's catch-all handler that returns 404

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@bito-code-review
Copy link
Contributor

bito-code-review bot commented Feb 20, 2026

Code Review Agent Run #3711ce

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 8f51f1b..8f51f1b
    • superset/dashboards/api.py
    • superset/utils/webdriver.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

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

  • /review - 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

AI Code Review powered by Bito Logo

@github-actions github-actions bot added the api Related to the REST API label Feb 20, 2026
@dosubot dosubot bot added the change:backend Requires changing the backend label Feb 20, 2026
@codecov
Copy link

codecov bot commented Feb 20, 2026

Codecov Report

❌ Patch coverage is 0% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.43%. Comparing base (6fdaa8e) to head (8f51f1b).
⚠️ Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
superset/utils/webdriver.py 0.00% 9 Missing ⚠️
superset/dashboards/api.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #38130       +/-   ##
===========================================
+ Coverage        0   66.43%   +66.43%     
===========================================
  Files           0      670      +670     
  Lines           0    51441    +51441     
  Branches        0     5803     +5803     
===========================================
+ Hits            0    34177    +34177     
- Misses          0    15869    +15869     
- Partials        0     1395     +1395     
Flag Coverage Δ
hive 41.42% <0.00%> (?)
mysql 64.54% <0.00%> (?)
postgres 64.62% <0.00%> (?)
presto 41.44% <0.00%> (?)
python 66.41% <0.00%> (?)
sqlite 64.21% <0.00%> (?)
unit 100.00% <ø> (?)

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

☔ View full report in Codecov by Sentry.
📢 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api Related to the REST API change:backend Requires changing the backend size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant