fix: stop health monitor spam and garbled notification metadata
Root causes:
1. No dedup for monitor restarts — _last_known_state is cleared on stop,
so every restart re-sent notifications for all unhealthy instances.
2. Aggressive error classification — any Docker state other than 'running'
was treated as 'error', including transient 'created' and 'restarting'.
3. Confusing metadata — when new_status == previous_status (after restart),
notifications showed previous_status equal to current status.
Fixes:
- _check_instance: when previous is None (first check) and new_status equals
the DB status, just record the snapshot and skip _handle_state_change.
This prevents duplicate events/notifications on monitor restart.
- _derive_status: only treat 'exited' and 'dead' as error. Preserve current
status for transient Docker states ('created', 'restarting').
- _derive_status: if DB says 'running' but container is 'not_found',
return 'error' instead of preserving 'running' (fixes silent failure).
- _handle_state_change: improved unhealthy message to 'Container tunnel is
unreachable' instead of generic 'Container is now unhealthy'.
Quality gates: py_compile all backend files pass, tsc --noEmit pass,
npm run build pass, 82/82 tests pass
This commit is contained in:
@@ -149,12 +149,22 @@ class HealthMonitor:
|
|||||||
instance.status,
|
instance.status,
|
||||||
)
|
)
|
||||||
|
|
||||||
# If first check or state changed
|
# If first check or snapshot changed
|
||||||
if previous is None or not self._snapshots_equal(previous, snapshot):
|
if previous is None:
|
||||||
await self._handle_state_change(
|
# Monitor restart / first time seeing this instance.
|
||||||
session, instance, previous, snapshot, new_status
|
# Only act if the derived status is different from the DB status.
|
||||||
)
|
# This prevents duplicate notifications after monitor restarts.
|
||||||
self._last_known_state[instance.id] = snapshot
|
if new_status == instance.status:
|
||||||
|
self._last_known_state[instance.id] = snapshot
|
||||||
|
return
|
||||||
|
elif self._snapshots_equal(previous, snapshot):
|
||||||
|
# Nothing changed since last poll — skip entirely.
|
||||||
|
return
|
||||||
|
|
||||||
|
await self._handle_state_change(
|
||||||
|
session, instance, previous, snapshot, new_status
|
||||||
|
)
|
||||||
|
self._last_known_state[instance.id] = snapshot
|
||||||
|
|
||||||
def _derive_status(
|
def _derive_status(
|
||||||
self,
|
self,
|
||||||
@@ -169,24 +179,34 @@ class HealthMonitor:
|
|||||||
alerts for instances that are still starting or have no container yet.
|
alerts for instances that are still starting or have no container yet.
|
||||||
"""
|
"""
|
||||||
if snapshot.container_status == "not_found":
|
if snapshot.container_status == "not_found":
|
||||||
# If the container was never seen running, assume it's still
|
# Still starting — container may not exist yet.
|
||||||
# starting or was deleted intentionally; don't flag as error.
|
if current_status == "starting":
|
||||||
if previous is None and current_status == "starting":
|
|
||||||
return "starting"
|
return "starting"
|
||||||
|
# Container disappeared while it was supposed to be running.
|
||||||
|
if current_status == "running":
|
||||||
|
return "error"
|
||||||
|
# If we have previous memory and the container was running,
|
||||||
|
# mark as error (handles monitor restart edge case).
|
||||||
if previous is not None and previous.container_status == "running":
|
if previous is not None and previous.container_status == "running":
|
||||||
return "error"
|
return "error"
|
||||||
# Fall back to current status to avoid spurious errors.
|
# Fall back to current status to avoid spurious errors.
|
||||||
return current_status or "error"
|
return current_status or "error"
|
||||||
|
|
||||||
if snapshot.container_status == "exited":
|
if snapshot.container_status in ("exited", "dead"):
|
||||||
return "error"
|
return "error"
|
||||||
|
|
||||||
if snapshot.container_status != "running":
|
if snapshot.container_status == "running":
|
||||||
return "error"
|
if snapshot.tunnel_healthy is False:
|
||||||
|
return "unhealthy"
|
||||||
|
return "running"
|
||||||
|
|
||||||
if snapshot.tunnel_healthy is False:
|
# Transient states (created, restarting) — preserve current status
|
||||||
return "unhealthy"
|
# instead of treating them as an error. The next poll will resolve.
|
||||||
return "running"
|
if snapshot.container_status in ("created", "restarting"):
|
||||||
|
return current_status or "starting"
|
||||||
|
|
||||||
|
# Unknown/unexpected state (paused, etc.)
|
||||||
|
return "error"
|
||||||
|
|
||||||
def _snapshots_equal(self, a: HealthSnapshot, b: HealthSnapshot) -> bool:
|
def _snapshots_equal(self, a: HealthSnapshot, b: HealthSnapshot) -> bool:
|
||||||
"""Compare two snapshots for equality."""
|
"""Compare two snapshots for equality."""
|
||||||
@@ -242,7 +262,10 @@ class HealthMonitor:
|
|||||||
message += f" (exit code: {snapshot.exit_code})"
|
message += f" (exit code: {snapshot.exit_code})"
|
||||||
else:
|
else:
|
||||||
event_type = "instance.health_changed"
|
event_type = "instance.health_changed"
|
||||||
message = f"Container is now {new_status}"
|
if new_status == "unhealthy":
|
||||||
|
message = "Container tunnel is unreachable"
|
||||||
|
else:
|
||||||
|
message = f"Container is now {new_status}"
|
||||||
|
|
||||||
payload: InstanceEventPayload = {
|
payload: InstanceEventPayload = {
|
||||||
"event": event_type,
|
"event": event_type,
|
||||||
|
|||||||
Reference in New Issue
Block a user