From 18ec213504aa01b7c5e990ab5e569466edbbfa4f Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Sun, 16 Aug 2026 18:36:56 +0200 Subject: [PATCH] fix(health): bound and gather the remaining three health probes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit paperless, system_settings and scheduler ran serially after the five probes bounded in 80229f2, with no timeout of their own (scheduler defaults to 30.0s, system_settings' connect() is unbounded) — the same defect class, just further down the same function. All eight probes now join one asyncio.gather(return_exceptions=True), each bounded at 2s via the existing _bounded_probe. paperless and system_settings keep their three-state result: an unconfigured service is excluded from the probe list entirely rather than run through the bound, so "not configured" (None) cannot collapse into "unhealthy" (False) the way it would if the bound's bool-only wrapper were applied uniformly. scheduler has no config gate and joins the gather plainly. Neither is read by /health today — only src/main.py and tests/test_integration.py call check_service_health(), and both read only the original five — so this is unreachable from the live endpoint. Bounded rather than deleted: bounding cannot break a consumer that reads these keys later, deleting could. --- CHANGELOG.md | 8 +++ src/core/dependencies.py | 141 ++++++++++++++++++++++++--------------- 2 files changed, 95 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 268e1b9..1f3cb47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 reason unrelated to its own liveness. A probe that times out now reports that service unhealthy, the same as one that raises; the others are unaffected. +- The three remaining probes in `check_service_health()` (paperless, + system_settings, scheduler — computed internally but not currently read by + `/health`) are now bounded and joined into the same `asyncio.gather` as the + five above, rather than running serially and unbounded after it. paperless + and system_settings keep their three-state result (`None` not configured, + `False` configured but unhealthy/timed out, `True` healthy) — an + unconfigured service's probe is never run, so "not configured" cannot be + collapsed into "unhealthy". ## [1.9.1] - 2026-08-08 diff --git a/src/core/dependencies.py b/src/core/dependencies.py index 931489e..6da54ee 100644 --- a/src/core/dependencies.py +++ b/src/core/dependencies.py @@ -634,6 +634,36 @@ async def _probe_ollama() -> bool: return False +async def _probe_paperless() -> bool: + """Paperless-ngx connectivity probe. Returns True if healthy.""" + try: + paperless = get_paperless_client() + return await paperless.health_check() + except Exception as e: + logger.error(f"Paperless health check failed: {e}") + return False + + +async def _probe_system_settings() -> bool: + """System settings database connectivity probe. Returns True if healthy.""" + try: + settings_client = get_settings_client() + return await settings_client.health_check() + except Exception as e: + logger.error(f"System settings health check failed: {e}") + return False + + +async def _probe_scheduler() -> bool: + """Scheduler connectivity probe. Returns True if healthy.""" + try: + scheduler = get_scheduler_client() + return await scheduler.health_check() + except Exception as e: + logger.error(f"Scheduler health check failed: {e}") + return False + + async def _bounded_probe(coro) -> bool: """ Run a probe coroutine bounded by HEALTH_PROBE_TIMEOUT. @@ -654,11 +684,28 @@ async def check_service_health() -> dict: """ Check health of all service clients. - The five core probes (neo4j, qdrant, wikijs, searxng, ollama) run - concurrently, each bounded at HEALTH_PROBE_TIMEOUT, so a single hung - dependency cannot block the others or push the endpoint past the - container healthcheck's timeout. return_exceptions=True on the gather - means one probe raising cannot cancel its siblings. + All eight probes (neo4j, qdrant, wikijs, searxng, ollama, paperless, + system_settings, scheduler) run concurrently in one gather, each bounded + at HEALTH_PROBE_TIMEOUT, so a single hung dependency cannot block the + others or push the endpoint past the container healthcheck's timeout. + return_exceptions=True means one probe raising cannot cancel its + siblings. + + paperless and system_settings are three-state: None means "not + configured" (no probe is run at all — a probe never joins the gather + for a service that has no credentials to check), False means configured + but unreachable/unhealthy (including a timeout), True means healthy. + Collapsing "not configured" into "unhealthy" would be a different claim + than the one this function is making, so that decision is made before + the gather rather than by feeding an unconfigured probe through the + same bool-returning bound as the rest. + + Only neo4j, qdrant, wikijs, searxng and ollama are read by /health + (src/main.py) — paperless, system_settings and scheduler are computed + here but not currently surfaced by any caller (checked: the only two + callers are src/main.py and tests/test_integration.py, and the test + asserts only the five). Bounded rather than removed, since bounding + cannot break a hypothetical consumer and deleting could. Returns: Dictionary with health status of each service: @@ -667,7 +714,10 @@ async def check_service_health() -> dict: "qdrant": bool, "wikijs": bool, "searxng": bool, - "ollama": bool + "ollama": bool, + "paperless": bool | None, + "system_settings": bool | None, + "scheduler": bool } Usage: @@ -676,65 +726,48 @@ async def check_service_health() -> dict: True """ health = {} + settings = get_settings() - neo4j_result, qdrant_result, wikijs_result, searxng_result, ollama_result = ( - await asyncio.gather( - _bounded_probe(_probe_neo4j()), - _bounded_probe(_probe_qdrant()), - _bounded_probe(_probe_wikijs()), - _bounded_probe(_probe_searxng()), - _bounded_probe(_probe_ollama()), - return_exceptions=True, - ) - ) + # Build the probe list dynamically: paperless and system_settings only + # join it when configured, so an unconfigured service is never bounded, + # timed out, or reported False — it is set to None directly, below. + probe_names = ["neo4j", "qdrant", "wikijs", "searxng", "ollama"] + probes = [ + _bounded_probe(_probe_neo4j()), + _bounded_probe(_probe_qdrant()), + _bounded_probe(_probe_wikijs()), + _bounded_probe(_probe_searxng()), + _bounded_probe(_probe_ollama()), + ] + + if settings.paperless_token: + probe_names.append("paperless") + probes.append(_bounded_probe(_probe_paperless())) + else: + health["paperless"] = None # Not configured + + if settings.system_settings_password: + probe_names.append("system_settings") + probes.append(_bounded_probe(_probe_system_settings())) + else: + health["system_settings"] = None # Not configured + + # Scheduler has no config gate - it always runs. + probe_names.append("scheduler") + probes.append(_bounded_probe(_probe_scheduler())) + + results = await asyncio.gather(*probes, return_exceptions=True) # _bounded_probe already catches everything from its own probe, but # return_exceptions=True also guards against a bug in _bounded_probe # itself surfacing as an unhandled exception here. - for name, result in ( - ("neo4j", neo4j_result), - ("qdrant", qdrant_result), - ("wikijs", wikijs_result), - ("searxng", searxng_result), - ("ollama", ollama_result), - ): + for name, result in zip(probe_names, results): if isinstance(result, BaseException): logger.error(f"{name} health check raised unexpectedly: {result}") health[name] = False else: health[name] = result - # Paperless-ngx - settings = get_settings() - if settings.paperless_token: - try: - paperless = get_paperless_client() - health["paperless"] = await paperless.health_check() - except Exception as e: - logger.error(f"Paperless health check failed: {e}") - health["paperless"] = False - else: - health["paperless"] = None # Not configured - - # System Settings database - if settings.system_settings_password: - try: - settings_client = get_settings_client() - health["system_settings"] = await settings_client.health_check() - except Exception as e: - logger.error(f"System settings health check failed: {e}") - health["system_settings"] = False - else: - health["system_settings"] = None # Not configured - - # Scheduler - try: - scheduler = get_scheduler_client() - health["scheduler"] = await scheduler.health_check() - except Exception as e: - logger.error(f"Scheduler health check failed: {e}") - health["scheduler"] = False - return health