You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "rjgoyln (via GitHub)" <gi...@apache.org> on 2026/08/02 05:05:18 UTC
[PR] Fix airflowctl tracebacks on JSON errors behind a reverse proxy [airflow]
rjgoyln opened a new pull request, #70936:
URL: https://github.com/apache/airflow/pull/70936
`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 built 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 (Starlette only appends one for
`text/*`), 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 is now parsed instead of compared whole:
| `content-type` | before | after |
|---|---|---|
| `application/json` | ✅ friendly | ✅ friendly |
| `application/json; charset=utf-8` | ❌ traceback | ✅ friendly |
| `application/json;charset=UTF-8` | ❌ traceback | ✅ friendly |
| `Application/JSON` | ❌ traceback | ✅ friendly |
| `application/problem+json` | ❌ traceback | ✅ friendly |
| `application/jsonp` | `raise_for_status()` | `raise_for_status()` |
Matching more responses also routes two further crash paths into the error
handler, both fixed here:
- a JSON body that is valid but **not an object** — `dict()` raised `ValueError`
from the same unhandled path
- a body that does not decode at all — now left to `raise_for_status()`
### Tests
11 of the added tests fail without the source change. They reuse the existing
`make_unread_json_response` helper (extended with a `content_type` parameter) so
they exercise the unread/streaming body path rather than an eagerly-loaded one.
No newsfragment: `airflow-ctl` release managers regenerate the changelog from
`git log`.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude Code (Opus 5)
Generated-by: Claude Code (Opus 5) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix airflowctl tracebacks on JSON errors behind a reverse proxy [airflow]
Posted by "rjgoyln (via GitHub)" <gi...@apache.org>.
rjgoyln commented on code in PR #70936:
URL: https://github.com/apache/airflow/pull/70936#discussion_r3703443177
##########
airflow-ctl/src/airflowctl/api/operations.py:
##########
@@ -103,6 +103,20 @@ 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.
Review Comment:
OK, I got it. Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix API client errors losing detail behind a reverse proxy [airflow]
Posted by "rjgoyln (via GitHub)" <gi...@apache.org>.
rjgoyln commented on code in PR #70936:
URL: https://github.com/apache/airflow/pull/70936#discussion_r4073187371
##########
task-sdk/src/airflow/sdk/api/client.py:
##########
@@ -1305,6 +1305,14 @@ def dags(self) -> DagsOperations:
return DagsOperations(self)
+def _has_json_body(response: httpx.Response) -> bool:
+ """Check whether a response declares a JSON body."""
+ # Media types are case-insensitive and may carry parameters (RFC 9110), so a proxy that
Review Comment:
Thanks for the review.
Rebased onto current `main` and pushed, so CI is running against today’s tree.
Added the newsfragment at `airflow-core/newsfragments/70936.bugfix.rst`.
On the nit, I added `# Kept in sync with <the other one>` to each copy rather than merging the comments. The `operations.py` comment explains the consequence of silently skipping the `ServerResponseError` path, so I felt it was worth keeping there; the cross-reference points to the twin.
The `extra=` check was a good catch. I had verified that the `isinstance` guard covered the body shape, but hadn’t established that the key collision is currently harmless under the logger. Thanks for pointing that out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix airflowctl tracebacks on JSON errors behind a reverse proxy [airflow]
Posted by "SameerMesiah97 (via GitHub)" <gi...@apache.org>.
SameerMesiah97 commented on code in PR #70936:
URL: https://github.com/apache/airflow/pull/70936#discussion_r3700257374
##########
airflow-ctl/src/airflowctl/api/operations.py:
##########
@@ -103,6 +103,20 @@ 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.
Review Comment:
Keep this line in the dosctring.
The remaining part of the docstring should be in a comment above the line where you are parsing the JSON.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix API client errors losing detail behind a reverse proxy [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #70936:
URL: https://github.com/apache/airflow/pull/70936#issuecomment-5839163927
Hello @rjgoyln - 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 24 open pull requests, so - as a one-time step of introducing the limit - we closed the ones where maintainers have not engaged yet:
* #70758 - Support OpenTelemetry auto-instrumentation and vendor distros
* #70840 - Honor OTEL_PROPAGATORS when propagating trace context across Airflow
* #70936 - Fix API client errors losing detail behind a reverse proxy
* #70940 - Fix airflowctl config lint filters matching partial config names
* #71194 - Stop failing reschedule-mode sensors on a stale executor success
* #71868 - Drop the cncf.kubernetes provider dependency from airflow-core tests
* #72041 - Drop the git provider dependency from airflow-core tests
* #72053 - Fail deferred sensors on non-timeout errors when soft_fail is set
* #72249 - Drop the celery provider dependency from airflow-core tests
* #72301 - Honor config_file in deferrable Kubernetes tasks
* #72591 - Fix Dataproc job not cancelled when killing a deferred task
* #72595 - Stop the scheduler deadlocking with the triggerer on deferred-task timeouts
* #72596 - Make the scheduler's HITL and orphan sweeps retry transient DB errors
* #72758 - Keep the triggerer running when a trigger constructor raises
* #72814 - Stop failing scheduled tasks when a serialized Dag is briefly missing
* #73022 - Stop fetching the run conf once per task instance in the scheduler
* #73076 - Catch format-string arguments passed to exception constructors
* #73131 - Fail deferrable KPO when a GC'd pod takes its XCom with it
* #73166 - Block new EM102 f-string exception messages in prek
* #73170 - Fail the local yml mounts prek hook when synchronization fails
* #73204 - Fix provider.yaml doc URL check with several suspended providers
These pull requests stay open because maintainers are already engaged in them - they count towards your limit:
* #70620 - Batch upstream map-length lookups when resolving mapped kwargs
* #71850 - Re-anchor custom queued deadline references when a Dag run is cleared
* #73221 - Make the default task failure email say what failed
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix API client errors losing detail behind a reverse proxy [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk closed pull request #70936: Fix API client errors losing detail behind a reverse proxy
URL: https://github.com/apache/airflow/pull/70936
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix API client errors losing detail behind a reverse proxy [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #70936:
URL: https://github.com/apache/airflow/pull/70936#discussion_r4063923939
##########
task-sdk/src/airflow/sdk/api/client.py:
##########
@@ -1305,6 +1305,14 @@ def dags(self) -> DagsOperations:
return DagsOperations(self)
+def _has_json_body(response: httpx.Response) -> bool:
+ """Check whether a response declares a JSON body."""
+ # Media types are case-insensitive and may carry parameters (RFC 9110), so a proxy that
Review Comment:
Nit, and I'm deliberately *not* asking you to extract this into `shared/` — `airflow-ctl` doesn't consume any shared distribution today, so wiring one up for three lines would cost more than it saves.
But the twin in `airflow-ctl/src/airflowctl/api/operations.py:106` is the same function with a different comment: that one explains the consequence (the friendly `ServerResponseError` handling gets silently skipped), this one stops at the media-type point. The copies have diverged in the PR that created them, which is usually where drift starts.
A cross-reference in each — *"kept in sync with `airflowctl.api.operations._has_json_body`"* and vice versa — would be enough to make the next person notice there are two.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org