This is an automated email from the ASF dual-hosted git repository.
pierrejeambrun pushed a commit to branch v3-2-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/v3-2-test by this push:
new 09629a9d7f2 [v3-2-test] Guard finally-block logger.info in HTTP access
log middleware (#67501) (#67632)
09629a9d7f2 is described below
commit 09629a9d7f27dbed050c393b133898ed2f8e6201
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
