Skip to content

Honor AirflowFailException raised inside on_retry_callback - #66781

Closed
1fanwang wants to merge 2 commits into
apache:mainfrom
1fanwang:fix/airflow-fail-in-retry-callback
Closed

Honor AirflowFailException raised inside on_retry_callback#66781
1fanwang wants to merge 2 commits into
apache:mainfrom
1fanwang:fix/airflow-fail-in-retry-callback

Conversation

@1fanwang

@1fanwang 1fanwang commented May 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Retry callbacks can now stop a task by raising AirflowFailException. Previously Airflow logged the exception and retried anyway. Other callback exceptions still allow retries.

The worker reports the outcome after callbacks, so failure callbacks, listeners and email use the final state.

Closes #60172.

Testing

SQLite Dag test through the real task runner and Execution API; not a scheduler deployment.

Save as repro.py in callback-proof under the PR checkout:

Dag, commands and raw output 
from datetime import timedelta
from os import environ
from airflow.sdk import DAG, Context, task
from airflow.sdk.exceptions import AirflowFailException

def stop_retry(context: Context) -> None:
    if environ.get("GENERIC_CALLBACK"):
        raise RuntimeError("callback cleanup failed")
    raise AirflowFailException("give up")

with DAG(dag_id="retry_callback_proof", schedule=None) as dag:
    @task(retries=1, retry_delay=timedelta(seconds=0), on_retry_callback=stop_retry)
    def failing_task() -> None:
        raise RuntimeError("transient")
    failing_task()

if __name__ == "__main__":
    run = dag.test()
    instance = run.get_task_instance(task_id="failing_task")
    print(f"task_state={instance.state} try_number={instance.try_number}")

From the repository root:

ROOT="$PWD"
export AIRFLOW_HOME="$ROOT/callback-proof/airflow-home"
export AIRFLOW__DATABASE__SQL_ALCHEMY_CONN="sqlite:///$AIRFLOW_HOME/airflow.db"
export AIRFLOW__CORE__DAGS_FOLDER="$ROOT/callback-proof"
export AIRFLOW__CORE__LOAD_EXAMPLES=False
uv run --project task-sdk airflow db migrate
BASE=$(git merge-base upstream/main HEAD)
for ref in "$BASE" HEAD; do
  git checkout "$ref" -- task-sdk/src/airflow/sdk/execution_time/{task_runner,supervisor}.py
  uv run --project "$ROOT/task-sdk" --directory "$ROOT/callback-proof" python repro.py
done
GENERIC_CALLBACK=1 uv run --project "$ROOT/task-sdk" --directory "$ROOT/callback-proof" python repro.py

Before, after, then the ordinary-exception control:

task_state=failed try_number=2
task_state=failed try_number=1
task_state=failed try_number=2
Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

Generated-by: GitHub Copilot CLI (GPT-6 Astra) following the guidelines

SameerMesiah97
SameerMesiah97 reviewed May 12, 2026
View reviewed changes

@SameerMesiah97 SameerMesiah97 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approach looks fine to me but someone more familiar with this area should weigh in. I have left some comments.

Comment thread airflow-core/newsfragments/66781.bugfix.rst Resolved
Comment thread task-sdk/src/airflow/sdk/execution_time/task_runner.py Resolved
Comment thread task-sdk/src/airflow/sdk/execution_time/task_runner.py Resolved
Comment thread task-sdk/tests/task_sdk/execution_time/test_task_runner.py Resolved
Comment thread task-sdk/tests/task_sdk/execution_time/test_task_runner.py Resolved
@1fanwang
1fanwang force-pushed the fix/airflow-fail-in-retry-callback branch from f59182e to 8f538c8 Compare May 13, 2026 21:25
@potiuk potiuk added the ready for maintainer review Set by triage when all criteria pass, or by the fork CI reconciler when fork CI is green. label May 19, 2026
@1fanwang
1fanwang force-pushed the fix/airflow-fail-in-retry-callback branch from 8f538c8 to c47b237 Compare May 19, 2026 18:53
Nataneljpwd
Nataneljpwd approved these changes Jun 1, 2026
View reviewed changes

@Nataneljpwd Nataneljpwd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me!

Comment thread task-sdk/src/airflow/sdk/execution_time/task_runner.py Resolved
@1fanwang
1fanwang force-pushed the fix/airflow-fail-in-retry-callback branch 3 times, most recently from acfbaf0 to ce3ab59 Compare June 19, 2026 18:47
@potiuk potiuk removed the ready for maintainer review Set by triage when all criteria pass, or by the fork CI reconciler when fork CI is green. label Jul 15, 2026
@potiuk
potiuk marked this pull request as draft July 20, 2026 11:54
@potiuk

potiuk commented Jul 20, 2026

Copy link
Copy Markdown
Member

@1fanwang This PR has been converted to draft because it does not yet meet our Pull Request quality criteria.

Issues found:

  • ❌ Merge conflicts: This PR has merge conflicts with main and cannot be merged as-is. Rebase locally and resolve them: git fetch upstream main && git rebase upstream/main, resolve the conflicts, then force-push.

Note: Your branch is 649 commits behind main. Some check failures may be caused by changes in the base branch rather than by your PR. Please rebase your branch and push again to get up-to-date CI results.

What to do next:

  • Fix each issue listed above.
  • Make sure static checks pass locally (prek run --from-ref main --stage pre-commit).
  • Mark the PR as "Ready for review" when you're done.

Converting a PR to draft is not a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the Airflow Slack.

Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you.

@1fanwang
1fanwang force-pushed the fix/airflow-fail-in-retry-callback branch from ce3ab59 to 22345de Compare July 22, 2026 22:55
@1fanwang
1fanwang marked this pull request as ready for review July 22, 2026 22:56
@1fanwang
1fanwang force-pushed the fix/airflow-fail-in-retry-callback branch 3 times, most recently from 1f98c4f to f9d5dd7 Compare July 25, 2026 08:59
@potiuk potiuk added the ready for maintainer review Set by triage when all criteria pass, or by the fork CI reconciler when fork CI is green. label Jul 28, 2026
@1fanwang
Honor AirflowFailException raised inside on_retry_callback
18a5399 
Raising AirflowFailException is the documented way to fail a task without
retrying. Until now that signal was silently swallowed when raised from
on_retry_callback: the catch-all `except Exception` inside
`_run_task_state_change_callbacks` ate it, the task stayed UP_FOR_RETRY,
and another attempt was scheduled.

The retry path now defers sending its terminal message until after
on_retry_callback has run. If the callback raises AirflowFailException,
the state is promoted to FAILED, the pending RetryTask is replaced with
TaskState(FAILED), and the failure-path finalizers (on_failure_callback,
listener.on_task_instance_failed, email_on_failure) run as if the task
had failed without ever attempting a retry. Other exceptions from
on_retry_callback are still logged and swallowed, so callbacks that
optimistically clean up partial data continue to work unchanged.

closes: apache#60172
Signed-off-by: 1fanwang <[email protected]>
@1fanwang
Add spacing in AirflowFailException branch for readability
b61cc42 
Separate the log / state-promotion / message-rewrite / failure-finalizer
blocks in the on_retry_callback failure path with blank lines.

Signed-off-by: 1fanwang <[email protected]>
@1fanwang
1fanwang force-pushed the fix/airflow-fail-in-retry-callback branch from 381c313 to b61cc42 Compare August 13, 2026 05:33
@potiuk potiuk mentioned this pull request Sep 25, 2026
Closed
1 task done
@potiuk potiuk added the closed because of open PR limit Closed as a one-time step of introducing the open pull request limit label Sep 25, 2026
@potiuk

potiuk commented Sep 25, 2026

Copy link
Copy Markdown
Member

Hello @1fanwang - thank you for your contributions to Apache Airflow!

The Airflow community has introduced a limit of 5 open pull requests at a time for contributors without write access to the repository. You currently have 34 open pull requests, so - as a one-time step of introducing the limit - we closed the ones where maintainers have not engaged yet:

These pull requests stay open because maintainers are already engaged in them - they count towards your limit:

This is not a judgement of you or of your changes. We never told contributors before that opening many pull requests at once was a problem, so there is nothing to feel bad about - and nothing is lost: your branches, commits and the review history stay where they are.

What we ask you to do is to make your first prioritization decision: choose which of the pull requests above matter most to you, and reopen them (up to 5 open at a time, including the ones still open) with the "Reopen pull request" button or gh pr reopen <PR_NUMBER> --repo apache/airflow. Reopen the ones you are ready to follow through - keep them rebased, respond to review comments and fix failing checks.

While your pull requests are waiting for review, the most valuable thing you can do is help in other ways - reviewing other contributors' pull requests, helping with issues, and taking part in the discussions on the devlist and Slack.

Why we introduced the limit, what it means for you and how to reopen or restore a pull request is explained in https://github.com/apache/airflow/blob/main/contributing-docs/32_open_pull_request_limit.rst.

Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting

@potiuk potiuk closed this Sep 25, 2026
@potiuk potiuk mentioned this pull request Sep 25, 2026
Closed
1 task done
This was referenced Sep 25, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@luc-pimentel luc-pimentel mentioned this pull request Sep 28, 2026
Closed
1 task done
@deepujain deepujain mentioned this pull request Oct 27, 2026
Open
1 task done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ashb ashb Awaiting requested review from ashb ashb is a code owner
@kaxil kaxil Awaiting requested review from kaxil
@amoghrajesh amoghrajesh Awaiting requested review from amoghrajesh amoghrajesh is a code owner
2 more reviewers
@SameerMesiah97 SameerMesiah97 SameerMesiah97 left review comments
@Nataneljpwd Nataneljpwd Nataneljpwd approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@1fanwang 1fanwang

Labels

area:task-sdk closed because of open PR limit Closed as a one-time step of introducing the open pull request limit ready for maintainer review Set by triage when all criteria pass, or by the fork CI reconciler when fork CI is green.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Fix handling AirflowFailException on_retry_callback

4 participants

@1fanwang @potiuk @SameerMesiah97 @Nataneljpwd