fix: add logging to bare except blocks in browser/core packages#803
fix: add logging to bare except blocks in browser/core packages#803dashitongzhi wants to merge 3 commits into
Conversation
Several files had bare 'except Exception:' blocks that silently swallowed errors, making debugging difficult. Added logger.debug calls with exc_info=True to the most impactful ones: - csspaths.py: log when build_csspath falls back to basic selector - form_filling.py: log when exact select_option match fails - observation.py: log JPEG header parsing and PIL image open failures All use debug level to avoid noise in production while remaining available for troubleshooting when needed.
|
PR author is not in the allowed authors list. |
WalkthroughAdded debug logging (including full traceback via Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR aims to make previously silent exception paths in the browser/core stack diagnosable by adding debug-level logging when fallbacks are triggered.
Changes:
- Added debug logs when JPEG header parsing or PIL decoding fails during screenshot validation.
- Added debug log when exact
select_optionfails before attempting a case-insensitive fallback. - Added debug log when CSS selector construction fails before falling back to a basic selector.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/notte-core/src/notte_core/browser/observation.py | Adds debug logs on screenshot parsing/decoding failure paths. |
| packages/notte-browser/src/notte_browser/form_filling.py | Adds debug log when exact select matching fails before fallback logic. |
| packages/notte-browser/src/notte_browser/dom/csspaths.py | Adds debug log when CSS selector construction fails before fallback selector. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return True | ||
| except Exception: | ||
| except Exception as e: | ||
| logger.debug(f"Exact match failed for {field_type} field, trying case-insensitive match: {e}", exc_info=True) |
| except Exception: | ||
| # Fallback to a more basic selector if something goes wrong | ||
| except Exception as e: | ||
| logger.debug(f"build_csspath failed, falling back to basic selector: {e}", exc_info=True) |
| logger.debug("JPEG header parsing failed, falling through to PIL path", exc_info=True) | ||
|
|
||
| # Slow path: use PIL for non-JPEG or images that need padding | ||
| try: | ||
| img = Image.open(io.BytesIO(v)) | ||
| except Exception: | ||
| logger.debug("PIL Image.open failed for screenshot data, returning empty observation screenshot", exc_info=True) |
| logger.debug("JPEG header parsing failed, falling through to PIL path", exc_info=True) | ||
|
|
||
| # Slow path: use PIL for non-JPEG or images that need padding | ||
| try: | ||
| img = Image.open(io.BytesIO(v)) | ||
| except Exception: | ||
| logger.debug("PIL Image.open failed for screenshot data, returning empty observation screenshot", exc_info=True) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/notte-browser/src/notte_browser/dom/csspaths.py`:
- Around line 2-4: Import block in csspaths.py is not formatted/sorted per Ruff;
reorder the imports so stdlib imports (import re) come first, then a blank line,
then local/package imports (from notte_core.common.logging import logger), or
simply run the suggested auto-fix command to apply Ruff’s import ordering: ruff
check --fix packages/notte-browser/src/notte_browser/dom/csspaths.py; ensure the
top-level symbols "re" and "logger" are separated into proper import groups with
a blank line between them.
In `@packages/notte-core/src/notte_core/browser/observation.py`:
- Around line 15-16: The import order for nuit_core modules is incorrect: move
the `from notte_core.common.logging import logger` import to its alphabetical
position within the existing `notte_core.*` block (so it appears after `from
notte_core.common.config ...` and before any later `notte_core` imports) to
satisfy Ruff I001; locate the `logger` import in this file (it sits alongside
`from notte_core.actions import ActionUnion`) and reorder it accordingly (or run
`ruff check --fix` on this file to auto-fix).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 91d27433-5424-4d1d-b3e7-e5bf753c06cf
📒 Files selected for processing (3)
packages/notte-browser/src/notte_browser/dom/csspaths.pypackages/notte-browser/src/notte_browser/form_filling.pypackages/notte-core/src/notte_core/browser/observation.py
|
2027 auto-runs evals against preview deployments of your docs. To enable this, install one of:
Once a preview is deployed, open a new PR and we'll run the eval automatically. Evaluating agent experience using 2027.dev · View dashboard |
![mendral-app[bot]](https://avatars.githubusercontent.com/in/1975249?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Fixes I001 import sorting in csspaths.py and observation.py, and reformats 3 files to pass pre-commit ruff hooks.
There was a problem hiding this comment.
LGTM
Both previous import-sorting issues are fixed in commit 129ff98. The CI failures are all integration tests hitting the staging cluster (HTTP 529 "Cluster is under heavy load" / 502 Bad Gateway) — a pre-existing infrastructure issue tracked in insight #01KP9XGDY6FQENXZ0W8V6QK5H4, not caused by this PR. Code is ready to merge.
Tag @mendral-app with feedback or questions. View session
Summary
Multiple files had bare
except Exception:blocks that silently swallowed errors, making it very difficult to diagnose issues in production. This PR addslogger.debugcalls withexc_info=Trueto the most impactful ones.Changes
csspaths.py (
build_csspath)except Exception:→except Exception as e:with debug loggingform_filling.py (
_fill_select_field)except Exception:→except Exception as e:with debug loggingselect_optionmatch fails, now logs the reason before trying case-insensitive matchobservation.py (
Screenshot.validate_raw)Image.openexception: logs before returning empty observation screenshotAll logging uses
logger.debugwithexc_info=Trueto avoid noise in production while providing full traceback when debug level is enabled.Summary by CodeRabbit
Note
Adds
logger.debug(..., exc_info=True)to three bareexcept Exception:blocks incsspaths.py,form_filling.py, andobservation.pyto surface previously swallowed errors during debugging. A follow-up commit fixes import sorting to pass ruff pre-commit checks.Written by Mendral for commit 129ff98.