From 15403e120f4731cdf2f7efdc45aae2bd03148ed8 Mon Sep 17 00:00:00 2001 From: Deepak kumar Date: Mon, 1 Jun 2026 07:43:02 -0700 Subject: [PATCH] [v3-2-test] UI: Return 400 instead of 500 from structure_data on malformed asset_expression (#67489) * UI: Return clear 500 detail from structure_data when asset_expression is malformed 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 or asset type, get_upstream_assets() raises TypeError("Unsupported type: ..."). The exception escaped uncaught and FastAPI returned a generic {"detail": "Internal Server Error"} body with no context about which Dag triggered it, forcing operators to dig through server logs to identify the broken Dag. Wrap the call in try/except TypeError and re-raise as HTTPException(500) with a detail message identifying the Dag id and version. Still a 500 (the underlying data corruption is genuinely server-side, not bad client input), but now with a controlled, debuggable response body. Regression test mocks get_upstream_assets to raise TypeError and asserts the response is 500 with a detail message that includes the Dag id. * Use 400 BAD_REQUEST for malformed asset_expression per review feedback Per @jason810496 review feedback on #67489: the malformed asset_expression ultimately originates from user-authored Dag code (via the Task SDK), so the appropriate response is 400 BAD_REQUEST rather than 500 INTERNAL_SERVER_ERROR. - Change status code from 500 to 400 in structure_data. - Add HTTP_400_BAD_REQUEST to create_openapi_http_exception_doc so the OpenAPI spec advertises the new error response. - Update regression test to assert 400 and rename accordingly. Detail message is unchanged per reviewer: "It's fine to add more context". * Revert uv.lock diff --------- (cherry picked from commit eccbdb1b082be73b870e0a354d980ce2f542cc46) Co-authored-by: Deepak kumar Co-authored-by: pierrejeambrun --- .../core_api/openapi/_private_ui.yaml | 6 +++++ .../core_api/routes/ui/structure.py | 19 +++++++++++--- .../ui/openapi-gen/requests/services.gen.ts | 1 + .../ui/openapi-gen/requests/types.gen.ts | 4 +++ .../core_api/routes/ui/test_structure.py | 25 +++++++++++++++++++ 5 files changed, 51 insertions(+), 4 deletions(-) diff --git a/airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml b/airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml index 81fd117e917dc..63ea1bc04d004 100644 --- a/airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml +++ b/airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml @@ -788,6 +788,12 @@ paths: application/json: schema: $ref: '#/components/schemas/StructureDataResponse' + '400': + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPExceptionResponse' + description: Bad Request '404': content: application/json: diff --git a/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/structure.py b/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/structure.py index 87184788413f3..597f44db424bb 100644 --- a/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/structure.py +++ b/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/structure.py @@ -42,7 +42,12 @@ @structure_router.get( "/structure_data", - responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]), + responses=create_openapi_http_exception_doc( + [ + status.HTTP_400_BAD_REQUEST, + status.HTTP_404_NOT_FOUND, + ] + ), dependencies=[ Depends(requires_access_dag("GET")), Depends(requires_access_dag("GET", DagAccessEntity.DEPENDENCIES)), @@ -161,9 +166,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_400_BAD_REQUEST, + f"Malformed asset_expression in Dag {dag_id!r} version {version_number}: {e}", + ) from e data["nodes"] += upstream_asset_nodes data["edges"] += upstream_asset_edges diff --git a/airflow-core/src/airflow/ui/openapi-gen/requests/services.gen.ts b/airflow-core/src/airflow/ui/openapi-gen/requests/services.gen.ts index 673f20a1d9855..f372133e77724 100644 --- a/airflow-core/src/airflow/ui/openapi-gen/requests/services.gen.ts +++ b/airflow-core/src/airflow/ui/openapi-gen/requests/services.gen.ts @@ -4256,6 +4256,7 @@ export class StructureService { version_number: data.versionNumber }, errors: { + 400: 'Bad Request', 404: 'Not Found', 422: 'Validation Error' } diff --git a/airflow-core/src/airflow/ui/openapi-gen/requests/types.gen.ts b/airflow-core/src/airflow/ui/openapi-gen/requests/types.gen.ts index 3062baff3a8a3..cdd4d73211b44 100644 --- a/airflow-core/src/airflow/ui/openapi-gen/requests/types.gen.ts +++ b/airflow-core/src/airflow/ui/openapi-gen/requests/types.gen.ts @@ -7254,6 +7254,10 @@ export type $OpenApiTs = { * Successful Response */ 200: StructureDataResponse; + /** + * Bad Request + */ + 400: HTTPExceptionResponse; /** * Not Found */ diff --git a/airflow-core/tests/unit/api_fastapi/core_api/routes/ui/test_structure.py b/airflow-core/tests/unit/api_fastapi/core_api/routes/ui/test_structure.py index db436ca3cf93a..8e504c132c4eb 100644 --- a/airflow-core/tests/unit/api_fastapi/core_api/routes/ui/test_structure.py +++ b/airflow-core/tests/unit/api_fastapi/core_api/routes/ui/test_structure.py @@ -709,6 +709,31 @@ def test_should_return_404(self, test_client): assert response.status_code == 404 assert response.json()["detail"] == "Dag with id not_existing was not found" + @pytest.mark.usefixtures("make_dags") + def test_should_return_400_on_malformed_asset_expression(self, test_client): + """A TypeError from get_upstream_assets surfaces as a 400 with a clear message. + + The asset_expression ultimately comes from user-authored Dag code (via the Task SDK), + so a malformed expression is bad input that ended up persisted -- not a server fault. + Without the try/except wrap, the TypeError propagates uncaught and FastAPI returns a + generic ``{"detail": "Internal Server Error"}`` 500 body with no context about which + Dag triggered it. With the wrap, the response identifies the Dag and version, which + is what a caller needs to fix the upstream Dag definition. + """ + with mock.patch( + "airflow.api_fastapi.core_api.routes.ui.structure.get_upstream_assets", + side_effect=TypeError("Unsupported type: dict_keys(['weird-op'])"), + ): + response = test_client.get( + "/structure/structure_data", + params={"dag_id": DAG_ID, "external_dependencies": True}, + ) + assert response.status_code == 400 + detail = response.json()["detail"] + assert "Malformed asset_expression" in detail + assert DAG_ID in detail + assert "Unsupported type" in detail + def test_should_return_404_when_dag_version_not_found(self, test_client): response = test_client.get( "/structure/structure_data", params={"dag_id": DAG_ID, "version_number": 999}