Skip to content

Commit 382b27b

Browse files
swissmoclaude
andauthored
fix: demote disconnect-caused stateless session crash logs (#2257)
* fix: demote disconnect-caused stateless session crash logs Every HTTP entry point runs Streamable HTTP in stateless mode. When a tool call outlives the client's patience, the mcp SDK's session manager already catches the resulting anyio.ClosedResourceError (the client is gone, the response can't be delivered) but logs it as an alarming ERROR-level traceback under "Stateless session crashed" -- an expected protocol race, not a server bug. Add SessionDisconnectLogFilter, following the existing StatelessSessionLogFilter/ToolValidationLogFilter pattern, to demote only this specific known-benign case to a one-line WARNING; any other exception on that logger keeps its full ERROR traceback. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: handle nested ExceptionGroup and install filters on every HTTP launcher Addresses two Codex review findings on the disconnect-log-demotion fix: 1. mcp.server.lowlevel.server.Server.run() (and fastmcp's own wrapping run()) dispatch message handling through nested anyio task groups, so a ClosedResourceError raised while responding is always delivered as an ExceptionGroup (observed doubly-nested in production), never as a bare exception. SessionDisconnectLogFilter's isinstance check missed this entirely. Add _is_only_closed_resource_errors() to recursively unwrap (possibly nested) ExceptionGroups and demote only when every leaf is a ClosedResourceError; a mixed group (a real bug alongside the disconnect race) stays at ERROR with its traceback intact. 2. The three log-noise filters were only wired through ha_mcp.__main__._setup_logging(), which only the CLI entry points call. Both the Home Assistant app (homeassistant-addon/start.py) and the in-process embedded server (custom_components/ha_mcp_tools/ embedded_server.py) build their own HTTP app directly and never got any of the three filters. Extract them into a new side-effect-free ha_mcp.log_filters module (importing ha_mcp.__main__ directly runs process-global side effects that must never happen in-process, same reasoning as ha_mcp.browser_landing) with a shared install_sdk_log_filters() every launcher now calls -- ImportError- guarded in the embedded server the same way as register_browser_landing, since the component and server package version independently. Regression tests reproduce the real task-group boundary (a live anyio.create_task_group() child raising ClosedResourceError) rather than a hand-built exc_info, per the review's own critique, plus a mixed-group case and hermetic _serve() coverage for both the install and its backward-compat ImportError fallback. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * style: apply ruff format to the review-comment fix Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: diagnose silent filter-install failures, dedup filters, address CI findings - embedded_server.py's ImportError guard around the log-filter install was too broad: it silently swallowed ANY import failure, not just "older ha-mcp missing the module" (the intended case). Since third-party dependencies (fastmcp, pydantic) are deliberately NOT reinstalled per config entry (_purge_ha_mcp_modules only purges ha_mcp.* modules), a version mismatch there could break the import invisibly -- no crash, no log, the filters just silently never install. Narrow the tolerated case to ModuleNotFoundError for exactly "ha_mcp.log_filters", and log a WARNING for anything else (including a failure from install_sdk_log_filters() itself), so this class of failure is never invisible again. - install_sdk_log_filters() had no dedup guard: the in-process embedded server calls it on every _serve() reload without a process restart, so filters accumulated one more redundant instance per reload (CodeRabbit). Add _add_filter_once() and a regression test that calls the installer twice and asserts exactly one filter of each type. - Narrow a test helper's `except BaseException` to `except Exception` (CodeQL py/catch-base-exception): the child coroutines it runs only ever raise Exception subclasses. - Add docstrings to functions/methods touched by this change that lacked one (log_filters.py's three filter() methods, new/modified test methods and fixtures in test_session_disconnect_log_filter.py and test_embedded_server.py). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: make filter dedup survive module reloads, not just repeat calls CodeRabbit follow-up: _add_filter_once's isinstance() check only dedups filters within a single loaded generation of ha_mcp.log_filters. The in-process embedded server purges ha_mcp.* from sys.modules and re-imports fresh on every reinstall (_purge_ha_mcp_modules), so filter_cls is a BRAND NEW class object each reload -- isinstance() against a stale-generation instance never matches, so filters accumulated one more stale instance per reload indefinitely (the actual real-world case for this server, not the same-generation repeat-call case the previous fix covered). Match by (module, qualname) instead, which is stable across reloads of the same module path, and actively replace any stale-generation match rather than just skipping when one is found -- a logger always carries exactly one, current-generation filter of each type. Regression test simulates a stale-generation instance directly (a dynamically-built class sharing the real one's module/qualname) rather than via importlib.reload(): reloading the real module mutates shared global state and was observed to break unrelated sibling test files that import these classes at their own module scope -- caught by running the full affected test set with --maxfail=0 before this commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * debug: add temporary diagnostics for the live filter-not-firing report A SHA-pinned override (ruling out any git/uv caching explanation) still shows the unfiltered ERROR traceback in the live embedded server, and StatelessSessionLogFilter (unrelated, pre-existing-pattern logic) isn't suppressing "Terminating session: None" there either -- so something is preventing install_sdk_log_filters() from taking effect in that specific deployment, for a reason not yet identified from reading the code or reproducing locally. Two temporary WARNING-level log lines (visible without changing log level): - install_sdk_log_filters() logs each target logger's filter count right after installing, so we don't have to wait for a real disconnect race to see whether the filters actually attached. - SessionDisconnectLogFilter.filter() logs the exact type/module/repr of whatever it sees in exc_info, and how it classified it, whenever it's invoked on a matching record -- proving whether the filter runs at all, and if so, exactly what it's failing to recognize. Marked TEMPORARY; remove once the live mismatch is diagnosed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * debug: add entry-point diagnostics to _install_log_filters_if_available The server-package-side diagnostics (log_filters.py, commit 7423224) confirmed install_sdk_log_filters() itself never logs anything -- but that leaves open whether _install_log_filters_if_available() is even being entered, or is silently returning via its "older ha-mcp" ModuleNotFoundError branch despite a full custom_components/ha_mcp_tools/ directory replacement (ruling out the single-file cross-module ModuleNotFoundError seen earlier). Add an unconditional entry log using embedded_server's own _LOGGER (already proven visible throughout this deployment across many other log lines), and stop silencing the "older ha-mcp" ModuleNotFoundError branch so its exact err.name/message is visible too, rather than assuming which case it hit. Marked TEMPORARY; remove alongside the other temporary diagnostics once the live mismatch is diagnosed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * debug: remove temporary diagnostics -- fix confirmed working live Live-validated end-to-end against the real production scenario after finding the actual root cause of the whole investigation: the HACS- delivered custom_components/ha_mcp_tools/ was stale/incompatible with this branch (missing dependency_diagnostics.py entirely -- a ModuleNotFoundError on setup), completely unrelated to anything in this PR's actual code. The "Developer: ha-mcp package override" only controls the ha_mcp server package; HACS manages the custom component separately and was never pointed at this branch, so install_sdk_log_filters() was never once reached in any earlier test regardless of what was pushed. Once the full custom_components/ha_mcp_tools/ directory was manually synced to this branch and Home Assistant was fully restarted, the live log confirmed everything working exactly as designed: SessionDisconnectLogFilter saw exc_info on mcp.server.streamable_http_manager: type='ExceptionGroup' module='builtins' repr=ExceptionGroup('unhandled errors in a TaskGroup', [ExceptionGroup('unhandled errors in a TaskGroup', [ClosedResourceError()])]) classified=True Stateless session crashed: client disconnected before response delivery The doubly-nested ExceptionGroup(ExceptionGroup(ClosedResourceError())) shape -- exactly what the Codex review comment predicted and what the task-group-boundary regression tests reproduce -- is correctly recognized and demoted to a clean WARNING with no traceback. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: recognize BrokenResourceError, close silent-branch test gap Addresses two well-researched review findings, both verified directly against the pinned mcp==1.28.1/anyio==4.10.0 in this repo's venv before acting on them: 1. _is_only_closed_resource_errors (renamed _is_only_disconnect_teardown_errors) only recognized anyio.ClosedResourceError, missing its sibling anyio.BrokenResourceError -- confirmed a plain Exception subclass, not a subclass of ClosedResourceError. Per the reviewer's anyio-internals walkthrough, which one surfaces for the exact same disconnect race depends on which end of the memory stream closes first (our end closing raises Closed; the peer's streamable_http terminate() closing first can wake a parked sender into raising Broken instead). This repo already treats the two as an equivalent pair for streamable-HTTP memory-stream teardown elsewhere (tests/src/haos_runtime.py's transient-error tuple) -- now the classifier does too. Not a regression either way: a BrokenResourceError-only leaf simply fails closed (stays at ERROR) without this, same as before the PR existed. Also independently confirmed the reviewer's mcp-version finding while verifying this: at the exact pinned mcp==1.28.1, Server._handle_request already catches (BrokenResourceError, ClosedResourceError) around respond() and drops it at DEBUG (mcp/server/lowlevel/server.py:805-815) -- absent at older mcp versions. The live-validated production capture this PR's tests describe as "the shape actually logged in production" is accurate for whichever mcp version that deployment has (older, given third-party deps aren't reinstalled per ha-mcp reload), not a version-independent guarantee -- worth keeping in mind if this classifier ever needs revisiting again as mcp gets upgraded. 2. No test constructed a genuine ModuleNotFoundError for exactly ha_mcp.log_filters to prove the "older ha-mcp" branch in _install_log_filters_if_available() actually stays silent -- the existing test only exercised the module-present-but-attribute-missing ImportError case, and asserted on survival, not on log output. Dropping the err.name check would have left the suite green while every older install gained a startup warning. Added TestInstallLogFiltersIfAvailable: calls the function directly (not through the full _serve()/_thread_main harness) with caplog, covering all three branches -- genuinely missing module (silent), attribute missing (warns), and a different missing dependency inside the import (warns, naming it) -- using the same __path__ = [] sys.modules technique _stub_ha_mcp_surface already established for this class of problem. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: make the genuinely-missing-module test deterministic CodeRabbit review finding, verified against this repo's own pre-existing documented knowledge before acting: test_genuinely_missing_module_is_silent faked the parent ha_mcp package with an empty __path__ and omitted ha_mcp.log_filters from sys.modules -- but _stub_ha_mcp_surface's own docstring (unrelated pre-existing code, written before this test existed) already documents that this exact technique is unreliable in this repo: "the editable install (uv sync) adds a meta-path finder that resolves ha_mcp.* by name and would re-import the REAL module even though the parent ha_mcp is faked with an empty __path__ (live-found in CI)". If that finder wins the race, the test passes vacuously: no exception is raised at all (the real module imports fine), so the silent branch is never actually exercised -- caplog.records == [] would be true for the wrong reason. Switched to patching builtins.__import__ to deterministically raise ModuleNotFoundError(name="ha_mcp.log_filters") for that exact name, matching the same technique test_different_missing_dependency_warns_and_names_it already uses for the sibling case just below it. Bypasses the meta-path finder entirely since the patched __import__ intercepts before any real import machinery runs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent cbb17ee commit 382b27b

10 files changed

Lines changed: 918 additions & 85 deletions

custom_components/ha_mcp_tools/embedded_server.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,49 @@ def _worker_startup_failure(exc: BaseException) -> EmbeddedServerError:
244244
return failure
245245

246246

247+
def _install_log_filters_if_available() -> None:
248+
"""Attach the shared MCP SDK/fastmcp log-noise filters, if this ha-mcp has them.
249+
250+
Mirrors the ``register_browser_landing`` guard just above ``_serve``'s call
251+
site: the installed server version is user-controlled (channel choice,
252+
pip-spec override), so an older ha-mcp without ``ha_mcp.log_filters`` must
253+
keep serving -- the filters are simply absent there, as they are today.
254+
255+
Only a ``ModuleNotFoundError`` for exactly ``ha_mcp.log_filters`` is that
256+
"older ha-mcp" case and is swallowed silently. Anything else -- a version
257+
mismatch in a third-party dependency this import touches (fastmcp,
258+
pydantic), which ``_purge_ha_mcp_modules`` deliberately does NOT
259+
reinstall per config entry, so a stale one left over from a previous
260+
install can break this import -- is logged instead of silently doing
261+
nothing: log cosmetics must never block startup, but they also must
262+
never fail invisibly.
263+
"""
264+
try:
265+
from ha_mcp.log_filters import install_sdk_log_filters
266+
except ModuleNotFoundError as err:
267+
if err.name != "ha_mcp.log_filters":
268+
_LOGGER.warning(
269+
"Could not install MCP SDK log-noise filters (missing "
270+
"dependency %s); continuing without them: %s",
271+
err.name,
272+
err,
273+
)
274+
return
275+
except ImportError as err:
276+
_LOGGER.warning(
277+
"Could not install MCP SDK log-noise filters; continuing without them: %s",
278+
err,
279+
)
280+
return
281+
try:
282+
install_sdk_log_filters()
283+
except Exception as err:
284+
_LOGGER.warning(
285+
"Could not install MCP SDK log-noise filters; continuing without them: %s",
286+
err,
287+
)
288+
289+
247290
class EmbeddedServerManager:
248291
"""Manage the lifecycle of the in-process ha-mcp server for one config entry."""
249292

@@ -1669,6 +1712,12 @@ async def _serve(self, access_token: str, stop_event: asyncio.Event) -> None:
16691712
else:
16701713
register_browser_landing(server.mcp, self._secret_path)
16711714

1715+
# Parity with the CLI HTTP runner: demote the MCP SDK/fastmcp log
1716+
# noise (routine stateless teardown, benign tool-validation
1717+
# tracebacks, disconnect-caused "session crashed" tracebacks) that
1718+
# every other HTTP launcher already filters.
1719+
_install_log_filters_if_available()
1720+
16721721
# Own the uvicorn server instead of calling mcp.run_async(): cancelling
16731722
# run_async's task does NOT release the listening socket in-process
16741723
# (live-found: the next bring-up failed with EADDRINUSE and uvicorn's

homeassistant-addon/start.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -914,13 +914,13 @@ def main() -> int:
914914
# Import and register browser landing before server start
915915
log_info("Importing ha_mcp module...")
916916
from ha_mcp.__main__ import (
917-
StatelessSessionLogFilter,
918917
_get_server,
919918
_get_timestamped_uvicorn_log_config,
920919
_log_startup_version,
921920
mcp,
922921
register_browser_landing,
923922
)
923+
from ha_mcp.log_filters import install_sdk_log_filters
924924
from ha_mcp.settings_ui import register_settings_routes
925925

926926
# Importing ha_mcp pulled in fastmcp, which attached its rich log
@@ -966,9 +966,7 @@ def main() -> int:
966966
register_settings_routes(
967967
server_instance.mcp, server_instance, secret_path=secret_path
968968
)
969-
logging.getLogger("mcp.server.streamable_http").addFilter(
970-
StatelessSessionLogFilter()
971-
)
969+
install_sdk_log_filters()
972970

973971
# fastmcp's DNS-rebinding guard is defaulted off in ha_mcp's _create_server
974972
# (reached above via _get_server() / the `mcp` proxy, before the app is

src/ha_mcp/__main__.py

Lines changed: 2 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,13 @@
3333
from collections.abc import Coroutine # noqa: E402
3434
from typing import TYPE_CHECKING, Any, NoReturn # noqa: E402
3535

36-
from fastmcp.exceptions import ToolError # noqa: E402
37-
from pydantic import ValidationError as PydanticValidationError # noqa: E402
38-
3936
from ha_mcp.browser_landing import ( # noqa: E402
4037
register_browser_landing as _register_landing_route,
4138
)
4239
from ha_mcp.browser_landing import ( # noqa: E402
4340
register_healthz as _register_healthz_route,
4441
)
42+
from ha_mcp.log_filters import install_sdk_log_filters # noqa: E402
4543

4644
if TYPE_CHECKING:
4745
from fastmcp import FastMCP
@@ -384,68 +382,6 @@ def __getattr__(self, name: str) -> Any:
384382
_LOG_DATE_FORMAT = "%Y-%m-%d %H:%M:%S"
385383

386384

387-
class StatelessSessionLogFilter(logging.Filter):
388-
"""Suppress the routine 'Terminating session: None' log from the MCP SDK.
389-
390-
In stateless HTTP mode every request creates and tears down a temporary
391-
session whose id is ``None``, so the SDK emits an INFO
392-
``Terminating session: None`` (mcp/server/streamable_http.py) on *every*
393-
request. The line is routine but looks alarming and has repeatedly
394-
confused users into thinking the connection is broken.
395-
396-
Returning ``False`` drops the record at this logger before it reaches any
397-
handler. (Merely downgrading the level to DEBUG did not work: the level
398-
gate is applied before the filter runs, so the record was already admitted
399-
and still emitted -- just relabelled.) Real session terminations carry an
400-
actual id and are not matched, so they still log.
401-
402-
# TODO: remove when modelcontextprotocol/python-sdk#2329 is resolved
403-
"""
404-
405-
def filter(self, record: logging.LogRecord) -> bool:
406-
if record.name != "mcp.server.streamable_http":
407-
return True
408-
try:
409-
message = record.getMessage()
410-
except (ValueError, TypeError):
411-
# A malformed %-format record on this logger is not our target, and
412-
# a filter must not raise: filters run in Logger.handle() with no
413-
# exception handling, so a raise would crash the logging call.
414-
return True
415-
# Drop the stateless teardown noise; keep everything else.
416-
return "Terminating session: None" not in message
417-
418-
419-
class ToolValidationLogFilter(logging.Filter):
420-
"""Demote fastmcp tool-failure tracebacks to single-line warnings.
421-
422-
Pydantic ValidationError and tool-raised ToolError aren't server bugs,
423-
so the traceback through fastmcp/pydantic internals is just noise. The
424-
structured error detail is preserved in the WARNING message; stack is
425-
intentionally dropped because these are user-input errors, not bugs.
426-
"""
427-
428-
def filter(self, record: logging.LogRecord) -> bool:
429-
if record.name != "fastmcp.server.server" or not record.exc_info:
430-
return True
431-
432-
msg = record.getMessage()
433-
err = record.exc_info[1]
434-
if "Error validating tool" in msg and isinstance(err, PydanticValidationError):
435-
record.msg = f"{msg}: {err.errors(include_url=False)}"
436-
elif "Error calling tool" in msg and isinstance(err, ToolError):
437-
record.msg = f"{msg}: {err}"
438-
else:
439-
return True
440-
441-
record.args = ()
442-
record.levelno = logging.WARNING
443-
record.levelname = "WARNING"
444-
record.exc_info = None
445-
record.exc_text = None
446-
return True
447-
448-
449385
class ProbeAccessLogFilter(logging.Filter):
450386
"""Drop benign, non-MCP HTTP probe noise from the uvicorn access log.
451387
@@ -528,10 +464,7 @@ def _setup_logging(log_level_str: str, force: bool = True) -> None:
528464
# disabled. Prevent that NOTSET namespace from inheriting our root handler.
529465
fastmcp_logger.setLevel(logging.CRITICAL + 1)
530466

531-
logging.getLogger("mcp.server.streamable_http").addFilter(
532-
StatelessSessionLogFilter()
533-
)
534-
logging.getLogger("fastmcp.server.server").addFilter(ToolValidationLogFilter())
467+
install_sdk_log_filters()
535468

536469

537470
def _log_startup_version() -> None:

src/ha_mcp/log_filters.py

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
"""MCP SDK / fastmcp log-noise filters shared across every HTTP launcher.
2+
3+
Extracted from :mod:`ha_mcp.__main__` for the same reason as
4+
:mod:`ha_mcp.browser_landing`: the in-process server (the ``ha_mcp_tools``
5+
custom-component worker thread) must never import ``ha_mcp.__main__``, since
6+
that module runs process-global side effects at import time
7+
(``truststore.inject_into_ssl()``, signal handlers, ``asyncio.run``). These
8+
filters only call ``logging.Logger.addFilter`` on specific named loggers --
9+
no ``basicConfig``, no handler or root-logger changes -- so
10+
``install_sdk_log_filters()`` is safe to call from any launcher, including
11+
one that must leave Home Assistant's own logging configuration untouched
12+
(see the ``log_config=None`` comment in ``embedded_server.py``).
13+
"""
14+
15+
from __future__ import annotations
16+
17+
import logging
18+
19+
import anyio
20+
from fastmcp.exceptions import ToolError
21+
from pydantic import ValidationError as PydanticValidationError
22+
23+
24+
class StatelessSessionLogFilter(logging.Filter):
25+
"""Suppress the routine 'Terminating session: None' log from the MCP SDK.
26+
27+
In stateless HTTP mode every request creates and tears down a temporary
28+
session whose id is ``None``, so the SDK emits an INFO
29+
``Terminating session: None`` (mcp/server/streamable_http.py) on *every*
30+
request. The line is routine but looks alarming and has repeatedly
31+
confused users into thinking the connection is broken.
32+
33+
Returning ``False`` drops the record at this logger before it reaches any
34+
handler. (Merely downgrading the level to DEBUG did not work: the level
35+
gate is applied before the filter runs, so the record was already admitted
36+
and still emitted -- just relabelled.) Real session terminations carry an
37+
actual id and are not matched, so they still log.
38+
39+
# TODO: remove when modelcontextprotocol/python-sdk#2329 is resolved
40+
"""
41+
42+
def filter(self, record: logging.LogRecord) -> bool:
43+
"""Drop the routine stateless-teardown record; pass everything else."""
44+
if record.name != "mcp.server.streamable_http":
45+
return True
46+
try:
47+
message = record.getMessage()
48+
except (ValueError, TypeError):
49+
# A malformed %-format record on this logger is not our target, and
50+
# a filter must not raise: filters run in Logger.handle() with no
51+
# exception handling, so a raise would crash the logging call.
52+
return True
53+
# Drop the stateless teardown noise; keep everything else.
54+
return "Terminating session: None" not in message
55+
56+
57+
class ToolValidationLogFilter(logging.Filter):
58+
"""Demote fastmcp tool-failure tracebacks to single-line warnings.
59+
60+
Pydantic ValidationError and tool-raised ToolError aren't server bugs,
61+
so the traceback through fastmcp/pydantic internals is just noise. The
62+
structured error detail is preserved in the WARNING message; stack is
63+
intentionally dropped because these are user-input errors, not bugs.
64+
"""
65+
66+
def filter(self, record: logging.LogRecord) -> bool:
67+
"""Demote a known-benign validation/tool-error record to WARNING."""
68+
if record.name != "fastmcp.server.server" or not record.exc_info:
69+
return True
70+
71+
msg = record.getMessage()
72+
err = record.exc_info[1]
73+
if "Error validating tool" in msg and isinstance(err, PydanticValidationError):
74+
record.msg = f"{msg}: {err.errors(include_url=False)}"
75+
elif "Error calling tool" in msg and isinstance(err, ToolError):
76+
record.msg = f"{msg}: {err}"
77+
else:
78+
return True
79+
80+
record.args = ()
81+
record.levelno = logging.WARNING
82+
record.levelname = "WARNING"
83+
record.exc_info = None
84+
record.exc_text = None
85+
return True
86+
87+
88+
_DISCONNECT_TEARDOWN_ERRORS = (anyio.ClosedResourceError, anyio.BrokenResourceError)
89+
90+
91+
def _is_only_disconnect_teardown_errors(err: BaseException) -> bool:
92+
"""True if ``err`` is (or an ExceptionGroup wrapping only) a benign
93+
streamable-HTTP memory-stream teardown error.
94+
95+
``mcp.server.lowlevel.server.Server.run()`` dispatches each incoming
96+
message via ``anyio.create_task_group().start_soon(...)``, so an error
97+
raised while delivering one message's response is raised from a
98+
task-group child -- anyio always wraps that in an ``ExceptionGroup``,
99+
even for a single failure. The exception a "session crashed" log record
100+
carries is therefore ``ExceptionGroup(...[<teardown error>])``, not a
101+
bare one.
102+
103+
Both ``ClosedResourceError`` and ``BrokenResourceError`` -- sibling
104+
``Exception`` subclasses, neither a subclass of the other -- can surface
105+
here for the same disconnect race: which one depends on which end
106+
closed first (our end closing raises ``Closed``; the peer's
107+
``streamable_http`` ``terminate()`` closing first can wake a parked
108+
sender into raising ``Broken`` instead, even though the situation is
109+
identical from the caller's perspective). This repo already treats the
110+
two as an equivalent pair for streamable-HTTP memory-stream teardown
111+
elsewhere -- see ``tests/src/haos_runtime.py``'s transient-error tuple.
112+
113+
Recurse through (possibly nested) groups; any other leaf type means
114+
this is not the known-benign disconnect race, so the caller should
115+
leave the record alone.
116+
"""
117+
if isinstance(err, _DISCONNECT_TEARDOWN_ERRORS):
118+
return True
119+
if isinstance(err, BaseExceptionGroup):
120+
return bool(err.exceptions) and all(
121+
_is_only_disconnect_teardown_errors(sub) for sub in err.exceptions
122+
)
123+
return False
124+
125+
126+
class SessionDisconnectLogFilter(logging.Filter):
127+
"""Demote 'session crashed' tracebacks caused by an already-gone client.
128+
129+
Every HTTP entry point runs Streamable HTTP in stateless mode (see
130+
``ha_mcp.__main__._http_run_kwargs``). A tool call slow enough to outlast
131+
the client's patience -- a busy Home Assistant instance, a
132+
resource-contended local LLM host on the client side, or an ordinary HTTP
133+
timeout -- lets the SDK finish serving the response and tear the
134+
transport down while ``app.run()`` is still working; the eventual attempt
135+
to deliver the response then writes into an already-closed memory stream
136+
and raises ``anyio.ClosedResourceError`` or ``anyio.BrokenResourceError``
137+
(wrapped in an ``ExceptionGroup`` -- see
138+
``_is_only_disconnect_teardown_errors``). The SDK already catches this
139+
(``except Exception: logger.exception(...)`` in both the stateless and
140+
stateful session runners of mcp/server/streamable_http_manager.py) -- it
141+
just logs it as an alarming ERROR-level traceback. That's an expected
142+
race in a stateless HTTP protocol (the client already gave up), not a
143+
server bug, so demote it the same way ToolValidationLogFilter demotes
144+
other known-benign failures. Any other exception on this logger -- an
145+
actual crash -- is left untouched.
146+
"""
147+
148+
def filter(self, record: logging.LogRecord) -> bool:
149+
"""Demote a disconnect-caused 'session crashed' record to WARNING."""
150+
if record.name != "mcp.server.streamable_http_manager" or not record.exc_info:
151+
return True
152+
153+
err = record.exc_info[1]
154+
if err is None or not _is_only_disconnect_teardown_errors(err):
155+
return True
156+
157+
record.msg = (
158+
f"{record.getMessage()}: client disconnected before response delivery"
159+
)
160+
record.args = ()
161+
record.levelno = logging.WARNING
162+
record.levelname = "WARNING"
163+
record.exc_info = None
164+
record.exc_text = None
165+
return True
166+
167+
168+
def _add_filter_once(logger_name: str, filter_cls: type[logging.Filter]) -> None:
169+
"""Attach one ``filter_cls`` instance to ``logger_name``, replacing any stale one.
170+
171+
``install_sdk_log_filters()`` can run more than once per process: the
172+
in-process embedded server calls it on every ``_serve()`` reload without
173+
a process restart, and process-wide ``logging`` state (including each
174+
named logger's filter list) persists across those reloads. Each reload
175+
also re-imports this module from scratch (the embedded server purges
176+
``ha_mcp.*`` from ``sys.modules`` before reinstalling -- see
177+
``_purge_ha_mcp_modules``), so ``filter_cls`` is a BRAND NEW class object
178+
each time, even though its name is unchanged. A same-``isinstance``-check
179+
against a filter instance from a PREVIOUS generation would therefore
180+
never match, letting filters accumulate one more stale instance per
181+
reload forever. Match by ``(module, qualname)`` instead -- stable across
182+
reloads of the same module path -- and drop every stale-generation match
183+
before attaching the current one, so a logger never carries more than
184+
one filter of a given conceptual type, and it's always this generation's.
185+
"""
186+
logger = logging.getLogger(logger_name)
187+
identity = (filter_cls.__module__, filter_cls.__qualname__)
188+
logger.filters[:] = [
189+
f
190+
for f in logger.filters
191+
if (type(f).__module__, type(f).__qualname__) != identity
192+
]
193+
logger.addFilter(filter_cls())
194+
195+
196+
def install_sdk_log_filters() -> None:
197+
"""Attach the demotion filters above to their target SDK/fastmcp loggers.
198+
199+
Every HTTP launcher must call this: the CLI (``ha_mcp.__main__``), the
200+
Home Assistant app's ``start.py``, and the in-process embedded server
201+
(``ha_mcp_tools/embedded_server.py``) each build and run their own
202+
Streamable HTTP app, so none of them share another launcher's logging
203+
setup. Safe to call repeatedly -- see ``_add_filter_once``.
204+
"""
205+
_add_filter_once("mcp.server.streamable_http", StatelessSessionLogFilter)
206+
_add_filter_once("mcp.server.streamable_http_manager", SessionDisconnectLogFilter)
207+
_add_filter_once("fastmcp.server.server", ToolValidationLogFilter)

0 commit comments

Comments
 (0)