From cc0e023941ea6b325a5b821a4c8440ba8d03868f Mon Sep 17 00:00:00 2001 From: Mortdecai Date: Sat, 24 Oct 2026 11:06:03 -0400 Subject: [PATCH] fix: catch helper timeouts in run(); report probed state when a command fails run() now turns subprocess.TimeoutExpired into rc=124, so the poll loop and on_connect can no longer die on a stalled lgtv.sh (34adf28 only guarded the command path). A failed ON/OFF publishes a fresh status probe instead of the requested state. Payload decode uses errors="replace"; safe_handle (34adf28) already kept bad bytes from killing paho's thread. tests/test_bridge.py replays the reviewer's three repros. Fixes-finding: uncaught-helper-timeout-stops-bridge Fixes-finding: failed-power-commands-publish-success-state Fixes-finding: malformed-mqtt-payload-kills-network-loop Co-Authored-By: Claude Opus 5.5 (1M context) --- lgtv_mqtt.py | 34 ++++++++++++++++++++-------- tests/test_bridge.py | 54 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 10 deletions(-) create mode 100644 tests/test_bridge.py diff --git a/lgtv_mqtt.py b/lgtv_mqtt.py index 3c36e0c..0020ad1 100755 --- a/lgtv_mqtt.py +++ b/lgtv_mqtt.py @@ -44,9 +44,15 @@ log = logging.getLogger(__name__) def run(cmd, timeout=60): - result = subprocess.run( - cmd, shell=True, capture_output=True, text=True, timeout=timeout - ) + # Every caller (poll loop, on_connect, commands) routes through here, so a + # stalled helper becomes a plain failure instead of an exception. + try: + result = subprocess.run( + cmd, shell=True, capture_output=True, text=True, timeout=timeout + ) + except subprocess.TimeoutExpired: + log.warning(f"timed out after {timeout}s: {cmd}") + return "timeout", 124 return result.stdout.strip(), result.returncode @@ -129,23 +135,31 @@ def safe_handle(client, msg): def handle_command(client, msg): - command = msg.payload.decode().strip().upper() + command = msg.payload.decode(errors="replace").strip().upper() log.info(f"Received command: {command}") if command == "ON": - tv_on() + ok = tv_on() + elif command == "OFF": + ok = tv_off() + else: + log.warning(f"Unknown command: {command}") + return + + if not ok: + # Failed command: report what the TV actually is, not what was asked. + state = get_tv_state() + client.publish(STATE_TOPIC, state, retain=True) + log.warning(f"{command} failed; published probed state: {state}") + elif command == "ON": # Publish ON optimistically immediately — WoL takes ~20s to appear on network. # The 30s poll loop will correct it if it failed. client.publish(STATE_TOPIC, "ON", retain=True) log.info("State published: ON (optimistic)") - elif command == "OFF": - tv_off() + else: time.sleep(3) client.publish(STATE_TOPIC, "OFF", retain=True) log.info("State published: OFF") - else: - log.warning(f"Unknown command: {command}") - return def main(): diff --git a/tests/test_bridge.py b/tests/test_bridge.py new file mode 100644 index 0000000..76c3068 --- /dev/null +++ b/tests/test_bridge.py @@ -0,0 +1,54 @@ +"""Regression checks for the 3 hermes reviewer findings. No broker, no TV. +Run: python3 tests/test_bridge.py""" +import subprocess +from pathlib import Path +from types import SimpleNamespace + +import paho.mqtt.client as mqtt + +SRC = Path(__file__).resolve().parents[1] / "lgtv_mqtt.py" + + +def load(run): + ns = {"__name__": "test"} + exec(compile(SRC.read_text(), str(SRC), "exec"), ns) + ns["run"] = run + ns["time"] = SimpleNamespace(sleep=lambda _: None) + return ns + + +def command(ns, payload): + sent = [] + client = SimpleNamespace(publish=lambda topic, p, retain=False: sent.append(p)) + ns["handle_command"](client, SimpleNamespace(payload=payload)) + return sent + + +# failed-power-commands-publish-success-state: on/off fail, TV really still ON +ns = load(lambda cmd, **k: ("TV: ON", 0) if cmd.endswith("status") else ("", 1)) +assert command(ns, b"OFF") == ["ON"], "failed OFF must not report OFF" +ns = load(lambda cmd, **k: ("TV: OFF", 0) if cmd.endswith("status") else ("", 1)) +assert command(ns, b"ON") == ["OFF"], "failed ON must not report ON" +ns = load(lambda cmd, **k: ("", 0)) +assert command(ns, b"ON") == ["ON"] and command(ns, b"OFF") == ["OFF"] + +# uncaught-helper-timeout-stops-bridge: real run(), helper that outlives timeout +ns = {"__name__": "test"} +exec(compile(SRC.read_text(), str(SRC), "exec"), ns) +assert ns["run"]("sleep 5", timeout=0.2) == ("timeout", 124) +ns["LGTV_SCRIPT"] = "sleep 5;" +ns["run"].__defaults__ = (0.2,) +assert ns["get_tv_state"]() == "OFF" + +# malformed-mqtt-payload-kills-network-loop: real paho dispatch survives +ns = load(lambda cmd, **k: ("", 0)) +client = mqtt.Client(client_id="test-no-network") +client.on_message = ns["on_message"] +msg = mqtt.MQTTMessage() +msg.topic = b"lgtv/switch/command" +msg.payload = b"\xff" +client._handle_on_message(msg) +ns["worker"].shutdown(wait=True) +assert command(ns, b"\xff") == [] # decoded, logged as unknown, ignored + +print("ok")