fix(rq): Only capture exception if RQ job has failed (ignore retries) (getsentry#1076)

BobReid · web-flow · commit 76aa18927411 · 2021-05-03T17:25:32.000+02:00
diff --git a/sentry_sdk/integrations/rq.py b/sentry_sdk/integrations/rq.py
@@ -3,30 +3,28 @@
 import weakref
 
 from sentry_sdk.hub import Hub
-from sentry_sdk.integrations import Integration, DidNotEnable
+from sentry_sdk.integrations import DidNotEnable, Integration
+from sentry_sdk.integrations.logging import ignore_logger
 from sentry_sdk.tracing import Transaction
 from sentry_sdk.utils import capture_internal_exceptions, event_from_exception
 
-
 try:
-    from rq.version import VERSION as RQ_VERSION
+    from rq.queue import Queue
     from rq.timeouts import JobTimeoutException
+    from rq.version import VERSION as RQ_VERSION
     from rq.worker import Worker
-    from rq.queue import Queue
 except ImportError:
     raise DidNotEnable("RQ not installed")
 
 from sentry_sdk._types import MYPY
 
 if MYPY:
-    from typing import Any
-    from typing import Dict
-    from typing import Callable
-
-    from rq.job import Job
+    from typing import Any, Callable, Dict
 
-    from sentry_sdk.utils import ExcInfo
     from sentry_sdk._types import EventProcessor
+    from sentry_sdk.utils import ExcInfo
+
+    from rq.job import Job
 
 
 class RqIntegration(Integration):
@@ -89,7 +87,9 @@ def sentry_patched_perform_job(self, job, *args, **kwargs):
 
         def sentry_patched_handle_exception(self, job, *exc_info, **kwargs):
             # type: (Worker, Any, *Any, **Any) -> Any
-            _capture_exception(exc_info)  # type: ignore
+            if job.is_failed:
+                _capture_exception(exc_info)  # type: ignore
+
             return old_handle_exception(self, job, *exc_info, **kwargs)
 
         Worker.handle_exception = sentry_patched_handle_exception
@@ -108,6 +108,8 @@ def sentry_patched_enqueue_job(self, job, **kwargs):
 
         Queue.enqueue_job = sentry_patched_enqueue_job
 
+        ignore_logger("rq.worker")
+
 
 def _make_event_processor(weak_job):
     # type: (Callable[[], Job]) -> EventProcessor
diff --git a/tests/integrations/rq/test_rq.py b/tests/integrations/rq/test_rq.py
@@ -1,8 +1,7 @@
-from sentry_sdk.integrations.rq import RqIntegration
-
 import pytest
-
 from fakeredis import FakeStrictRedis
+from sentry_sdk.integrations.rq import RqIntegration
+
 import rq
 
 try:
@@ -177,3 +176,19 @@ def test_traces_sampler_gets_correct_values_in_sampling_context(
             }
         )
     )
+
+
+@pytest.mark.skipif(
+    rq.__version__.split(".") < ["1", "5"], reason="At least rq-1.5 required"
+)
+def test_job_with_retries(sentry_init, capture_events):
+    sentry_init(integrations=[RqIntegration()])
+    events = capture_events()
+
+    queue = rq.Queue(connection=FakeStrictRedis())
+    worker = rq.SimpleWorker([queue], connection=queue.connection)
+
+    queue.enqueue(crashing_job, foo=42, retry=rq.Retry(max=1))
+    worker.work(burst=True)
+
+    assert len(events) == 1
