sentry-python: Celery integration captures retried exceptions that it shouldn't when one task directly calls another

I have one Celery task that directly calls another, but as a function, not as a task. The first task is auto-retried for a certain set of exceptions, and the second task is retried for a different set of exceptions. Here is an example task in my actual codebase, but here is an isolated illustration of the problem.

Consider the following two tasks, bar which is auto-retried for exceptions A or B, and a wrapper task bat which is auto-retried for exceptions C or D.

def foo():
    ...  # do something that might raise exception A, B, C, or D

@app.task(autoretry_for=(A, B))
def bar():
    return foo()

@app.task(autoretry_for=(C, D))
def bat():
    return bar()

Now invoke bat:

bar.delay()

Now suppose that during the execution of bat, foo() raises exception C. Even though Celery will retry the task, the exception C will be reported to Sentry.

About this issue

Original URL
State: closed
Created 5 years ago
Reactions: 10
Comments: 17 (9 by maintainers)

Commits related to this issue

Work around excessive Sentry reporting of retryable exceptions Work around a bug in the Sentry Python SDK that caused excessive reporting of certain GraceDB exceptions that are listed in tasks' `auto... — committed to lpsinger/gwcelery by lpsinger 5 years ago

Most upvoted comments

Quick update: This issue is still open. And because multiple persons are reporting this it will be kept in our minds for one of the next SDK updates.

antonpirker on Feb 22, 2022

Closing because the clarifying comment kind of makes every one happy (and if not happy, at least not sad anymore.)

antonpirker on Nov 13, 2023

Hey @HoneyryderChuck and @YPCrumble Thanks for your feedback. You are right, better keeping this open for other people to find it if the encounter the same problem.

For transparency, here are some notes(a tl;dr) from our internal discussions/investigations:

this is edge casey, if you use a task decorator, this is what happens
what daniel reported is a more general problem (value of fixing questionable)
we might be able to fix the autoretry behavior separately and neglect this problem
if we do want to fix it
- we need to remove task.run/__call__ patches. patching __call__ is a can of worms because celery is very magical and we have to live with celery’s assumptions and magic
- capture the exception slightly more above in celery/concurrency/* (i.e.e when a worker executes a certain task from the queue and not the actual task function itself)
- for instance here - https://github.com/celery/celery/blob/b96ab282a8a2ea3d97d034f862e9fd6aceb0a0b5/celery/concurrency/base.py#L139-L155
- this would need to be tested thoroughly, risk of breaking existing working stuff

antonpirker on Sep 15, 2023

Are you really closing an issue based on two recommended workarounds which do not work for simple scenarios? I mean, the first recommendation is literally a one-liner with no context. You raise Retry, how do you deal with retry limits? How do you do exponential backoff?.. The second , I mean, I don’t want to handle the exception and pass, I want to either retry it an not get a sentry error notification, or I exhausted retries and I want an error notification. How do you achieve that?

I get that this may be harder to implement than it looks like, and we’re all busy people and have to manage priorities, but can we not sweep this under the rug? I can be sad and accept that it won’t be fixed soon, but I think it’d be more honest to the community if this were kept open, it’s still an unfixed bug.

HoneyryderChuck on Sep 8, 2023