Fix: prioritize request ignore_result over task definition by patri27826 · Pull Request #10184 · celery/celery

patri27826 · 2026-03-08T16:20:33Z

Description

Previous if you set apply_sync(ignore_result=False) with @app.task(ignore_result=True), it would failed to get the task results, because in the trace.py we only use task.ignore_result as the condition to decide whether to publish result.

Add new function get_actual_ignore_result to handle the handle the priority lookup: Request > Task

Changes

Introduced get_actual_ignore_result to handle the priority lookup: Request > Task.
Refactored handle_error_state to respect these overrides.
Moved publish_result evaluation into the execution flow for better accuracy.

Test with three condition

@app.task(ignore_result=True) with apply_sync(ignore_result=False) => Expected Test Result Exist
@app.task(ignore_result=False) with apply_sync(ignore_result=True) => Expected Test Result Not Exist
app.conf.task_ignore_result = True with apply_sync(ignore_result=False) => Expected Test Result Exist

⚠️ Behavioral Changes

This PR fixes an inconsistency where ignore_result=True tasks would still store error results if the request did not explicitly provide an ignore_result key.

Old Behavior: If a request message lacked the ignore_result key, it defaulted to False in the worker request, causing store_errors to be True. This led to failure results being stored even for tasks decorated with @app.task(ignore_result=True).
New Behavior: The worker request now correctly falls back to the task-level ignore_result setting. If a task is configured to ignore results, it will no longer store error results by default unless overridden by the request.

User Impact: Users relying on "accidental" error persistence for tasks with ignore_result=True will notice that these errors are no longer stored in the backend.

Copilot

Pull request overview

This PR fixes result-publishing behavior when ignore_result is overridden at call time (e.g., apply_async(..., ignore_result=False)) but the task/app defaults would otherwise ignore results, addressing #9654 by ensuring request-level ignore_result takes precedence during tracing and error handling.

Changes:

Added get_actual_ignore_result() to compute the effective ignore_result using request-level overrides.
Updated tracing/error paths to use the effective ignore_result when deciding whether to publish/store results.
Added unit tests covering request-overrides-task and request-overrides-config scenarios.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
`celery/app/trace.py`	Computes effective `ignore_result` per-execution and uses it for `publish_result` and error storage decisions.
`celery/worker/request.py`	Makes `Request.store_errors` respect request-level `ignore_result` (via `_ignore_result`) rather than only task-level configuration.
`t/unit/tasks/test_trace.py`	Adds unit tests asserting request-level `ignore_result` precedence during tracing.

auvipy · 2026-03-08T17:37:13Z

please do not force push.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

celery/app/trace.py:195

handle_error_state() now derives ignore_result from the request override, but there isn’t a focused regression test asserting that request-level ignore_result=False/True changes whether failures are stored (i.e., the store_errors decision). Could we add a unit test that exercises a failure path with task.ignore_result=True and a request override of ignore_result=False (and the inverse) to ensure this behavior stays consistent?

    def handle_error_state(self, task, req,
                           eager=False, call_errbacks=True):
        ignore_result = get_actual_ignore_result(task, req)

        if ignore_result:
            store_errors = task.store_errors_even_if_ignored
        elif eager and task.store_eager_result:
            store_errors = True
        else:
            store_errors = not eager

codecov · 2026-03-08T17:44:43Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.23%. Comparing base (6582303) to head (d3fde16).
⚠️ Report is 73 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10184      +/-   ##
==========================================
+ Coverage   88.22%   88.23%   +0.01%     
==========================================
  Files         153      153              
  Lines       19559    19569      +10     
  Branches     2246     2249       +3     
==========================================
+ Hits        17256    17267      +11     
+ Misses       2005     2004       -1     
  Partials      298      298

Flag	Coverage Δ
unittests	`88.21% <100.00%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

auvipy

also, please fix the merge conflicts

This commit adds the missing unit tests to verify the priority logic for 'ignore_result' within the Request class, ensuring it correctly falls back to the task property when not specified in the message.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

You can also share your feedback on Copilot code review. Take the survey.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

You can also share your feedback on Copilot code review. Take the survey.

…nore_result

auvipy · 2026-03-14T16:49:08Z

@ChickenBenny one last review before merge

ChickenBenny · 2026-03-15T09:54:42Z

LGTM~

* fix: prioritize request ignore_result over task definition * fix: add comment to explain logic of get_actual_ignore_result * test: add missing tests for Request.ignore_result fallback This commit adds the missing unit tests to verify the priority logic for 'ignore_result' within the Request class, ensuring it correctly falls back to the task property when not specified in the message. * fix: pre-commit error * fix: add missing tests for handle_error_state while req is none * fix(worker): add test to ensure request.store_errors respects task.ignore_result * fix(docs): clarify fallback behavior for ignore_result and store_errors * fix(worker): handle ignore_result=None in request headers --------- Co-authored-by: Asif Saif Uddin {"Auvi":"অভি"} <[email protected]>

patri27826 marked this pull request as ready for review March 8, 2026 16:20

patri27826 marked this pull request as draft March 8, 2026 16:21

auvipy requested review from auvipy and Copilot March 8, 2026 17:10

Copilot started reviewing on behalf of auvipy March 8, 2026 17:11 View session

Copilot AI reviewed Mar 8, 2026

View reviewed changes

Comment thread t/unit/tasks/test_trace.py Outdated

Comment thread t/unit/tasks/test_trace.py Outdated

Comment thread celery/app/trace.py Outdated

patri27826 force-pushed the fix-apply-sync-ignore-result branch from ebd4c36 to 78ffc9f Compare March 8, 2026 17:16

patri27826 changed the title ~~Fix~~ Fix: prioritize request ignore_result over task definition Mar 8, 2026

patri27826 force-pushed the fix-apply-sync-ignore-result branch from 78ffc9f to 7a4f8f6 Compare March 8, 2026 17:31

patri27826 marked this pull request as ready for review March 8, 2026 17:31

auvipy requested a review from Copilot March 8, 2026 17:36

Copilot started reviewing on behalf of auvipy March 8, 2026 17:36 View session

Copilot AI reviewed Mar 8, 2026

View reviewed changes

Comment thread celery/worker/request.py Outdated

ChickenBenny reviewed Mar 9, 2026

View reviewed changes

Comment thread celery/app/trace.py

Comment thread celery/worker/request.py

Comment thread celery/app/trace.py Outdated

auvipy requested changes Mar 11, 2026

View reviewed changes

patri27826 added 3 commits March 13, 2026 23:16

fix: prioritize request ignore_result over task definition

14cd8e8

fix: add comment to explain logic of get_actual_ignore_result

64099a3

test: add missing tests for Request.ignore_result fallback

3e80328

This commit adds the missing unit tests to verify the priority logic for 'ignore_result' within the Request class, ensuring it correctly falls back to the task property when not specified in the message.

patri27826 force-pushed the fix-apply-sync-ignore-result branch from 4755766 to 3e80328 Compare March 13, 2026 16:59

fix: pre-commit error

7f63f61

patri27826 requested review from ChickenBenny and auvipy March 13, 2026 17:39

auvipy added this to the 5.7.0 milestone Mar 14, 2026

auvipy requested review from Copilot and removed request for ChickenBenny March 14, 2026 04:37

Copilot started reviewing on behalf of auvipy March 14, 2026 04:37 View session

Copilot AI reviewed Mar 14, 2026

View reviewed changes

auvipy mentioned this pull request Mar 14, 2026

Fix for apply_async(ignore_result=False) #9655

Closed

patri27826 and others added 2 commits March 14, 2026 22:32

fix: add missing tests for handle_error_state while req is none

5bb9ea5

Merge branch 'main' into fix-apply-sync-ignore-result

19bbc64

auvipy requested a review from Copilot March 14, 2026 15:42

Copilot started reviewing on behalf of auvipy March 14, 2026 15:43 View session

Copilot AI reviewed Mar 14, 2026

View reviewed changes

Comment thread t/unit/worker/test_request.py

patri27826 added 3 commits March 15, 2026 00:18

fix(worker): add test to ensure request.store_errors respects task.ig…

145dcca

…nore_result

fix(docs): clarify fallback behavior for ignore_result and store_errors

ee1ab34

fix(worker): handle ignore_result=None in request headers

6a2b14c

Merge branch 'main' into fix-apply-sync-ignore-result

d3fde16

auvipy approved these changes Mar 15, 2026

View reviewed changes

auvipy merged commit b8f8521 into celery:main Mar 15, 2026
16 of 17 checks passed

auvipy added the Component: Result Backend label Mar 15, 2026

Uh oh!

Uh oh!

Conversation

patri27826 commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Test with three condition

⚠️ Behavioral Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

auvipy commented Mar 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

codecov Bot commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

auvipy left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

auvipy commented Mar 14, 2026

Uh oh!

ChickenBenny commented Mar 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

patri27826 commented Mar 8, 2026 •

edited

Loading

codecov Bot commented Mar 8, 2026 •

edited

Loading