Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 44 additions & 2 deletions apps/worker/tasks/bundle_analysis_notify.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
BUNDLE_ANALYSIS_NOTIFY_MAX_RETRIES,
bundle_analysis_notify_task_name,
)
from shared.django_apps.upload_breadcrumbs.models import Errors, Milestones
from shared.yaml import UserYaml
from tasks.base import BaseCodecovTask

Expand Down Expand Up @@ -55,21 +56,42 @@ def run_impl(
commitid=commitid,
)

bc_kwargs = {
"commit_sha": commitid,
"repo_id": repoid,
"task_name": self.name,
"parent_task_id": self.request.parent_id,
}

self._call_upload_breadcrumb_task(
milestone=Milestones.LOCK_ACQUIRING, **bc_kwargs
)

try:
with lock_manager.locked(
LockType.BUNDLE_ANALYSIS_NOTIFY,
max_retries=self.max_retries,
retry_num=self.attempts,
):
return self.process_impl_within_lock(
self._call_upload_breadcrumb_task(
milestone=Milestones.LOCK_ACQUIRED, **bc_kwargs
)
result = self.process_impl_within_lock(
db_session=db_session,
repoid=repoid,
commitid=commitid,
commit_yaml=commit_yaml,
previous_result=previous_result,
**kwargs,
)
self._call_upload_breadcrumb_task(
milestone=Milestones.LOCK_RELEASED, **bc_kwargs
)
return result
except LockRetry as retry:
self._call_upload_breadcrumb_task(
error=Errors.INTERNAL_LOCK_ERROR, **bc_kwargs
)
if retry.max_retries_exceeded:
log.error(
"Not retrying lock acquisition - max retries exceeded",
Expand Down Expand Up @@ -112,14 +134,30 @@ def process_impl_within_lock(
)
assert commit, "commit not found"

bc_kwargs = {
"commit_sha": commitid,
"repo_id": repoid,
"task_name": self.name,
"parent_task_id": self.request.parent_id,
}

self._call_upload_breadcrumb_task(
milestone=Milestones.NOTIFICATIONS_TRIGGERED, **bc_kwargs
)

# previous_result is the list of processing results from prior processor tasks
# (they get accumulated as we execute each task in succession)
processing_results = (
previous_result if isinstance(previous_result, list) else []
)

if all(result["error"] is not None for result in processing_results):
# every processor errored, nothing to notify on
self._call_upload_breadcrumb_task(
milestone=Milestones.NOTIFICATIONS_SENT,
error=Errors.UNKNOWN,
error_text="All bundle analysis processors errored",
**bc_kwargs,
)
return {
"notify_attempted": False,
"notify_succeeded": NotificationSuccess.ALL_ERRORED,
Expand All @@ -133,6 +171,10 @@ def process_impl_within_lock(
)
result = notifier.notify()

self._call_upload_breadcrumb_task(
milestone=Milestones.NOTIFICATIONS_SENT, **bc_kwargs
)

log.info(
"Finished bundle analysis notify",
extra={
Expand Down
55 changes: 51 additions & 4 deletions apps/worker/tasks/bundle_analysis_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
BUNDLE_ANALYSIS_PROCESSOR_MAX_RETRIES,
bundle_analysis_processor_task_name,
)
from shared.django_apps.upload_breadcrumbs.models import Errors, Milestones
from shared.reports.enums import UploadState
from shared.yaml import UserYaml
from tasks.base import BaseCodecovTask
Expand Down Expand Up @@ -124,21 +125,42 @@ def run_impl(
commitid=commitid,
)

bc_kwargs = {
"commit_sha": commitid,
"repo_id": repoid,
"task_name": self.name,
"parent_task_id": self.request.parent_id,
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected keyword arguments crash all breadcrumb calls

High Severity

The bc_kwargs dictionaries include task_name and parent_task_id, but _call_upload_breadcrumb_task only accepts commit_sha, repo_id, milestone, upload_ids, error, and error_text — it has no **kwargs. Every call using **bc_kwargs will raise a TypeError for unexpected keyword arguments. Since the error occurs at the call site (not inside the method's try/except), it will propagate unhandled and crash the bundle analysis tasks.

Additional Locations (2)

Fix in Cursor Fix in Web


self._call_upload_breadcrumb_task(
milestone=Milestones.LOCK_ACQUIRING, **bc_kwargs
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-existent milestone enum values cause AttributeError

High Severity

Milestones.LOCK_ACQUIRING, Milestones.LOCK_ACQUIRED, and Milestones.LOCK_RELEASED don't exist in the Milestones enum. The enum only defines FETCHING_COMMIT_DETAILS, COMMIT_PROCESSED, PREPARING_FOR_REPORT, READY_FOR_REPORT, WAITING_FOR_COVERAGE_UPLOAD, COMPILING_UPLOADS, PROCESSING_UPLOAD, NOTIFICATIONS_TRIGGERED, UPLOAD_COMPLETE, and NOTIFICATIONS_SENT. Accessing these non-existent attributes raises an AttributeError at runtime, crashing the tasks.

Additional Locations (1)

Fix in Cursor Fix in Web

)

try:
with lock_manager.locked(
LockType.BUNDLE_ANALYSIS_PROCESSING,
retry_num=self.attempts,
max_retries=self.max_retries,
):
return self.process_impl_within_lock(
self._call_upload_breadcrumb_task(
milestone=Milestones.LOCK_ACQUIRED, **bc_kwargs
)
result = self.process_impl_within_lock(
db_session,
repoid,
commitid,
UserYaml.from_dict(commit_yaml),
params,
previous_result,
)
self._call_upload_breadcrumb_task(
milestone=Milestones.LOCK_RELEASED, **bc_kwargs
)
return result
except LockRetry as retry:
self._call_upload_breadcrumb_task(
error=Errors.INTERNAL_LOCK_ERROR, **bc_kwargs
)
# Honor LockManager cap (Redis attempt count) so re-delivered messages stop.
if retry.max_retries_exceeded or self._has_exceeded_max_attempts(
self.max_retries
Expand Down Expand Up @@ -255,6 +277,18 @@ def process_impl_within_lock(

assert upload is not None

bc_kwargs = {
"commit_sha": commitid,
"repo_id": repoid,
"upload_ids": [upload.id_],
"task_name": self.name,
"parent_task_id": self.request.parent_id,
}

self._call_upload_breadcrumb_task(
milestone=Milestones.PROCESSING_UPLOAD, **bc_kwargs
)

# Override base commit of comparisons with a custom commit SHA if applicable
compare_sha: str | None = cast(
str | None, params.get("bundle_analysis_compare_sha")
Expand Down Expand Up @@ -285,6 +319,9 @@ def process_impl_within_lock(
attempts=self.attempts,
max_retries=self.max_retries,
)
self._call_upload_breadcrumb_task(
error=Errors.INTERNAL_OUT_OF_RETRIES, **bc_kwargs
)
try:
result.update_upload(carriedforward=carriedforward)
db_session.commit()
Expand All @@ -301,6 +338,9 @@ def process_impl_within_lock(
db_session, upload, commitid, repoid, log_suffix=" fallback"
)
return processing_results
self._call_upload_breadcrumb_task(
error=Errors.INTERNAL_RETRYING, **bc_kwargs
)
log.warn(
"Attempting to retry bundle analysis upload",
extra={
Expand All @@ -319,10 +359,12 @@ def process_impl_within_lock(
db_session.commit()

processing_results.append(result.as_dict())

self._call_upload_breadcrumb_task(
milestone=Milestones.UPLOAD_COMPLETE, **bc_kwargs
)
except (CeleryError, SoftTimeLimitExceeded):
# This generally happens when the task needs to be retried because we attempt to access
# the upload before it is saved to GCS. Anecdotally, it takes around 30s for it to be
# saved, but if the BA processor runs before that we will error and need to retry.
self._call_upload_breadcrumb_task(error=Errors.TASK_TIMED_OUT, **bc_kwargs)
raise
except Exception:
log.exception(
Expand All @@ -336,6 +378,11 @@ def process_impl_within_lock(
"parent_task": self.request.parent_id,
},
)
self._call_upload_breadcrumb_task(
error=Errors.UNKNOWN,
error_text="Unhandled exception during bundle analysis processing",
**bc_kwargs,
)
_set_upload_error_and_commit(db_session, upload, commitid, repoid)
return processing_results
finally:
Expand Down
Loading