Skip to content

Fire failure listeners when TriggerDagRunOperator API call fails - #70506

Open
ABiscuitttt wants to merge 3 commits into
apache:mainfrom
ABiscuitttt:fix-trigger-dagrun-failure-listeners
Open

Fire failure listeners when TriggerDagRunOperator API call fails#70506
ABiscuitttt wants to merge 3 commits into
apache:mainfrom
ABiscuitttt:fix-trigger-dagrun-failure-listeners

Conversation

@ABiscuitttt

@ABiscuitttt ABiscuitttt commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

When TriggerDagRunOperator targets a nonexistent Dag (or the API server otherwise rejects the trigger with 403/5xx), the supervisor reports the error by raising AirflowRuntimeError from the TriggerDagRun comms round-trip. That raise happens inside run()'s except DagRunTriggerException handler, and an exception raised in an except handler is not caught by the sibling except clauses — it escaped run() entirely. main() then never reached finalize(), the only place on_task_instance_failed / on_failure_callback listeners run. The task still showed FAILED in the UI (via the supervisor's exit-code fallback), but alerting plugins were silently skipped for exactly this failure mode.

The fix routes the error through the normal failure path (_handle_current_task_failed), so the task returns a terminal state, finalize() fires the listeners, and retry policy is honored (UP_FOR_RETRY when retries remain).

Relationship to prior work on #63089: the reported UnboundLocalError symptom was already fixed by #67955 (the state variable is now initialized and the ti.finish stats emission is guarded), and #63207 was an earlier, unmerged attempt at the full fix. What remains — and what this PR addresses — is the API error escaping run() entirely, which silently skips failure listeners, failure callbacks, and retry handling.

Regression tests cover both the no-retry (FAILED) and retry (UP_FOR_RETRY) paths, asserting the terminal state reaches the supervisor instead of the error escaping run().

closes: #63089


Was generative AI tooling used to co-author this PR?
  • Yes — Kimi Code CLI

Generated-by: Kimi Code CLI following the guidelines

@ABiscuitttt

ABiscuitttt commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Some context on how we ran into this: we run a global failure-alert listener plugin that pages our on-call channel (Feishu/Lark) through the on_task_instance_failed hook when a task instance reaches its final FAILED state:

@hookimpl
def on_task_instance_failed(previous_state, task_instance, error):
    if task_instance.state != TaskInstanceState.FAILED:
        return  # still up_for_retry; only alert on final failure
    send_alert(task_instance, error)  # internal notification

When a TriggerDagRunOperator points at a non-existent Dag, the task dies from the API error without this hook ever firing, so no alert goes out — from the on-call perspective the task fails silently. That is what led to this fix: routing the API error through the normal failure path makes finalize() fire the listener (and honor the retry policy) as expected.

When TriggerDagRunOperator targets a nonexistent DAG (or the API server
otherwise rejects the trigger with 403/5xx), the supervisor reports the
error by raising AirflowRuntimeError from the TriggerDagRun comms
round-trip. That raise happens inside run()'s `except
DagRunTriggerException` handler, and an exception raised in an except
handler is not caught by the sibling except clauses, so it escaped run()
entirely. main() then never reached finalize() — the only place
on_task_instance_failed and on_failure_callback listeners run — so the
task showed FAILED in the UI (via the supervisor's exit-code fallback)
while alerting plugins and failure callbacks were silently skipped.

Route the error through the normal failure path instead, so the task
returns a terminal state, finalize() fires the listeners, and retry
policy is honored.

closes: apache#63089
Airflow prose convention is "Dag", and the test module already imports
TriggerDagRunOperator at the top level.
@ABiscuitttt
ABiscuitttt force-pushed the fix-trigger-dagrun-failure-listeners branch from 6ea96f7 to 102c4be Compare July 28, 2026 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UnboundLocalError on TriggerDagRunOperator triggering a dag that doesn't exist

1 participant