This is an automated email from the ASF dual-hosted git repository.
henry3260 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 352ca600361 Fix API client errors losing detail behind a reverse proxy
(#75493)
352ca600361 is described below
commit 352ca600361fea10561f225d063b23355421a36b
Author: rjgoyln <[email protected]>
AuthorDate: Mon Oct 26 03:32:45 2026 +0800
Fix API client errors losing detail behind a reverse proxy (#75493)
* Fix airflowctl tracebacks on JSON errors behind a reverse proxy
airflowctl compared the error response content-type against the exact
string "application/json". Any reverse proxy, ingress, or API gateway
that returns "application/json; charset=utf-8" — or emits its own
"application/problem+json" error — fell outside that comparison, so
ServerResponseError was never constructed and the friendly "not found"
messages in the dags and tasks commands were skipped. The bare
httpx.HTTPStatusError raised instead is not handled by
safe_call_command, so users saw a raw traceback. Airflow's own API
server does not send a charset, which is why this only surfaces once
something sits in front of it.
Media types are case-insensitive and may carry parameters (RFC 9110),
so the header has to be parsed rather than compared whole.
Matching more responses also routes bodies into the error path that are
valid JSON but not objects, which dict() rejected with a ValueError from
that same unprotected path, and bodies that do not decode at all.
* Move media type rationale from docstring into an inline comment
* Apply the same JSON media type handling to the Task SDK client
The Task SDK client carries the identical exact-string content-type
comparison, so a task talking to the API server through a proxy that
declares a charset loses the server's error detail and, because the
raised exception is then a plain httpx.HTTPStatusError, no longer hits
the status-specific handling in the supervisor and the task runner.
* Cross-reference the twin JSON media type checks
The airflowctl and Task SDK copies are the same three lines with
different comments, and they already drifted in the commits that
created them. A pointer in each makes the next person notice there
are two.
* Add newsfragment for the API client error handling fix
Requested during review: the Task SDK half of the change ships in
airflow-core, and both effects are visible to users running behind a
proxy.
* Use httpx2 in the Task SDK JSON media type check
The Task SDK moved from httpx to httpx2 (#73116) after this branch was
written, so the rebased helper and its tests referred to a module that
is no longer imported there.
* Describe the airflowctl symptom as it is on main now
Since #73307 airflowctl catches the bare HTTPStatusError and prints one
line instead of a traceback, so the newsfragment overstated it; the
server's own message is still what gets lost.
* Keep only the airflowctl test cases the fix changes
The bare media type and the 404/500 axis repeat what
test_error_parsing_with_unread_body already covers, and dict() of an
object body worked before; none of them fail without the fix.
* Explain the non-object log guard by what it protects
The previous comment described the dict() call the guard replaced,
which is no longer in the code.
* Limit the newsfragment to the Task SDK fix
airflow-core's release notes ship only the Task SDK side; airflowctl's
changelog is built from git log. The task runner never handled
ServerResponseError, so it is no longer named.
* Rename the newsfragment to this PR's number
* Shorten the media type comments
The longer versions restated what the diff and the PR already explain,
and the two _has_json_body copies now carry the same note.
---------
Co-authored-by: Henry Chen <[email protected]>
---
airflow-core/newsfragments/75493.bugfix.rst | 1 +
airflow-ctl/src/airflowctl/api/client.py | 4 +-
airflow-ctl/src/airflowctl/api/operations.py | 18 ++++++-
airflow-ctl/tests/airflow_ctl/api/test_client.py | 63 +++++++++++++++++++++++-
task-sdk/src/airflow/sdk/api/client.py | 10 +++-
task-sdk/tests/task_sdk/api/test_client.py | 29 +++++++++++
6 files changed, 119 insertions(+), 6 deletions(-)
diff --git a/airflow-core/newsfragments/75493.bugfix.rst
b/airflow-core/newsfragments/75493.bugfix.rst
new file mode 100644
index 00000000000..ec1de259838
--- /dev/null
+++ b/airflow-core/newsfragments/75493.bugfix.rst
@@ -0,0 +1 @@
+Fix the Task SDK client raising a bare ``HTTPStatusError`` instead of
``ServerResponseError`` when a reverse proxy rewrites the ``content-type`` of a
JSON error response, for example to ``application/json; charset=utf-8``. The
supervisor's and the client's status-specific handling, such as treating a 404
as a missing connection or variable, now applies to those responses.
diff --git a/airflow-ctl/src/airflowctl/api/client.py
b/airflow-ctl/src/airflowctl/api/client.py
index 1236ccec6b7..da8e364a786 100644
--- a/airflow-ctl/src/airflowctl/api/client.py
+++ b/airflow-ctl/src/airflowctl/api/client.py
@@ -113,7 +113,9 @@ def get_json_error(response: httpx.Response):
# This part is used in integration tests to verify the error message
# If you are updating here don't forget to update the airflow-ctl-tests
if not
response.request.extensions.get("airflowctl_suppress_error_log"):
- log.warning("Server error ", extra=dict(err.response.json()))
+ detail = err.response.json()
+ # ``extra`` needs a mapping, but a JSON error body can be any JSON
value.
+ log.warning("Server error ", extra=detail if isinstance(detail,
dict) else {"detail": detail})
raise err
diff --git a/airflow-ctl/src/airflowctl/api/operations.py
b/airflow-ctl/src/airflowctl/api/operations.py
index 5cc6e1f4eae..a28048d5a05 100644
--- a/airflow-ctl/src/airflowctl/api/operations.py
+++ b/airflow-ctl/src/airflowctl/api/operations.py
@@ -106,6 +106,14 @@ def _build_query_params(**values: Any) -> dict[str, Any]:
return {name: _serialize_query_param(value) for name, value in
values.items() if value is not None}
+def _has_json_body(response: httpx.Response) -> bool:
+ """Check whether a response declares a JSON body."""
+ # Kept in sync with airflow.sdk.api.client._has_json_body.
+ # Media types are case-insensitive and may carry parameters (RFC 9110).
+ media_type = response.headers.get("content-type",
"").partition(";")[0].strip().lower()
+ return media_type == "application/json" or media_type.endswith("+json")
+
+
# Generic Server Response Error
class ServerResponseError(httpx.HTTPStatusError):
"""Server response error (Generic)."""
@@ -115,16 +123,22 @@ class ServerResponseError(httpx.HTTPStatusError):
if response.status_code < 400:
return None
- if response.headers.get("content-type") != "application/json":
+ if not _has_json_body(response):
return None
# httpx runs response event hooks before it reads the body, so the
body has to be
# pulled in explicitly here or ``.json()`` raises
``httpx.ResponseNotRead``.
response.read()
+ try:
+ detail = response.json()
+ except ValueError:
+ # A JSON content-type does not guarantee a JSON body; leave it to
raise_for_status().
+ return None
+
error_kind = "Client" if response.status_code < 500 else "Server"
return cls(
- message=f"{error_kind} error message: {response.json()}",
+ message=f"{error_kind} error message: {detail}",
request=response.request,
response=response,
)
diff --git a/airflow-ctl/tests/airflow_ctl/api/test_client.py
b/airflow-ctl/tests/airflow_ctl/api/test_client.py
index 4edd4958833..dadf5d80367 100644
--- a/airflow-ctl/tests/airflow_ctl/api/test_client.py
+++ b/airflow-ctl/tests/airflow_ctl/api/test_client.py
@@ -23,6 +23,7 @@ import shutil
import subprocess
import sys
import tempfile
+from typing import Any
from unittest.mock import MagicMock, patch
import httpx
@@ -57,7 +58,9 @@ def make_client_w_responses(responses: list[httpx.Response])
-> Client:
return Client(base_url="", token="", mounts={"'http://":
httpx.MockTransport(handle_request)})
-def make_unread_json_response(status_code: int, payload: dict, **kwargs) ->
httpx.Response:
+def make_unread_json_response(
+ status_code: int, payload: Any, content_type: str = "application/json",
**kwargs
+) -> httpx.Response:
"""
Build a JSON response whose body has not been read yet.
@@ -67,7 +70,7 @@ def make_unread_json_response(status_code: int, payload:
dict, **kwargs) -> http
"""
return httpx.Response(
status_code,
- headers={"content-type": "application/json"},
+ headers={"content-type": content_type},
content=iter([json.dumps(payload).encode()]),
**kwargs,
)
@@ -144,6 +147,62 @@ class TestClient:
assert err.value.args == (expected_message,)
+ @pytest.mark.parametrize(
+ "content_type",
+ [
+ pytest.param("application/json; charset=utf-8", id="charset"),
+ pytest.param("application/json;charset=UTF-8",
id="charset-no-space"),
+ pytest.param("Application/JSON", id="mixed-case"),
+ pytest.param("application/problem+json", id="problem-json"),
+ ],
+ )
+ def test_error_parsing_json_media_type_variants(self, content_type):
+ response = make_unread_json_response(
+ 404,
+ {"detail": "boom"},
+ content_type=content_type,
+ request=httpx.Request("GET", "http://error";),
+ )
+
+ with pytest.raises(ServerResponseError):
+ get_json_error(response)
+
+ def test_error_parsing_media_type_merely_prefixed_with_json(self):
+ response = make_unread_json_response(
+ 404,
+ {"detail": "boom"},
+ content_type="application/jsonp",
+ request=httpx.Request("GET", "http://error";),
+ )
+
+ assert get_json_error(response) is None
+
+ def test_error_parsing_undecodable_json_body(self):
+ response = httpx.Response(
+ 404,
+ headers={"content-type": "application/json"},
+ content=iter([b"{truncated"]),
+ request=httpx.Request("GET", "http://error";),
+ )
+
+ assert get_json_error(response) is None
+
+ @pytest.mark.parametrize(
+ ("payload", "expected_extra"),
+ [
+ pytest.param(["a", "b"], {"detail": ["a", "b"]}, id="array"),
+ pytest.param("plain message", {"detail": "plain message"},
id="string"),
+ ],
+ )
+ @patch("airflowctl.api.client.log.warning", autospec=True)
+ def test_error_log_accepts_non_object_json_body(self, mock_warning,
payload, expected_extra):
+ response = make_unread_json_response(404, payload,
request=httpx.Request("GET", "http://error";))
+
+ with pytest.raises(ServerResponseError):
+ get_json_error(response)
+
+ assert mock_warning.call_args.kwargs["extra"] == expected_extra
+
@pytest.mark.parametrize(
("suppress_error_log", "expected_warning_count"),
[
diff --git a/task-sdk/src/airflow/sdk/api/client.py
b/task-sdk/src/airflow/sdk/api/client.py
index cc67a2697ab..bdf5ba34bff 100644
--- a/task-sdk/src/airflow/sdk/api/client.py
+++ b/task-sdk/src/airflow/sdk/api/client.py
@@ -1419,6 +1419,14 @@ class Client(httpx2.Client):
return DagsOperations(self)
+def _has_json_body(response: httpx2.Response) -> bool:
+ """Check whether a response declares a JSON body."""
+ # Kept in sync with airflowctl.api.operations._has_json_body.
+ # Media types are case-insensitive and may carry parameters (RFC 9110).
+ media_type = response.headers.get("content-type",
"").partition(";")[0].strip().lower()
+ return media_type == "application/json" or media_type.endswith("+json")
+
+
# This is only used for parsing. ServerResponseError is raised instead
class _ErrorBody(BaseModel):
detail: list[RemoteValidationError] | dict[str, Any] | str
@@ -1446,7 +1454,7 @@ class ServerResponseError(httpx2.HTTPStatusError):
if not (400 <= response.status_code < 600):
return None
- if response.headers.get("content-type") != "application/json":
+ if not _has_json_body(response):
return None
detail: list[RemoteValidationError] | dict[str, Any] | None = None
diff --git a/task-sdk/tests/task_sdk/api/test_client.py
b/task-sdk/tests/task_sdk/api/test_client.py
index c92653386cf..9b23b65c153 100644
--- a/task-sdk/tests/task_sdk/api/test_client.py
+++ b/task-sdk/tests/task_sdk/api/test_client.py
@@ -200,6 +200,35 @@ class TestClient:
assert err.value.args == ("Not found",)
assert err.value.detail is None
+ @pytest.mark.parametrize(
+ "content_type",
+ [
+ pytest.param("application/json; charset=utf-8", id="charset"),
+ pytest.param("application/json;charset=UTF-8",
id="charset-no-space"),
+ pytest.param("Application/JSON", id="mixed-case"),
+ pytest.param("application/problem+json", id="problem-json"),
+ ],
+ )
+ def test_error_parsing_json_media_type_variants(self, content_type):
+ responses = [
+ httpx2.Response(404, json={"detail": "Not found"},
headers={"content-type": content_type})
+ ]
+ client = make_client_w_responses(responses)
+
+ with pytest.raises(ServerResponseError) as err:
+ client.get("http://error";)
+ assert err.value.args == ("Not found",)
+
+ def test_error_parsing_media_type_merely_prefixed_with_json(self):
+ responses = [
+ httpx2.Response(404, json={"detail": "Not found"},
headers={"content-type": "application/jsonp"})
+ ]
+ client = make_client_w_responses(responses)
+
+ with pytest.raises(httpx2.HTTPStatusError) as err:
+ client.get("http://error";)
+ assert not isinstance(err.value, ServerResponseError)
+
def test_server_response_error_pickling(self):
responses = [httpx2.Response(404, json={"detail": {"message": "Invalid
input"}})]
client = make_client_w_responses(responses)
