fix(worker, shared): Handle urllib3 ReadTimeoutError during report chunk retrieval#854
fix(worker, shared): Handle urllib3 ReadTimeoutError during report chunk retrieval#854sentry[bot] wants to merge 1 commit intomainfrom
Conversation
| ) | ||
| return None | ||
|
|
There was a problem hiding this comment.
Bug: The exception handler for urllib3.exceptions.ReadTimeoutError is incomplete. Due to the Minio client's retry configuration, a urllib3.exceptions.MaxRetryError will be raised, which is not caught.
Severity: HIGH
Suggested Fix
Update the except block to also catch urllib3.exceptions.MaxRetryError. This will ensure that read timeouts are handled correctly even after all retries from the Minio client's urllib3 configuration are exhausted.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: apps/worker/services/report/__init__.py#L346-L348
Potential issue: The Minio client is configured with `Retry(total=5)`. When a
`ReadTimeoutError` occurs while reading data chunks, `urllib3` retries the operation. If
all retries fail, `urllib3` raises a `MaxRetryError` wrapping the original
`ReadTimeoutError`. The new `try...except` block only catches
`urllib3.exceptions.ReadTimeoutError`, not `urllib3.exceptions.MaxRetryError`. This will
result in an unhandled exception, causing the worker to crash, which is the exact issue
the change was intended to prevent.
Did we get this right? 👍 / 👎 to inform future reviews.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #854 +/- ##
==========================================
- Coverage 92.25% 92.24% -0.02%
==========================================
Files 1307 1307
Lines 48017 48025 +8
Branches 1636 1636
==========================================
+ Hits 44299 44301 +2
- Misses 3407 3413 +6
Partials 311 311
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Fixes WORKER-Y8M. The issue was that: ReportService.get_existing_report_for_commit fails to catch ReadTimeoutError during GCS chunk retrieval, crashing UploadFinisher.
urllib3.exceptionsimport toapps/worker/services/report/__init__.py.urllib3.exceptions.ReadTimeoutErrorhandling inapps/worker/services/report/__init__.pyto log a warning and returnNonewhen a timeout occurs while reading report chunks.urllib3.exceptionsimport tolibs/shared/shared/reports/api_report_service.py.urllib3.exceptions.ReadTimeoutErrorhandling inlibs/shared/shared/reports/api_report_service.pyto log a warning and returnNonewhen a timeout occurs while reading report chunks.This fix was generated by Seer in Sentry, triggered automatically. 👁️ Run ID: 13513494
Not quite right? Click here to continue debugging with Seer.
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
Note
Low Risk
Low risk: adds defensive handling for storage read timeouts, changing behavior from crashing to logging and returning
None(which may skip report-based processing on transient failures).Overview
Prevents worker/shared report-building code from crashing when chunk retrieval from storage times out.
Both
ReportService.get_existing_report_for_commit(worker) andbuild_report_from_commit(shared) now catchurllib3.exceptions.ReadTimeoutError, log a warning with commit/repo context, and returnNoneto allow downstream flows (e.g., upload finishing/comparisons) to continue gracefully.Reviewed by Cursor Bugbot for commit cf82439. Bugbot is set up for automated code reviews on this repo. Configure here.