refactor: Rename LockMaxRetriesExceeded to LockMaxRetriesExceededError and enhance its initialization with detailed parameters

Author: drazisil-codecov

apps/worker/services/lock_manager.py

@@ -39,9 +39,24 @@ def __init__(self, countdown: int):
         self.countdown = countdown
 
 
-class LockMaxRetriesExceeded(Exception):
+class LockMaxRetriesExceededError(Exception):
     """Raised when lock acquisition exceeds max retries."""
 
+    def __init__(
+        self,
+        retry_num: int,
+        max_attempts: int,
+        lock_name: str,
+        repoid: int,
+        commitid: str,
+    ):
+        self.retry_num = retry_num
+        self.max_attempts = max_attempts
+        self.lock_name = lock_name
+        self.repoid = repoid
+        self.commitid = commitid
+        super().__init__("Lock acquisition failed after exceeding max retries")
+
 
 class LockManager:
     def __init__(
@@ -131,9 +146,12 @@ def locked(
             # None means no max retries (infinite retries)
             max_attempts = max_retries + 1 if max_retries is not None else None
             if max_attempts is not None and retry_num >= max_attempts:
-                error_msg = (
-                    f"Lock acquisition failed after {retry_num} attempts (max: {max_attempts}). "
-                    f"Lock: {lock_name}, Repo: {self.repoid}, Commit: {self.commitid}"
+                error = LockMaxRetriesExceededError(
+                    retry_num=retry_num,
+                    max_attempts=max_attempts,
+                    lock_name=lock_name,
+                    repoid=self.repoid,
+                    commitid=self.commitid,
                 )
                 log.error(
                     "Not retrying since we already had too many retries",
@@ -150,7 +168,7 @@ def locked(
                     exc_info=True,
                 )
                 sentry_sdk.capture_exception(
-                    LockMaxRetriesExceeded(error_msg),
+                    error,
                     contexts={
                         "lock_acquisition": {
                             "blocking_timeout": self.blocking_timeout,
@@ -171,6 +189,7 @@ def locked(
                         "lock_type": lock_type.value,
                     },
                 )
+                # TODO: should we raise this, or would a return be ok?
                 raise LockRetry(countdown)
 
             log.warning(

apps/worker/tasks/bundle_analysis_processor.py

@@ -189,6 +189,7 @@ def process_impl_within_lock(
             str | None, params.get("bundle_analysis_compare_sha")
         )
 
+        result: ProcessingResult | None = None
         try:
             log.info(
                 "Processing bundle analysis upload",
@@ -204,9 +205,7 @@ def process_impl_within_lock(
             )
             assert params.get("commit") == commit.commitid
 
-            result: ProcessingResult = report_service.process_upload(
-                commit, upload, compare_sha
-            )
+            result = report_service.process_upload(commit, upload, compare_sha)
             if result.error and result.error.is_retryable:
                 if self._has_exceeded_max_attempts(self.max_retries):
                     attempts = self.attempts
@@ -232,7 +231,10 @@ def process_impl_within_lock(
                         "result": result.as_dict(),
                     },
                 )
-                self.retry(max_retries=self.max_retries, countdown=30 * (2**self.request.retries))
+                self.retry(
+                    max_retries=self.max_retries,
+                    countdown=30 * (2**self.request.retries),
+                )
             result.update_upload(carriedforward=carriedforward)
             db_session.commit()
 
@@ -270,9 +272,9 @@ def process_impl_within_lock(
                 )
             raise
         finally:
-            if "result" in locals() and result.bundle_report:
+            if result is not None and result.bundle_report:
                 result.bundle_report.cleanup()
-            if "result" in locals() and result.previous_bundle_report:
+            if result is not None and result.previous_bundle_report:
                 result.previous_bundle_report.cleanup()
 
         # Create task to save bundle measurements

apps/worker/tasks/tests/unit/test_bundle_analysis_processor_task.py

@@ -144,6 +144,7 @@ def test_bundle_analysis_processor_task_error(
 
     task = BundleAnalysisProcessorTask()
     retry = mocker.patch.object(task, "retry")
+    task.request.retries = 0
 
     result = task.run_impl(
         dbsession,
@@ -171,7 +172,7 @@ def test_bundle_analysis_processor_task_error(
 
     assert commit.state == "error"
     assert upload.state == "error"
-    retry.assert_called_once_with(countdown=30)
+    retry.assert_called_once_with(max_retries=5, countdown=30)
 
 
 def test_bundle_analysis_processor_task_general_error(
@@ -390,6 +391,7 @@ def test_bundle_analysis_process_upload_rate_limit_error(
 
     task = BundleAnalysisProcessorTask()
     retry = mocker.patch.object(task, "retry")
+    task.request.retries = 0
 
     ingest = mocker.patch("shared.bundle_analysis.BundleAnalysisReport.ingest")
     ingest.side_effect = PutRequestRateLimitError()
@@ -422,7 +424,7 @@ def test_bundle_analysis_process_upload_rate_limit_error(
 
     assert commit.state == "error"
     assert upload.state == "error"
-    retry.assert_called_once_with(countdown=30)
+    retry.assert_called_once_with(max_retries=5, countdown=30)
 
 
 def test_bundle_analysis_process_associate_no_parent_commit_id(