This is an automated email from the ASF dual-hosted git repository. vatsrahul1001 pushed a commit to branch v3-2-test in repository https://gitbox.apache.org/repos/asf/airflow.git
commit d133fb00a8b540ffa30a604f9379d8d5120202a8 Author: Jason(Zhe-You) Liu <[email protected]> AuthorDate: Thu May 28 21:18:37 2026 +0800 [v3-2-test] Guard finally-block logger.info in HTTP access log middleware (#67501) (#67632) The ``finally`` block in ``HttpAccessLogMiddleware`` called ``logger.info()`` without exception protection. If ``logger.info()`` raised — broken handler, OOM in the formatter, downstream forwarder unavailable — and the original ``try`` block was already propagating an application exception, Python's ``finally``-replacement semantics would discard the original exception in favour of the logger's, so uvicorn never saw the real failure. Wrap the emit in ``contextlib.suppress(Exception)`` so logging failures never disrupt the application or mask the original exception. The HTTP response has already been sent to the client by the time we reach the log emit, so swallowing the logger failure costs nothing beyond a missing log line for that one request. (cherry picked from commit b66f4433e004fad81b66023bd8caccee55e6e4f7) Co-authored-by: Jarek Potiuk <[email protected]> --- .../airflow/api_fastapi/common/http_access_log.py | 24 ++++++---- .../api_fastapi/common/test_http_access_log.py | 53 ++++++++++++++++++++++ 2 files changed, 68 insertions(+), 9 deletions(-) diff --git a/airflow-core/src/airflow/api_fastapi/common/http_access_log.py b/airflow-core/src/airflow/api_fastapi/common/http_access_log.py index 581c3593790..3a298c9c21f 100644 --- a/airflow-core/src/airflow/api_fastapi/common/http_access_log.py +++ b/airflow-core/src/airflow/api_fastapi/common/http_access_log.py @@ -95,12 +95,18 @@ class HttpAccessLogMiddleware: client = scope.get("client") client_addr = f"{client[0]}:{client[1]}" if client else None - logger.info( - "request finished", - method=method, - path=path, - query=query, - status_code=status, - duration_us=duration_us, - client_addr=client_addr, - ) + # Guard the log emit: if it raised inside a ``finally`` while the + # original ``try`` block was already propagating an app exception, + # Python's exception-replacement semantics would discard the + # original. Swallow logging failures so the application exception + # always reaches uvicorn intact. + with contextlib.suppress(Exception): + logger.info( + "request finished", + method=method, + path=path, + query=query, + status_code=status, + duration_us=duration_us, + client_addr=client_addr, + ) diff --git a/airflow-core/tests/unit/api_fastapi/common/test_http_access_log.py b/airflow-core/tests/unit/api_fastapi/common/test_http_access_log.py index 5e7e9e6e760..5542510aa4a 100644 --- a/airflow-core/tests/unit/api_fastapi/common/test_http_access_log.py +++ b/airflow-core/tests/unit/api_fastapi/common/test_http_access_log.py @@ -16,6 +16,7 @@ # under the License. from __future__ import annotations +import pytest import structlog import structlog.testing from starlette.applications import Starlette @@ -130,3 +131,55 @@ def test_non_http_scope_not_logged(): def test_health_paths_constant(): assert "/api/v2/monitor/health" in _HEALTH_PATHS + + +def test_logger_failure_does_not_mask_app_exception(monkeypatch): + """ + If ``logger.info`` raises while the app already raised, the original app exception must + still propagate (rather than being replaced by the logger's exception). + """ + import airflow.api_fastapi.common.http_access_log as mod + + def broken_info(*_args, **_kwargs): + raise RuntimeError("logger broken") + + monkeypatch.setattr(mod.logger, "info", broken_info) + + import asyncio + + async def raising_app(scope, receive, send): + # Send response.start so the middleware's response variable is populated, then raise. + await send({"type": "http.response.start", "status": 503, "headers": []}) + raise RuntimeError("app exception") + + middleware = HttpAccessLogMiddleware(raising_app) + scope = { + "type": "http", + "method": "GET", + "path": "/boom", + "query_string": b"", + "headers": [], + "client": ("test", 1), + } + + async def receive(): + return {"type": "http.request", "body": b""} + + async def send(_message): + return None + + with pytest.raises(RuntimeError, match="app exception"): + asyncio.run(middleware(scope, receive, send)) + + +def test_logger_failure_swallowed_on_clean_request(monkeypatch): + """No app exception + a broken logger must not break the request.""" + import airflow.api_fastapi.common.http_access_log as mod + + monkeypatch.setattr( + mod.logger, "info", lambda *_a, **_kw: (_ for _ in ()).throw(RuntimeError("logger broken")) + ) + + client = TestClient(_make_app(), raise_server_exceptions=False) + response = client.get("/") + assert response.status_code == 200
