From da11cef29a7dc2bff65229cc3f2809e7344e4e3d Mon Sep 17 00:00:00 2001
From: atlflyer <31390797+atlflyer@users.noreply.github.com>
Date: Sat, 2 Jul 2022 09:58:08 -0400
Subject: [PATCH] Report error code in log when command fails (#74319)

---
 homeassistant/components/command_line/__init__.py   | 12 +++++++++---
 homeassistant/components/command_line/cover.py      |  7 +++++--
 homeassistant/components/command_line/notify.py     |  6 +++++-
 tests/components/command_line/test_binary_sensor.py | 13 +++++++++++++
 tests/components/command_line/test_cover.py         |  3 ++-
 tests/components/command_line/test_notify.py        |  1 +
 tests/components/command_line/test_sensor.py        | 11 +++++++++++
 tests/components/command_line/test_switch.py        | 13 +++++++++++++
 8 files changed, 59 insertions(+), 7 deletions(-)

diff --git a/homeassistant/components/command_line/__init__.py b/homeassistant/components/command_line/__init__.py
index 1bcaaaa60a6..172c321c0ec 100644
--- a/homeassistant/components/command_line/__init__.py
+++ b/homeassistant/components/command_line/__init__.py
@@ -23,7 +23,11 @@ def call_shell_with_timeout(
         return 0
     except subprocess.CalledProcessError as proc_exception:
         if log_return_code:
-            _LOGGER.error("Command failed: %s", command)
+            _LOGGER.error(
+                "Command failed (with return code %s): %s",
+                proc_exception.returncode,
+                command,
+            )
         return proc_exception.returncode
     except subprocess.TimeoutExpired:
         _LOGGER.error("Timeout for command: %s", command)
@@ -40,8 +44,10 @@ def check_output_or_log(command: str, timeout: int) -> str | None:
             command, shell=True, timeout=timeout  # nosec # shell by design
         )
         return return_value.strip().decode("utf-8")
-    except subprocess.CalledProcessError:
-        _LOGGER.error("Command failed: %s", command)
+    except subprocess.CalledProcessError as err:
+        _LOGGER.error(
+            "Command failed (with return code %s): %s", err.returncode, command
+        )
     except subprocess.TimeoutExpired:
         _LOGGER.error("Timeout for command: %s", command)
     except subprocess.SubprocessError:
diff --git a/homeassistant/components/command_line/cover.py b/homeassistant/components/command_line/cover.py
index 609166f2d16..8298201228f 100644
--- a/homeassistant/components/command_line/cover.py
+++ b/homeassistant/components/command_line/cover.py
@@ -115,10 +115,13 @@ class CommandCover(CoverEntity):
         """Execute the actual commands."""
         _LOGGER.info("Running command: %s", command)
 
-        success = call_shell_with_timeout(command, self._timeout) == 0
+        returncode = call_shell_with_timeout(command, self._timeout)
+        success = returncode == 0
 
         if not success:
-            _LOGGER.error("Command failed: %s", command)
+            _LOGGER.error(
+                "Command failed (with return code %s): %s", returncode, command
+            )
 
         return success
 
diff --git a/homeassistant/components/command_line/notify.py b/homeassistant/components/command_line/notify.py
index 6f364947775..7bce5010d45 100644
--- a/homeassistant/components/command_line/notify.py
+++ b/homeassistant/components/command_line/notify.py
@@ -57,7 +57,11 @@ class CommandLineNotificationService(BaseNotificationService):
             try:
                 proc.communicate(input=message, timeout=self._timeout)
                 if proc.returncode != 0:
-                    _LOGGER.error("Command failed: %s", self.command)
+                    _LOGGER.error(
+                        "Command failed (with return code %s): %s",
+                        proc.returncode,
+                        self.command,
+                    )
             except subprocess.TimeoutExpired:
                 _LOGGER.error("Timeout for command: %s", self.command)
                 kill_subprocess(proc)
diff --git a/tests/components/command_line/test_binary_sensor.py b/tests/components/command_line/test_binary_sensor.py
index b523b02aa64..18f680fbb02 100644
--- a/tests/components/command_line/test_binary_sensor.py
+++ b/tests/components/command_line/test_binary_sensor.py
@@ -3,6 +3,8 @@ from __future__ import annotations
 
 from typing import Any
 
+from pytest import LogCaptureFixture
+
 from homeassistant import setup
 from homeassistant.components.binary_sensor import DOMAIN
 from homeassistant.const import STATE_OFF, STATE_ON
@@ -112,3 +114,14 @@ async def test_unique_id(hass: HomeAssistant) -> None:
         )
         is not None
     )
+
+
+async def test_return_code(caplog: LogCaptureFixture, hass: HomeAssistant) -> None:
+    """Test setting the state with a template."""
+    await setup_test_entity(
+        hass,
+        {
+            "command": "exit 33",
+        },
+    )
+    assert "return code 33" in caplog.text
diff --git a/tests/components/command_line/test_cover.py b/tests/components/command_line/test_cover.py
index 98c917f51ba..deb80953428 100644
--- a/tests/components/command_line/test_cover.py
+++ b/tests/components/command_line/test_cover.py
@@ -155,7 +155,7 @@ async def test_reload(hass: HomeAssistant) -> None:
 async def test_move_cover_failure(
     caplog: LogCaptureFixture, hass: HomeAssistant
 ) -> None:
-    """Test with state value."""
+    """Test command failure."""
 
     await setup_test_entity(
         hass,
@@ -165,6 +165,7 @@ async def test_move_cover_failure(
         DOMAIN, SERVICE_OPEN_COVER, {ATTR_ENTITY_ID: "cover.test"}, blocking=True
     )
     assert "Command failed" in caplog.text
+    assert "return code 1" in caplog.text
 
 
 async def test_unique_id(hass: HomeAssistant) -> None:
diff --git a/tests/components/command_line/test_notify.py b/tests/components/command_line/test_notify.py
index 26b53a827e7..5cef13e45b4 100644
--- a/tests/components/command_line/test_notify.py
+++ b/tests/components/command_line/test_notify.py
@@ -77,6 +77,7 @@ async def test_error_for_none_zero_exit_code(
         DOMAIN, "test", {"message": "error"}, blocking=True
     )
     assert "Command failed" in caplog.text
+    assert "return code 1" in caplog.text
 
 
 async def test_timeout(caplog: LogCaptureFixture, hass: HomeAssistant) -> None:
diff --git a/tests/components/command_line/test_sensor.py b/tests/components/command_line/test_sensor.py
index 62ec1dbe97b..bdb36eebaa1 100644
--- a/tests/components/command_line/test_sensor.py
+++ b/tests/components/command_line/test_sensor.py
@@ -128,6 +128,17 @@ async def test_bad_command(hass: HomeAssistant) -> None:
     assert entity_state.state == "unknown"
 
 
+async def test_return_code(caplog: LogCaptureFixture, hass: HomeAssistant) -> None:
+    """Test that an error return code is logged."""
+    await setup_test_entities(
+        hass,
+        {
+            "command": "exit 33",
+        },
+    )
+    assert "return code 33" in caplog.text
+
+
 async def test_update_with_json_attrs(hass: HomeAssistant) -> None:
     """Test attributes get extracted from a JSON result."""
     await setup_test_entities(
diff --git a/tests/components/command_line/test_switch.py b/tests/components/command_line/test_switch.py
index 307974ab3fe..267f7cf7b06 100644
--- a/tests/components/command_line/test_switch.py
+++ b/tests/components/command_line/test_switch.py
@@ -419,3 +419,16 @@ async def test_unique_id(hass: HomeAssistant) -> None:
         ent_reg.async_get_entity_id("switch", "command_line", "not-so-unique-anymore")
         is not None
     )
+
+
+async def test_command_failure(caplog: LogCaptureFixture, hass: HomeAssistant) -> None:
+    """Test command failure."""
+
+    await setup_test_entity(
+        hass,
+        {"test": {"command_off": "exit 33"}},
+    )
+    await hass.services.async_call(
+        DOMAIN, SERVICE_TURN_OFF, {ATTR_ENTITY_ID: "switch.test"}, blocking=True
+    )
+    assert "return code 33" in caplog.text