From 9af82ceb28499a5d5c2bbc3302291dd6a61fc15e Mon Sep 17 00:00:00 2001 From: "Armen Zambrano G." <44410+armenzg@users.noreply.github.com> Date: Mon, 24 Nov 2025 09:34:08 -0500 Subject: [PATCH 1/3] ref(taskbroker): Clarify docstring & class name Changes included: * A Markdown table is added to the docstring describing how to call the `retry` decorator. * It renames `RetryError` to `RetryTaskError` to help search the codebase. --- src/sentry/tasks/base.py | 44 ++++++++++++++++++---------------- src/sentry/taskworker/retry.py | 8 +++---- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/sentry/tasks/base.py b/src/sentry/tasks/base.py index b130b90f70ce0d..bdf0a76f379ea7 100644 --- a/src/sentry/tasks/base.py +++ b/src/sentry/tasks/base.py @@ -11,7 +11,7 @@ from sentry.taskworker.constants import CompressionType from sentry.taskworker.registry import TaskNamespace -from sentry.taskworker.retry import Retry, RetryError, retry_task +from sentry.taskworker.retry import Retry, RetryTaskError, retry_task from sentry.taskworker.state import current_task from sentry.taskworker.task import P, R, Task from sentry.taskworker.workerchild import ProcessingDeadlineExceeded @@ -123,10 +123,24 @@ def retry( >>> def my_task(): >>> ... - If timeouts is True, task timeout exceptions will trigger a retry. - If it is False, timeout exceptions will behave as specified by the other parameters. - """ + The first set of parameters define how different exceptions are handled. + Raising an error will still report a Sentry event. + + | Parameter | Retry | Report | Raise | Description | + |--------------------|-------|--------|-------|-------------| + | on | Yes | Yes | No | Exceptions that will trigger a retry & report to Sentry. | + | on_silent | Yes | No | No | Exceptions that will trigger a retry but not be captured to Sentry. | + | exclude | No | No | Yes | Exceptions that will not trigger a retry and will be raised. | + | ignore | No | No | No | Exceptions that will be ignored and not trigger a retry & not report to Sentry. | + | ignore_and_capture | No | Yes | No | Exceptions that will not trigger a retry and will be captured to Sentry. | + The following modifiers modify the behavior of the retry decorator. + + | Modifier | Description | + |------------------------|-------------| + | timeouts | ProcessingDeadlineExceeded trigger a retry. | + | raise_on_no_retries | Makes a RetryTaskError not be raised if no retries are left. | + """ if func: return retry()(func) @@ -138,30 +152,20 @@ def retry( def inner(func): @functools.wraps(func) def wrapped(*args, **kwargs): + task_state = current_task() + no_retries_remaining = task_state and not task_state.retries_remaining try: return func(*args, **kwargs) except ignore: return - except RetryError: - if ( - not raise_on_no_retries - and (task_state := current_task()) - and not task_state.retries_remaining - ): + except RetryTaskError: + if not raise_on_no_retries and no_retries_remaining: return - # If we haven't been asked to ignore no-retries, pass along the RetryError. + # If we haven't been asked to ignore no-retries, pass along the RetryTaskError. raise except timeout_exceptions: if timeouts: - with sentry_sdk.isolation_scope() as scope: - task_state = current_task() - if task_state: - scope.fingerprint = [ - "task.processing_deadline_exceeded", - task_state.namespace, - task_state.taskname, - ] - sentry_sdk.capture_exception(level="info") + sentry_sdk.capture_exception(level="info") retry_task(raise_on_no_retries=raise_on_no_retries) else: raise diff --git a/src/sentry/taskworker/retry.py b/src/sentry/taskworker/retry.py index 020bd59c906393..00d9a40fdc8555 100644 --- a/src/sentry/taskworker/retry.py +++ b/src/sentry/taskworker/retry.py @@ -14,14 +14,14 @@ from sentry.utils import metrics -class RetryError(Exception): +class RetryTaskError(Exception): """ Exception that tasks can raise to indicate that the current task activation should be retried. """ -class NoRetriesRemainingError(RetryError): +class NoRetriesRemainingError(RetryTaskError): """ Exception that is raised by retry helper methods to signal to tasks that the current attempt is terminal and there won't be any further retries. @@ -53,7 +53,7 @@ def retry_task(exc: Exception | None = None, raise_on_no_retries: bool = True) - raise NoRetriesRemainingError() else: return - raise RetryError() + raise RetryTaskError() class Retry: @@ -85,7 +85,7 @@ def should_retry(self, state: RetryState, exc: Exception) -> bool: return False # Explicit RetryError with attempts left. - if isinstance(exc, RetryError): + if isinstance(exc, RetryTaskError): return True # No retries for types on the ignore list From ff72ff2cf953574f53da78cdf01d316517f77d9f Mon Sep 17 00:00:00 2001 From: "Armen Zambrano G." <44410+armenzg@users.noreply.github.com> Date: Mon, 24 Nov 2025 09:45:01 -0500 Subject: [PATCH 2/3] Properly refactor the class rename --- .../notifications/notification_action/types.py | 4 ++-- src/sentry/taskworker/retry.py | 2 +- src/sentry/taskworker/tasks/examples.py | 8 ++++---- .../integrations/github/tasks/test_link_all_repos.py | 4 ++-- .../sentry/integrations/tasks/test_create_comment.py | 4 ++-- .../tasks/test_sync_assignee_outbound.py | 4 ++-- .../integrations/tasks/test_sync_status_outbound.py | 4 ++-- .../sentry/integrations/tasks/test_update_comment.py | 4 ++-- .../test_issue_alert_registry_handlers.py | 4 ++-- tests/sentry/tasks/test_base.py | 12 ++++++------ tests/sentry/taskworker/test_retry.py | 4 ++-- tests/sentry/taskworker/test_task.py | 4 ++-- 12 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/sentry/notifications/notification_action/types.py b/src/sentry/notifications/notification_action/types.py index 07f13d4a173546..092f10bfe50719 100644 --- a/src/sentry/notifications/notification_action/types.py +++ b/src/sentry/notifications/notification_action/types.py @@ -33,7 +33,7 @@ IntegrationConfigurationError, IntegrationFormError, ) -from sentry.taskworker.retry import RetryError +from sentry.taskworker.retry import RetryTaskError from sentry.taskworker.workerchild import ProcessingDeadlineExceeded from sentry.types.activity import ActivityType from sentry.types.rules import RuleFuture @@ -97,7 +97,7 @@ def invoke_future_with_error_handling( # monitor and potentially retry this action. raise except RETRYABLE_EXCEPTIONS as e: - raise RetryError from e + raise RetryTaskError from e except Exception as e: # This is just a redefinition of the safe_execute util function, as we # still want to report any unhandled exceptions. diff --git a/src/sentry/taskworker/retry.py b/src/sentry/taskworker/retry.py index 00d9a40fdc8555..cf78d9913bf5f8 100644 --- a/src/sentry/taskworker/retry.py +++ b/src/sentry/taskworker/retry.py @@ -84,7 +84,7 @@ def should_retry(self, state: RetryState, exc: Exception) -> bool: if self.max_attempts_reached(state): return False - # Explicit RetryError with attempts left. + # Explicit RetryTaskError with attempts left. if isinstance(exc, RetryTaskError): return True diff --git a/src/sentry/taskworker/tasks/examples.py b/src/sentry/taskworker/tasks/examples.py index 3113c1b0bc9cb1..285035bf9acae2 100644 --- a/src/sentry/taskworker/tasks/examples.py +++ b/src/sentry/taskworker/tasks/examples.py @@ -6,7 +6,7 @@ from sentry.taskworker.constants import CompressionType from sentry.taskworker.namespaces import exampletasks -from sentry.taskworker.retry import LastAction, NoRetriesRemainingError, Retry, RetryError +from sentry.taskworker.retry import LastAction, NoRetriesRemainingError, Retry, RetryTaskError from sentry.taskworker.retry import retry_task as retry_task_helper from sentry.utils.redis import redis_clusters @@ -22,7 +22,7 @@ def say_hello(name: str, *args: list[Any], **kwargs: dict[str, Any]) -> None: name="examples.retry_deadletter", retry=Retry(times=2, times_exceeded=LastAction.Deadletter) ) def retry_deadletter() -> None: - raise RetryError + raise RetryTaskError @exampletasks.register( @@ -43,7 +43,7 @@ def retry_state() -> None: def will_retry(failure: str) -> None: if failure == "retry": logger.debug("going to retry with explicit retry error") - raise RetryError + raise RetryTaskError if failure == "raise": logger.debug("raising runtimeerror") raise RuntimeError("oh no") @@ -71,7 +71,7 @@ def simple_task_wait_delivery() -> None: @exampletasks.register(name="examples.retry_task", retry=Retry(times=2)) def retry_task() -> None: - raise RetryError + raise RetryTaskError @exampletasks.register(name="examples.fail_task") diff --git a/tests/sentry/integrations/github/tasks/test_link_all_repos.py b/tests/sentry/integrations/github/tasks/test_link_all_repos.py index fada2927434f72..7949e7599da31f 100644 --- a/tests/sentry/integrations/github/tasks/test_link_all_repos.py +++ b/tests/sentry/integrations/github/tasks/test_link_all_repos.py @@ -11,7 +11,7 @@ from sentry.integrations.types import EventLifecycleOutcome from sentry.models.repository import Repository from sentry.silo.base import SiloMode -from sentry.taskworker.retry import RetryError +from sentry.taskworker.retry import RetryTaskError from sentry.testutils.asserts import assert_failure_metric, assert_halt_metric, assert_slo_metric from sentry.testutils.cases import IntegrationTestCase from sentry.testutils.silo import assume_test_silo_mode, control_silo_test @@ -194,7 +194,7 @@ def test_link_all_repos_api_error(self, mock_record: MagicMock, _: MagicMock) -> status=400, ) - with pytest.raises(RetryError): + with pytest.raises(RetryTaskError): link_all_repos( integration_key=self.key, integration_id=self.integration.id, diff --git a/tests/sentry/integrations/tasks/test_create_comment.py b/tests/sentry/integrations/tasks/test_create_comment.py index d1f77c8daeb743..9e46a6f9b20575 100644 --- a/tests/sentry/integrations/tasks/test_create_comment.py +++ b/tests/sentry/integrations/tasks/test_create_comment.py @@ -7,7 +7,7 @@ from sentry.integrations.tasks import create_comment from sentry.integrations.types import EventLifecycleOutcome from sentry.models.activity import Activity -from sentry.taskworker.retry import RetryError +from sentry.taskworker.retry import RetryTaskError from sentry.testutils.asserts import assert_failure_metric, assert_slo_metric from sentry.testutils.cases import TestCase from sentry.testutils.silo import assume_test_silo_mode_of @@ -157,7 +157,7 @@ def test_create_comment_failure( integration=self.example_integration, ) - with pytest.raises(RetryError): + with pytest.raises(RetryTaskError): create_comment(external_issue.id, self.user.id, self.activity.id) assert_failure_metric(mock_record_event, Exception("Something went wrong creating comment")) diff --git a/tests/sentry/integrations/tasks/test_sync_assignee_outbound.py b/tests/sentry/integrations/tasks/test_sync_assignee_outbound.py index c0a7d0de485949..b84f64977d5dc7 100644 --- a/tests/sentry/integrations/tasks/test_sync_assignee_outbound.py +++ b/tests/sentry/integrations/tasks/test_sync_assignee_outbound.py @@ -10,7 +10,7 @@ from sentry.integrations.tasks import sync_assignee_outbound from sentry.integrations.types import EventLifecycleOutcome from sentry.shared_integrations.exceptions import IntegrationConfigurationError -from sentry.taskworker.retry import RetryError +from sentry.taskworker.retry import RetryTaskError from sentry.testutils.asserts import assert_halt_metric, assert_success_metric from sentry.testutils.cases import TestCase from sentry.testutils.silo import assume_test_silo_mode_of @@ -72,7 +72,7 @@ def test_sync_failure( ) -> None: mock_sync_assignee.side_effect = raise_sync_assignee_exception - with pytest.raises(RetryError): + with pytest.raises(RetryTaskError): sync_assignee_outbound(self.external_issue.id, self.user.id, True, None) mock_record_failure.assert_called_once() diff --git a/tests/sentry/integrations/tasks/test_sync_status_outbound.py b/tests/sentry/integrations/tasks/test_sync_status_outbound.py index 5ee3f202564eb5..3c779ec2385b7d 100644 --- a/tests/sentry/integrations/tasks/test_sync_status_outbound.py +++ b/tests/sentry/integrations/tasks/test_sync_status_outbound.py @@ -7,7 +7,7 @@ from sentry.integrations.tasks import sync_status_outbound from sentry.integrations.types import EventLifecycleOutcome from sentry.shared_integrations.exceptions import ApiUnauthorized, IntegrationFormError -from sentry.taskworker.retry import RetryError +from sentry.taskworker.retry import RetryTaskError from sentry.testutils.asserts import assert_count_of_metric, assert_halt_metric from sentry.testutils.cases import TestCase from sentry.testutils.silo import assume_test_silo_mode_of, region_silo_test @@ -111,7 +111,7 @@ def test_failed_sync( group=self.group, key="foo_integration", integration=self.example_integration ) - with pytest.raises(RetryError): + with pytest.raises(RetryTaskError): sync_status_outbound(self.group.id, external_issue_id=external_issue.id) assert mock_record_failure.call_count == 1 diff --git a/tests/sentry/integrations/tasks/test_update_comment.py b/tests/sentry/integrations/tasks/test_update_comment.py index cf59a9fda181af..2ac1e011701685 100644 --- a/tests/sentry/integrations/tasks/test_update_comment.py +++ b/tests/sentry/integrations/tasks/test_update_comment.py @@ -7,7 +7,7 @@ from sentry.integrations.tasks import update_comment from sentry.integrations.types import EventLifecycleOutcome from sentry.models.activity import Activity -from sentry.taskworker.retry import RetryError +from sentry.taskworker.retry import RetryTaskError from sentry.testutils.asserts import assert_failure_metric, assert_slo_metric from sentry.testutils.cases import TestCase from sentry.testutils.silo import assume_test_silo_mode_of @@ -150,7 +150,7 @@ def test_update_comment_failure( integration=self.example_integration, ) - with pytest.raises(RetryError): + with pytest.raises(RetryTaskError): update_comment(external_issue.id, self.user.id, self.activity.id) assert_failure_metric(mock_record_event, Exception("Something went wrong updating comment")) diff --git a/tests/sentry/notifications/notification_action/test_issue_alert_registry_handlers.py b/tests/sentry/notifications/notification_action/test_issue_alert_registry_handlers.py index b223e5628471d0..44aa988482f1c9 100644 --- a/tests/sentry/notifications/notification_action/test_issue_alert_registry_handlers.py +++ b/tests/sentry/notifications/notification_action/test_issue_alert_registry_handlers.py @@ -895,11 +895,11 @@ def test_reraises_processing_deadline_exceeded(self): def test_raises_retry_error_for_api_error(self): from sentry.notifications.notification_action.types import invoke_future_with_error_handling from sentry.shared_integrations.exceptions import ApiError - from sentry.taskworker.retry import RetryError + from sentry.taskworker.retry import RetryTaskError self.mock_callback.side_effect = ApiError("API error", 500) - with pytest.raises(RetryError) as excinfo: + with pytest.raises(RetryTaskError) as excinfo: invoke_future_with_error_handling( self.event_data, self.mock_callback, self.mock_futures ) diff --git a/tests/sentry/tasks/test_base.py b/tests/sentry/tasks/test_base.py index 1f979e997f87ed..4e9366f525aeba 100644 --- a/tests/sentry/tasks/test_base.py +++ b/tests/sentry/tasks/test_base.py @@ -8,7 +8,7 @@ from sentry.taskworker.constants import CompressionType from sentry.taskworker.namespaces import exampletasks, test_tasks from sentry.taskworker.registry import TaskRegistry -from sentry.taskworker.retry import Retry, RetryError +from sentry.taskworker.retry import Retry, RetryTaskError from sentry.taskworker.state import CurrentTaskState from sentry.taskworker.workerchild import ProcessingDeadlineExceeded @@ -159,7 +159,7 @@ def test_exclude_exception_retry(capture_exception: MagicMock) -> None: @override_settings(SILO_MODE=SiloMode.CONTROL) @patch("sentry_sdk.capture_exception") def test_retry_on(capture_exception: MagicMock) -> None: - with pytest.raises(RetryError): + with pytest.raises(RetryTaskError): retry_on_task("bruh") assert capture_exception.call_count == 1 @@ -186,7 +186,7 @@ def test_retry_timeout_enabled_taskbroker(capture_exception) -> None: def timeout_retry_task(): raise ProcessingDeadlineExceeded() - with pytest.raises(RetryError): + with pytest.raises(RetryTaskError): timeout_retry_task() assert capture_exception.call_count == 1 @@ -213,7 +213,7 @@ def test_retry_timeout_enabled(capture_exception) -> None: def soft_timeout_retry_task(): raise ProcessingDeadlineExceeded() - with pytest.raises(RetryError): + with pytest.raises(RetryTaskError): soft_timeout_retry_task() assert capture_exception.call_count == 1 @@ -265,13 +265,13 @@ def test_retry_raise_if_no_retries_false(mock_current_task): @retry(on=(Exception,), raise_on_no_retries=False) def task_that_raises_retry_error(): - raise RetryError("try again") + raise RetryTaskError("try again") # No exception. task_that_raises_retry_error() mock_task_state.retries_remaining = True - with pytest.raises(RetryError): + with pytest.raises(RetryTaskError): task_that_raises_retry_error() diff --git a/tests/sentry/taskworker/test_retry.py b/tests/sentry/taskworker/test_retry.py index 01d02e6bd546e2..dde314667fefb9 100644 --- a/tests/sentry/taskworker/test_retry.py +++ b/tests/sentry/taskworker/test_retry.py @@ -7,7 +7,7 @@ ON_ATTEMPTS_EXCEEDED_DISCARD, ) -from sentry.taskworker.retry import LastAction, Retry, RetryError +from sentry.taskworker.retry import LastAction, Retry, RetryTaskError class RuntimeChildError(RuntimeError): @@ -64,7 +64,7 @@ def test_should_retry_retryerror() -> None: retry = Retry(times=5) state = retry.initial_state() - err = RetryError("something bad") + err = RetryTaskError("something bad") assert retry.should_retry(state, err) state.attempts = 4 diff --git a/tests/sentry/taskworker/test_task.py b/tests/sentry/taskworker/test_task.py index 631ec925a137db..6c1951255b67ed 100644 --- a/tests/sentry/taskworker/test_task.py +++ b/tests/sentry/taskworker/test_task.py @@ -10,7 +10,7 @@ ) from sentry.taskworker.registry import TaskNamespace -from sentry.taskworker.retry import LastAction, Retry, RetryError +from sentry.taskworker.retry import LastAction, Retry, RetryTaskError from sentry.taskworker.router import DefaultRouter from sentry.taskworker.task import Task from sentry.testutils.helpers.task_runner import TaskRunner @@ -147,7 +147,7 @@ def test_should_retry(task_namespace: TaskNamespace) -> None: namespace=task_namespace, retry=retry, ) - err = RetryError("try again plz") + err = RetryTaskError("try again plz") assert task.should_retry(state, err) state.attempts = 3 From 3dc2f02741ad540aef0a666a987070795d95c36f Mon Sep 17 00:00:00 2001 From: "Armen Zambrano G." <44410+armenzg@users.noreply.github.com> Date: Mon, 24 Nov 2025 10:44:42 -0500 Subject: [PATCH 3/3] Revert --- src/sentry/tasks/base.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/sentry/tasks/base.py b/src/sentry/tasks/base.py index bdf0a76f379ea7..72614cdadde4f0 100644 --- a/src/sentry/tasks/base.py +++ b/src/sentry/tasks/base.py @@ -165,7 +165,15 @@ def wrapped(*args, **kwargs): raise except timeout_exceptions: if timeouts: - sentry_sdk.capture_exception(level="info") + with sentry_sdk.isolation_scope() as scope: + task_state = current_task() + if task_state: + scope.fingerprint = [ + "task.processing_deadline_exceeded", + task_state.namespace, + task_state.taskname, + ] + sentry_sdk.capture_exception(level="info") retry_task(raise_on_no_retries=raise_on_no_retries) else: raise