Skip to content

Fix Task SDK silently ignoring AirflowRuntimeError when Variable.set() or Variable.delete() fails#68542

Merged
ashb merged 3 commits into
apache:mainfrom
jayachandrakasarla:fix-68537-task-sdk-ignoring-exceptions
Jun 18, 2026
Merged

Fix Task SDK silently ignoring AirflowRuntimeError when Variable.set() or Variable.delete() fails#68542
ashb merged 3 commits into
apache:mainfrom
jayachandrakasarla:fix-68537-task-sdk-ignoring-exceptions

Conversation

@jayachandrakasarla

@jayachandrakasarla jayachandrakasarla commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Closes #68537

As per the issue, Variable.set() and Variable.delete() in the Task SDK were silently ignoring AirflowRuntimeError, logging the exception but not re-raising, causing tasks to report success even when a variable write was rejected by the Execution API.

Added raise to both except blocks so errors now propagate and fail the task, consistent with the existing behavior of Variable.get().

Removed try-except blocks for set() and delete() methods as they are logging the error message twice.

Also added the unit test:

  • test_var_delete - basic success-path coverage for Variable.delete(), which was previously untested.

Here's a DAG for testing:

import datetime
from airflow.sdk import dag, task

@dag(
    dag_id="test_variable_error_propagation",
    start_date=datetime.datetime(2026, 1, 1),
    schedule=None,
    catchup=False,
)
def test_variable_error_propagation():
    @task
    def test_variable_set_and_delete_success():
        from airflow.sdk import Variable

        Variable.set("abc", "efg")
        Variable.delete("abc")
        print("Variable.delete succeeded")

    t1 = test_variable_set_and_delete_success()

    t1


test_variable_error_propagation()

Use the following middleware plugin with breeze to test 403 API errors

from __future__ import annotations

import json

from airflow.plugins_manager import AirflowPlugin


class RejectVariableWritesMiddleware:
    """Intercept PUT/DELETE /execution/variables/* and return 403."""

    def __init__(self, app) -> None:
        self.app = app

    async def __call__(self, scope, receive, send) -> None:
        if scope["type"] == "http":
            path: str = scope.get("path", "")
            method: str = scope.get("method", "")
            if method in ("PUT", "DELETE") and path.startswith("/execution/variables/"):
                body = json.dumps({"detail": "Variable writes rejected by test middleware"}).encode()
                await send(
                    {
                        "type": "http.response.start",
                        "status": 403,
                        "headers": [
                            [b"content-type", b"application/json"],
                            [b"content-length", str(len(body)).encode()],
                        ],
                    }
                )
                await send({"type": "http.response.body", "body": body})
                return
        await self.app(scope, receive, send)


class RejectVariableWritesPlugin(AirflowPlugin):
    name = "reject_variable_writes"
    fastapi_root_middlewares = [
        {
            "name": "RejectVariableWritesMiddleware",
            "middleware": RejectVariableWritesMiddleware,
        }
    ]

Was generative AI tooling used to co-author this PR?

[X] Yes

Used Claude to generate the middleware plugin to test 403 API errors.

@jayachandrakasarla
jayachandrakasarla force-pushed the fix-68537-task-sdk-ignoring-exceptions branch from 746769a to e5390fd Compare June 15, 2026 02:25
@jayachandrakasarla

Copy link
Copy Markdown
Contributor Author

@ashb, @kaxil, @amoghrajesh could you please review this? Thanks!

@ashb

ashb commented Jun 17, 2026

Copy link
Copy Markdown
Member

Please show what the task logs look like with this change.

I suspect this results in the error being logged twice, and instead were shots remove the try/except block entirely

@jayachandrakasarla

jayachandrakasarla commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

@ashb This is how it looks on main branch when using the ASGI middleware plugin to reject writes and deletes, error is thrown but the task is in success state:

image

And this is how it looks like after raising the exception when encountering errors with set and delete:

image

I can see the error being logged twice and this is how it looks after removing try-except blocks from set() and delete() methods.

image

Do you want me to push the changes by removing try-except blocks for set() and delete() methods?

@ashb

ashb commented Jun 17, 2026

Copy link
Copy Markdown
Member

Do you want me to push the changes by removing try-except blocks for set() and delete() methods?

I think its nicer. Do you?

@jayachandrakasarla

Copy link
Copy Markdown
Contributor Author

Do you want me to push the changes by removing try-except blocks for set() and delete() methods?

I think its nicer. Do you?

Yes. I've pushed the changes.

@potiuk potiuk added the ready for maintainer review Set after triaging when all criteria pass. label Jun 17, 2026
@ashb
ashb merged commit c59f4cd into apache:main Jun 18, 2026
102 checks passed
@jayachandrakasarla
jayachandrakasarla deleted the fix-68537-task-sdk-ignoring-exceptions branch June 18, 2026 13:19
@vatsrahul1001 vatsrahul1001 added this to the Airflow 3.3.0 milestone Jun 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:task-sdk ready for maintainer review Set after triaging when all criteria pass.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Task SDK: Variable.set/delete swallow API errors — a rejected write does not fail the task

4 participants