Skip to content

[airflow-ctl/v0-1-test] Fix API client errors losing detail behind a reverse proxy (#75493) - #75622

Merged
potiuk merged 1 commit into
airflow-ctl/v0-1-testfrom
backport-352ca60-airflow-ctl/v0-1-test
Oct 25, 2026
Merged

potiuk merged 1 commit into
airflow-ctl/v0-1-testfrom
backport-352ca60-airflow-ctl/v0-1-test

Conversation

@github-actions

Copy link
Copy Markdown
Contributor
  • 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.


(cherry picked from commit 352ca60)

Co-authored-by: rjgoyln 151457491+rjgoyln@users.noreply.github.com
Co-authored-by: Henry Chen henrychen@apache.org

…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.

---------
(cherry picked from commit 352ca60)

Co-authored-by: rjgoyln <151457491+rjgoyln@users.noreply.github.com>
Co-authored-by: Henry Chen <henrychen@apache.org>
@potiuk
potiuk marked this pull request as ready for review October 25, 2026 22:06
@potiuk
potiuk enabled auto-merge (squash) October 25, 2026 22:07
@potiuk
potiuk merged commit eb64662 into airflow-ctl/v0-1-test Oct 25, 2026
8 checks passed
@potiuk
potiuk deleted the backport-352ca60-airflow-ctl/v0-1-test branch October 25, 2026 22:08
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.

2 participants