Update Signal alert handling

python/signal_alert.py

@@ -5,23 +5,28 @@ from __future__ import annotations
 import json
 import logging
 import socket
-from os import getenv
 
 logger = logging.getLogger(__name__)
 
 
-def signal_alert(body: str) -> None:
+class SignalRPCError(RuntimeError):
-    """Send a signal alert.
+    """signal-cli returned an error or closed without responding."""
+
+
+def signal_alert(body: str, from_phone: str, to_phone: str, *, timeout: float = 4.0) -> None:
+    """Send a Signal alert through the local signal-cli daemon.
 
     Args:
-        body (str): The body of the alert.
+        body: The body of the alert.
-    """
+        from_phone: The Signal account sending the alert.
-    from_phone = getenv("SIGNAL_ALERT_FROM_PHONE")
+        to_phone: The Signal account receiving the alert.
-    to_phone = getenv("SIGNAL_ALERT_TO_PHONE")
+        timeout: Seconds to wait on each socket operation.
-    if not from_phone or not to_phone:
-        logger.info("SIGNAL_ALERT_FROM_PHONE or SIGNAL_ALERT_TO_PHONE not set")
-        return
 
+    Raises:
+        SignalRPCError: If signal-cli returns an error or closes the
+            connection before responding.
+        OSError: If the socket is unreachable or an operation times out.
+    """
     signal_rpc_id = "signal-alert"
     request = {
         "jsonrpc": "2.0",
@@ -36,7 +41,7 @@ def signal_alert(body: str) -> None:
 
     try:
         with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as client:
-            client.settimeout(4.0)
+            client.settimeout(timeout)
             client.connect("/run/signal-cli/socket")
             client.sendall(json.dumps(request).encode() + b"\n")
 
@@ -46,11 +51,14 @@ def signal_alert(body: str) -> None:
                 while b"\n" in buffer:
                     line, buffer = buffer.split(b"\n", maxsplit=1)
                     response = json.loads(line)
-                    if response.get("id") == signal_rpc_id:
+                    if response.get("id") != signal_rpc_id:
-                        if error := response.get("error"):
+                        continue
-                            logger.error("Signal RPC returned an error: %s", error)
+                    if rpc_error := response.get("error"):
-                        return
+                        error = f"Signal RPC returned an error: {rpc_error}"
-
+                        raise SignalRPCError(error)
-            logger.error("Signal RPC socket closed before returning a response")
+                    return
-    except OSError, json.JSONDecodeError:
+            error = "Signal RPC socket closed before returning a response"
-        logger.exception("Unable to communicate with signal-cli")
+            raise SignalRPCError(error)
+    except (OSError, json.JSONDecodeError) as exc:
+        error = f"Signal RPC failed: {exc!r}"
+        raise SignalRPCError(error) from exc

python/tools/snapshot_manager.py

@@ -6,6 +6,7 @@ import logging
 import sys
 import tomllib
 from functools import cache
+from os import getenv
 from pathlib import Path  # noqa: TC003 This is required for the typer CLI
 from re import compile as re_compile
 from re import search
@@ -24,6 +25,15 @@ def main(config_file: Path) -> None:
     configure_logger(level="DEBUG")
     logger.info("Starting snapshot_manager")
 
+    from_phone = getenv("SIGNAL_ALERT_FROM_PHONE")
+    to_phone = getenv("SIGNAL_ALERT_TO_PHONE")
+    if not from_phone:
+        error = "SIGNAL_ALERT_FROM_PHONE environment variable is not set"
+        raise ValueError(error)
+    if not to_phone:
+        error = "SIGNAL_ALERT_TO_PHONE environment variable is not set"
+        raise ValueError(error)
+
     try:
         time_stamp = get_time_stamp()
 
@@ -33,14 +43,14 @@ def main(config_file: Path) -> None:
             if status != "snapshot created":
                 msg = f"{dataset.name} failed to create snapshot {time_stamp}"
                 logger.error(msg)
-                signal_alert(msg)
+                signal_alert(msg, from_phone, to_phone)
                 continue
             count_lookup = get_count_lookup(config_file, dataset.name)
             logger.info(f"using {count_lookup} for {dataset.name}")
-            get_snapshots_to_delete(dataset, count_lookup)
+            get_snapshots_to_delete(dataset, count_lookup, from_phone, to_phone)
     except Exception:
         logger.exception("snapshot_manager failed")
-        signal_alert("snapshot_manager failed")
+        signal_alert("snapshot_manager failed", from_phone, to_phone)
         sys.exit(1)
     else:
         logger.info("snapshot_manager completed")
@@ -92,12 +102,16 @@ def load_config_data(config_file: Path) -> dict[str, dict[str, int]]:
 def get_snapshots_to_delete(
     dataset: Dataset,
     count_lookup: dict[str, int],
+    from_phone: str,
+    to_phone: str,
 ) -> None:
     """Get snapshots to delete.
 
     Args:
         dataset (Dataset): the dataset
         count_lookup (dict[str, int]): the count lookup
+        from_phone (str): The Signal account sending alerts.
+        to_phone (str): The Signal account receiving alerts.
     """
     snapshots = dataset.get_snapshots()
 
@@ -127,7 +141,7 @@ def get_snapshots_to_delete(
         for snapshot in snapshots_being_deleted:
             if error := dataset.delete_snapshot(snapshot):
                 error_message = f"{dataset.name}@{snapshot} failed to delete: {error}"
-                signal_alert(error_message)
+                signal_alert(error_message, from_phone, to_phone)
                 logger.error(error_message)
 
 

tests/test_signal_alert.py

@@ -4,25 +4,23 @@ from __future__ import annotations
 
 import json
 import socket
-from os import environ
 from typing import TYPE_CHECKING
 
-from python.signal_alert import signal_alert
+import pytest
+
+from python.signal_alert import SignalRPCError, signal_alert
 
 if TYPE_CHECKING:
     from pytest_mock import MockerFixture
 
 
 def test_signal_alert(mocker: MockerFixture) -> None:
-    environ["SIGNAL_ALERT_FROM_PHONE"] = "1234567890"
-    environ["SIGNAL_ALERT_TO_PHONE"] = "0987654321"
-
     mock_logger = mocker.patch("python.signal_alert.logger")
     mock_socket = mocker.patch("python.signal_alert.socket.socket")
     client = mock_socket.return_value.__enter__.return_value
     client.recv.side_effect = [b'{"jsonrpc":"2.0","result":{},"id":"signal-alert"}\n']
 
-    signal_alert("test")
+    assert signal_alert("test", "1234567890", "0987654321") is None
 
     mock_logger.info.assert_not_called()
     mock_socket.assert_called_once_with(socket.AF_UNIX, socket.SOCK_STREAM)
@@ -41,55 +39,31 @@ def test_signal_alert(mocker: MockerFixture) -> None:
     }
 
 
-def test_signal_alert_no_phones(mocker: MockerFixture) -> None:
-    environ.pop("SIGNAL_ALERT_FROM_PHONE", None)
-    environ.pop("SIGNAL_ALERT_TO_PHONE", None)
-    mock_logger = mocker.patch("python.signal_alert.logger")
-
-    signal_alert("test")
-
-    mock_logger.info.assert_called_once_with("SIGNAL_ALERT_FROM_PHONE or SIGNAL_ALERT_TO_PHONE not set")
-
-
 def test_signal_alert_socket_error(mocker: MockerFixture) -> None:
-    environ["SIGNAL_ALERT_FROM_PHONE"] = "1234567890"
-    environ["SIGNAL_ALERT_TO_PHONE"] = "0987654321"
-
-    mock_logger = mocker.patch("python.signal_alert.logger")
     mocker.patch("python.signal_alert.socket.socket", side_effect=ConnectionError("connection failed"))
 
-    signal_alert("test")
+    with pytest.raises(SignalRPCError, match="Signal RPC failed") as exc_info:
+        signal_alert("test", "1234567890", "0987654321")
 
-    mock_logger.exception.assert_called_once_with("Unable to communicate with signal-cli")
+    assert isinstance(exc_info.value.__cause__, ConnectionError)
 
 
 def test_signal_alert_rpc_error(mocker: MockerFixture) -> None:
-    environ["SIGNAL_ALERT_FROM_PHONE"] = "1234567890"
-    environ["SIGNAL_ALERT_TO_PHONE"] = "0987654321"
-
-    mock_logger = mocker.patch("python.signal_alert.logger")
     mock_socket = mocker.patch("python.signal_alert.socket.socket")
     client = mock_socket.return_value.__enter__.return_value
     client.recv.side_effect = [b'{"jsonrpc":"2.0","error":{"code":-1,"message":"failed"},"id":"signal-alert"}\n']
 
-    signal_alert("test")
+    with pytest.raises(SignalRPCError, match=r"Signal RPC returned an error:.*failed"):
-
+        signal_alert("test", "1234567890", "0987654321")
-    mock_logger.error.assert_called_once_with(
-        "Signal RPC returned an error: %s",
-        {"code": -1, "message": "failed"},
-    )
 
 
 def test_signal_alert_ignores_notifications(mocker: MockerFixture) -> None:
-    environ["SIGNAL_ALERT_FROM_PHONE"] = "1234567890"
-    environ["SIGNAL_ALERT_TO_PHONE"] = "0987654321"
-
     mock_socket = mocker.patch("python.signal_alert.socket.socket")
     client = mock_socket.return_value.__enter__.return_value
     client.recv.side_effect = [
         b'{"jsonrpc":"2.0","method":"receive"}\n{"jsonrpc":"2.0","result":{},"id":"signal-alert"}\n'
     ]
 
-    signal_alert("test")
+    assert signal_alert("test", "1234567890", "0987654321") is None
 
     assert client.recv.call_count == 1

tests/test_snapshot_manager.py

@@ -16,6 +16,17 @@ if TYPE_CHECKING:
     from pytest_mock import MockerFixture
 
 SNAPSHOT_MANAGER = "python.tools.snapshot_manager"
+FROM_PHONE = "1234567890"
+TO_PHONE = "0987654321"
+
+
+def patch_phone_numbers(mocker: MockerFixture) -> None:
+    """Patch the Signal phone number environment variables."""
+    phone_numbers = {
+        "SIGNAL_ALERT_FROM_PHONE": FROM_PHONE,
+        "SIGNAL_ALERT_TO_PHONE": TO_PHONE,
+    }
+    mocker.patch(f"{SNAPSHOT_MANAGER}.getenv", side_effect=phone_numbers.get)
 
 
 def patch_utcnow(mocker: MockerFixture, datetime_value: datetime) -> None:
@@ -34,6 +45,7 @@ def create_mock_snapshot(mocker: MockerFixture, name: str) -> Snapshot:
 def test_main(mocker: MockerFixture, fs: FakeFilesystem) -> None:
     """Test main."""
     load_config_data.cache_clear()
+    patch_phone_numbers(mocker)
 
     mocker.patch(f"{SNAPSHOT_MANAGER}.get_time_stamp", return_value="2023-01-01T00:00:00")
 
@@ -58,12 +70,15 @@ def test_main(mocker: MockerFixture, fs: FakeFilesystem) -> None:
             "daily": 0,
             "monthly": 0,
         },
+        FROM_PHONE,
+        TO_PHONE,
     )
 
 
 def test_main_create_snapshot_failure(mocker: MockerFixture, fs: FakeFilesystem) -> None:
     """Test main."""
     load_config_data.cache_clear()
+    patch_phone_numbers(mocker)
 
     mocker.patch(f"{SNAPSHOT_MANAGER}.get_time_stamp", return_value="2023-01-01T00:00:00")
 
@@ -78,7 +93,11 @@ def test_main_create_snapshot_failure(mocker: MockerFixture, fs: FakeFilesystem)
     fs.create_file("/mock_snapshot_config.toml", contents=mock_snapshot_config_toml)
     main(Path("/mock_snapshot_config.toml"))
 
-    mock_signal_alert.assert_called_once_with("test_dataset failed to create snapshot 2023-01-01T00:00:00")
+    mock_signal_alert.assert_called_once_with(
+        "test_dataset failed to create snapshot 2023-01-01T00:00:00",
+        FROM_PHONE,
+        TO_PHONE,
+    )
     mock_get_datasets.assert_called_once()
     mock_get_snapshots_to_delete.assert_not_called()
 
@@ -86,6 +105,7 @@ def test_main_create_snapshot_failure(mocker: MockerFixture, fs: FakeFilesystem)
 def test_main_exception(mocker: MockerFixture, fs: FakeFilesystem) -> None:
     """Test main."""
     load_config_data.cache_clear()
+    patch_phone_numbers(mocker)
 
     mocker.patch(f"{SNAPSHOT_MANAGER}.get_time_stamp", return_value="2023-01-01T00:00:00")
 
@@ -103,7 +123,7 @@ def test_main_exception(mocker: MockerFixture, fs: FakeFilesystem) -> None:
 
     assert isinstance(pytest_wrapped_e.value, SystemExit)
     assert pytest_wrapped_e.value.code == 1
-    mock_signal_alert.assert_called_once_with("snapshot_manager failed")
+    mock_signal_alert.assert_called_once_with("snapshot_manager failed", FROM_PHONE, TO_PHONE)
     mock_get_datasets.assert_called_once()
     mock_get_snapshots_to_delete.assert_not_called()
 
@@ -120,7 +140,12 @@ def test_get_snapshots_to_delete(mocker: MockerFixture) -> None:
 
     mock_signal_alert = mocker.patch(f"{SNAPSHOT_MANAGER}.signal_alert")
 
-    get_snapshots_to_delete(mock_dataset, {"15_min": 1, "hourly": 0, "daily": 0, "monthly": 0})
+    get_snapshots_to_delete(
+        mock_dataset,
+        {"15_min": 1, "hourly": 0, "daily": 0, "monthly": 0},
+        FROM_PHONE,
+        TO_PHONE,
+    )
 
     mock_signal_alert.assert_not_called()
     mock_dataset.delete_snapshot.assert_called_once_with("auto_202509150415")
@@ -135,7 +160,12 @@ def test_get_snapshots_to_delete_no_snapshot(mocker: MockerFixture) -> None:
 
     mock_signal_alert = mocker.patch(f"{SNAPSHOT_MANAGER}.signal_alert")
 
-    get_snapshots_to_delete(mock_dataset, {"15_min": 1, "hourly": 0, "daily": 0, "monthly": 0})
+    get_snapshots_to_delete(
+        mock_dataset,
+        {"15_min": 1, "hourly": 0, "daily": 0, "monthly": 0},
+        FROM_PHONE,
+        TO_PHONE,
+    )
 
     mock_signal_alert.assert_not_called()
     mock_dataset.delete_snapshot.assert_not_called()
@@ -153,10 +183,17 @@ def test_get_snapshots_to_delete_errored(mocker: MockerFixture) -> None:
 
     mock_signal_alert = mocker.patch(f"{SNAPSHOT_MANAGER}.signal_alert")
 
-    get_snapshots_to_delete(mock_dataset, {"15_min": 1, "hourly": 0, "daily": 0, "monthly": 0})
+    get_snapshots_to_delete(
+        mock_dataset,
+        {"15_min": 1, "hourly": 0, "daily": 0, "monthly": 0},
+        FROM_PHONE,
+        TO_PHONE,
+    )
 
     mock_signal_alert.assert_called_once_with(
-        "test_dataset@auto_202509150415 failed to delete: snapshot has dependent clones"
+        "test_dataset@auto_202509150415 failed to delete: snapshot has dependent clones",
+        FROM_PHONE,
+        TO_PHONE,
     )
     mock_dataset.delete_snapshot.assert_called_once_with("auto_202509150415")
 

users/richie/default.nix

@@ -41,6 +41,7 @@ in
         "ollama"
         "plugdev"
         "scanner"
+        "signal-cli"
         "transmission"
         "uaccess"
         "uucp"