fix: stop expected fetch failures raising spurious Sentry issues - #126
Conversation
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughChangesPipeline CSV downloads
Sentry error handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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.
Actionable comments posted: 1
🤖 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 `@request-processor/src/application/core/workflow.py`:
- Around line 325-326: Update the download handling try/except in the workflow
to catch both URLError and socket.timeout, rather than only HTTPError, so all
exceptions re-raised by download_file are logged and the workflow continues
without crashing.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8b19bc5f-a242-44cf-8e88-2a275ddeae06
📒 Files selected for processing (3)
request-processor/src/application/core/workflow.pyrequest-processor/src/tasks.pyrequest-processor/tests/unit/src/application/core/test_workflow.py
What type of PR is this? (check all applicable)
Description
Fixes three sources of spurious Sentry issues raised for expected/handled failures during Check URL and Add Data processing:
esridumpJSON-parse noise: thearcgisfallback plugin is tried against every fetched URL, including ones that are obviously not ArcGIS FeatureServers. When that probe fails,esridumplogs atERRORlevel internally before re-raising an exception that's already caught and handled one level up — Sentry's logging integration was capturing that as an issue regardless. Now ignored viaignore_logger("esridump.dumper")._capture_sentry_event(handled=True)calledcapture_message(level="warning"), but in Sentry a message'slevelonly sets severity — it's still filed as an alertable Issue. Expected user errors (bad/unreachable URLs) are now recorded as asentry_sdk.metrics.countcounter plus a structuredsentry_sdk.logger.warninglog entry (full detail preserved, searchable), instead of creating an Issue. Genuinely unexpected errors (handled=False) are unchanged and still captured as Issues.fetch_pipeline_csvstrieddigital-land/<collection>-collection/main/pipeline/*.csvbefore falling back to the centraldigital-land/configrepo. The per-collection repos have all been retired into thedigital-land-atticorg, so this first attempt now 404s for every collection on every run, adding a wasted request and (via the blanketlogger.errorindownload_file) a Sentry issue each time. Removed, so this now goes straight toCONFIG_URL, matching the pattern already used elsewhere inworkflow.py.Related Tickets & Documents
QA Instructions, Screenshots, Recordings
pytest tests/unit/src/application/core/test_workflow.py(19 passed).brownfield-land,conservation-area,listed-building-outline,tree-preservation-order,article-4-direction) and that the central config repo URL resolves for the same paths.Added/updated tests?
[optional] Are there any post deployment tasks we need to perform?
None.
[optional] Are there any dependencies on other PRs or Work?
None.