From f165f8f331bc5fcd3702bda84d365f53ea50f0b2 Mon Sep 17 00:00:00 2001 From: Richie Cahill Date: Wed, 30 Sep 2026 07:12:46 -0400 Subject: [PATCH] Update Signal alert handling --- python/signal_alert.py | 46 +++++++++++++++++------------- python/tools/snapshot_manager.py | 22 +++++++++++--- tests/test_signal_alert.py | 46 +++++++----------------------- tests/test_snapshot_manager.py | 49 ++++++++++++++++++++++++++++---- users/richie/default.nix | 1 + 5 files changed, 99 insertions(+), 65 deletions(-) diff --git a/python/signal_alert.py b/python/signal_alert.py index c5723d1..c66dc77 100644 --- a/python/signal_alert.py +++ b/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: - """Send a signal alert. +class SignalRPCError(RuntimeError): + """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. - """ - from_phone = getenv("SIGNAL_ALERT_FROM_PHONE") - to_phone = getenv("SIGNAL_ALERT_TO_PHONE") - if not from_phone or not to_phone: - logger.info("SIGNAL_ALERT_FROM_PHONE or SIGNAL_ALERT_TO_PHONE not set") - return + body: The body of the alert. + from_phone: The Signal account sending the alert. + to_phone: The Signal account receiving the alert. + timeout: Seconds to wait on each socket operation. + 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 error := response.get("error"): - logger.error("Signal RPC returned an error: %s", error) - return - - logger.error("Signal RPC socket closed before returning a response") - except OSError, json.JSONDecodeError: - logger.exception("Unable to communicate with signal-cli") + if response.get("id") != signal_rpc_id: + continue + if rpc_error := response.get("error"): + error = f"Signal RPC returned an error: {rpc_error}" + raise SignalRPCError(error) + return + error = "Signal RPC socket closed before returning a response" + raise SignalRPCError(error) + except (OSError, json.JSONDecodeError) as exc: + error = f"Signal RPC failed: {exc!r}" + raise SignalRPCError(error) from exc diff --git a/python/tools/snapshot_manager.py b/python/tools/snapshot_manager.py index 866c6a5..90eb478 100644 --- a/python/tools/snapshot_manager.py +++ b/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) diff --git a/tests/test_signal_alert.py b/tests/test_signal_alert.py index 8b78777..7b92631 100644 --- a/tests/test_signal_alert.py +++ b/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") - - mock_logger.error.assert_called_once_with( - "Signal RPC returned an error: %s", - {"code": -1, "message": "failed"}, - ) + with pytest.raises(SignalRPCError, match=r"Signal RPC returned an error:.*failed"): + signal_alert("test", "1234567890", "0987654321") 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 diff --git a/tests/test_snapshot_manager.py b/tests/test_snapshot_manager.py index a849829..30f6958 100644 --- a/tests/test_snapshot_manager.py +++ b/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") diff --git a/users/richie/default.nix b/users/richie/default.nix index c8368ff..787a500 100644 --- a/users/richie/default.nix +++ b/users/richie/default.nix @@ -41,6 +41,7 @@ in "ollama" "plugdev" "scanner" + "signal-cli" "transmission" "uaccess" "uucp"