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 (34adf28only 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) <noreply@anthropic.com>
This commit is contained in:
+24
-10
@@ -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():
|
||||
|
||||
@@ -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")
|
||||
Reference in New Issue
Block a user