You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "dkranchii (via GitHub)" <gi...@apache.org> on 2026/05/25 17:48:36 UTC

[PR] UI: Return clear 500 detail from structure_data on malformed asset_expression [airflow]

dkranchii opened a new pull request, #67489:
URL: https://github.com/apache/airflow/pull/67489

   The `/structure/structure_data` endpoint calls `get_upstream_assets()` to walk the serialized Dag's `asset_expression`. If the stored expression contains an unknown key (anything other than `any` / `all`) or an unknown asset type (anything other than `asset` / `asset-alias` / `asset-name-ref` / `asset-uri-ref`), `get_upstream_assets()` raises `TypeError("Unsupported type: ...")` at `services/ui/structure.py:69` or `:102`.
   
   The exception escaped uncaught and FastAPI returned a generic `{"detail": "Internal Server Error"}` response body with no context about *which* Dag triggered it. Operators had to dig through server logs to identify the broken Dag — a frustrating debugging loop for a multi-Dag deployment.
   
   This wraps the call in `try/except TypeError` and re-raises as `HTTPException(500)` with a detail message identifying the Dag id and version. It remains a 500 because the underlying issue is genuinely server-side stored-data corruption (not bad client input — the request is valid). The improvement is the response body: it's now controlled, documented, and debuggable.
   
   ### Before
   ```
   GET /structure/structure_data?dag_id=foo&external_dependencies=True → 500 Internal Server Error 
   
   {"detail": "Internal Server Error"}
   ```
   
   ### After
   ```
   GET /structure/structure_data?dag_id=foo&external_dependencies=True → 500 Internal Server Error 
   
   {"detail": "Malformed asset_expression in Dag 'foo' version 3: Unsupported type: dict_keys(['weird-op'])"}
   ```
   
   
   ### Tests
   
   Adds `TestStructureDataEndpoint::test_should_return_500_on_malformed_asset_expression` which mocks `get_upstream_assets` to raise `TypeError` and asserts the response surfaces a 500 with a detail message that names the Dag and includes the wrapped error.
   
   ### Related
   
   - AGENTS.md rule added in #67221: *"Translate domain-layer exceptions to HTTPException at FastAPI route boundaries."* This PR conforms to that rule by replacing an uncaught domain exception with a controlled `HTTPException` response.
   - Sibling PR: #67363 addressed the same class of bug in `ui/dags.py`.
   
   ---
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes — (Cursor) used for review and verify.
   


-- 
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] UI: Return 400 instead of 500 from structure_data on malformed asset_expression [airflow]

Posted by "dkranchii (via GitHub)" <gi...@apache.org>.
dkranchii commented on code in PR #67489:
URL: https://github.com/apache/airflow/pull/67489#discussion_r3330724434


##########
uv.lock:
##########


Review Comment:
   Why ?



-- 
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] UI: Return clear 500 detail from structure_data on malformed asset_expression [airflow]

Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #67489:
URL: https://github.com/apache/airflow/pull/67489#discussion_r3301276286


##########
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/structure.py:
##########
@@ -161,9 +161,15 @@ def structure_data(
                 )
 
         if (asset_expression := serialized_dag.dag_model.asset_expression) and entry_node_ref:
-            upstream_asset_nodes, upstream_asset_edges = get_upstream_assets(
-                asset_expression, entry_node_ref["id"]
-            )
+            try:
+                upstream_asset_nodes, upstream_asset_edges = get_upstream_assets(
+                    asset_expression, entry_node_ref["id"]
+                )
+            except TypeError as e:
+                raise HTTPException(
+                    status.HTTP_500_INTERNAL_SERVER_ERROR,
+                    f"Malformed asset_expression in Dag {dag_id!r} version {version_number}: {e}",
+                ) from e

Review Comment:
   We should return 400 status code instead of 500 internal error.



-- 
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] UI: Return 400 instead of 500 from structure_data on malformed asset_expression [airflow]

Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #67489:
URL: https://github.com/apache/airflow/pull/67489#discussion_r3334322967


##########
uv.lock:
##########


Review Comment:
   I just removed it, lets see what the CI says.



-- 
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] UI: Return 400 instead of 500 from structure_data on malformed asset_expression [airflow]

Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #67489:
URL: https://github.com/apache/airflow/pull/67489#issuecomment-4577220506

   CI still needs fixing
   


-- 
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] UI: Return 400 instead of 500 from structure_data on malformed asset_expression [airflow]

Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun merged PR #67489:
URL: https://github.com/apache/airflow/pull/67489


-- 
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] UI: Return 400 instead of 500 from structure_data on malformed asset_expression [airflow]

Posted by "dkranchii (via GitHub)" <gi...@apache.org>.
dkranchii commented on code in PR #67489:
URL: https://github.com/apache/airflow/pull/67489#discussion_r3330724434


##########
uv.lock:
##########


Review Comment:
   Why



-- 
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] UI: Return 400 instead of 500 from structure_data on malformed asset_expression [airflow]

Posted by "choo121600 (via GitHub)" <gi...@apache.org>.
choo121600 commented on code in PR #67489:
URL: https://github.com/apache/airflow/pull/67489#discussion_r3328314841


##########
uv.lock:
##########


Review Comment:
   This change seems unrelated to the scope of this PR. Could you please revert it?



-- 
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] UI: Return 400 instead of 500 from structure_data on malformed asset_expression [airflow]

Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #67489:
URL: https://github.com/apache/airflow/pull/67489#discussion_r3334246574


##########
uv.lock:
##########


Review Comment:
   +1, we just need to remove this and we are good to go.



-- 
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] UI: Return clear 500 detail from structure_data on malformed asset_expression [airflow]

Posted by "dkranchii (via GitHub)" <gi...@apache.org>.
dkranchii commented on code in PR #67489:
URL: https://github.com/apache/airflow/pull/67489#discussion_r3305029617


##########
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/structure.py:
##########
@@ -161,9 +161,15 @@ def structure_data(
                 )
 
         if (asset_expression := serialized_dag.dag_model.asset_expression) and entry_node_ref:
-            upstream_asset_nodes, upstream_asset_edges = get_upstream_assets(
-                asset_expression, entry_node_ref["id"]
-            )
+            try:
+                upstream_asset_nodes, upstream_asset_edges = get_upstream_assets(
+                    asset_expression, entry_node_ref["id"]
+                )
+            except TypeError as e:
+                raise HTTPException(
+                    status.HTTP_500_INTERNAL_SERVER_ERROR,
+                    f"Malformed asset_expression in Dag {dag_id!r} version {version_number}: {e}",
+                ) from e

Review Comment:
   @jason810496  Updated the status code. Can you please review. thanks



-- 
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] UI: Return 400 instead of 500 from structure_data on malformed asset_expression [airflow]

Posted by "dkranchii (via GitHub)" <gi...@apache.org>.
dkranchii commented on PR #67489:
URL: https://github.com/apache/airflow/pull/67489#issuecomment-4581947938

   @pierrejeambrun pls review. 


-- 
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] UI: Return 400 instead of 500 from structure_data on malformed asset_expression [airflow]

Posted by "dkranchii (via GitHub)" <gi...@apache.org>.
dkranchii commented on code in PR #67489:
URL: https://github.com/apache/airflow/pull/67489#discussion_r3330724434


##########
uv.lock:
##########


Review Comment:
   Why ?



-- 
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] UI: Return 400 instead of 500 from structure_data on malformed asset_expression [airflow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #67489:
URL: https://github.com/apache/airflow/pull/67489#issuecomment-4593678258

   ### Backport successfully created: v3-2-test
   
   Note: As of [Merging PRs targeted for Airflow 3.X](https://github.com/apache/airflow/blob/main/dev/README_AIRFLOW3_DEV.md#merging-prs-targeted-for-airflow-3x)
   the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.
   
   In matter of doubt please ask in [#release-management](https://apache-airflow.slack.com/archives/C03G9H97MM2) Slack channel.
   
   <table>
           <tr>
           <th>Status</th>
           <th>Branch</th>
           <th>Result</th>
       </tr>
       <tr>
           <td>✅</td>
           <td>v3-2-test</td>
           <td><a href="https://github.com/apache/airflow/pull/67849"><img src="https://img.shields.io/badge/PR-67849-blue" alt="PR Link"></a></td>
       </tr>
   </table>


-- 
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] UI: Return 400 instead of 500 from structure_data on malformed asset_expression [airflow]

Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #67489:
URL: https://github.com/apache/airflow/pull/67489#issuecomment-4593659248

   Unrelated CI failure, merging.


-- 
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] UI: Return clear 500 detail from structure_data on malformed asset_expression [airflow]

Posted by "dkranchii (via GitHub)" <gi...@apache.org>.
dkranchii commented on code in PR #67489:
URL: https://github.com/apache/airflow/pull/67489#discussion_r3305029617


##########
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/structure.py:
##########
@@ -161,9 +161,15 @@ def structure_data(
                 )
 
         if (asset_expression := serialized_dag.dag_model.asset_expression) and entry_node_ref:
-            upstream_asset_nodes, upstream_asset_edges = get_upstream_assets(
-                asset_expression, entry_node_ref["id"]
-            )
+            try:
+                upstream_asset_nodes, upstream_asset_edges = get_upstream_assets(
+                    asset_expression, entry_node_ref["id"]
+                )
+            except TypeError as e:
+                raise HTTPException(
+                    status.HTTP_500_INTERNAL_SERVER_ERROR,
+                    f"Malformed asset_expression in Dag {dag_id!r} version {version_number}: {e}",
+                ) from e

Review Comment:
   Updated the status code



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