feat(webhooks): retry delivery, with backoff and surface failures

- Refactor `send_webhook_notification` to retry failures with exponential backoff, add a request timeout, and raise `WebhookDeliveryError` on terminal failure instead of accepting it. - Results are persisted so a delivery failure can be surfaced without losing the result.

Closes #180

Author: alexhambley

app/utils/webhook_utils.py

@@ -1,29 +1,62 @@
-"""Utility methods for sending webhook notifications."""
-
-# Author: Alexander Hambley
-# License: MIT
-# Copyright (c) 2025 eScience Lab, The University of Manchester
+"""Webhook delivery with bounded retries and backoff."""
 
 import logging
-import requests
+import time
 
-from typing import Any
+from typing import Any, Callable
+
+import requests
 
 logger = logging.getLogger(__name__)
 
+DEFAULT_TIMEOUT = 10
 
-def send_webhook_notification(url: str, data: Any) -> None:
-    """
-    Sends a POST request to the specified webhook URL with the given data.
 
-    :param url: The URL to send the webhook notification to.
-    :param data: The data to send in the POST request.
-    :raises requests.RequestException: If an error occurs when sending the notification.
-    """
+class WebhookDeliveryError(Exception):
+    """Raised when a webhook could not be delivered after all retries."""
+
 
-    try:
-        response = requests.post(url, json=data)
-        response.raise_for_status()
-        logging.info(f"Webhook notification sent successfully to {url}")
-    except requests.RequestException as e:
-        logging.error(f"Failed to send webhook notification: {e}")
+def send_webhook_notification(
+    url: str,
+    data: Any,
+    max_attempts: int = 3,
+    base_delay: float = 0.5,
+    sleep: Callable[[float], None] = time.sleep,
+) -> None:
+    """
+    POST ``data`` to ``url`` as JSON, retrying transient failures.
+
+    Retries up to ``max_attempts`` times with exponential backoff. On final
+    failure it raises :class:`WebhookDeliveryError` rather than swallowing the
+    error, so the caller can surface it.
+
+    :param url: The webhook URL to POST to.
+    :param data: JSON-serialisable payload.
+    :param max_attempts: Total number of attempts before giving up.
+    :param base_delay: Base backoff delay in seconds (doubled each retry).
+    :param sleep: Sleep function (injectable for testing).
+    :raises WebhookDeliveryError: If delivery fails after ``max_attempts``.
+    """
+    last_error = None
+
+    for attempt in range(1, max_attempts + 1):
+        try:
+            response = requests.post(url, json=data, timeout=DEFAULT_TIMEOUT)
+            response.raise_for_status()
+            logger.info("Webhook delivered to %s (attempt %d)", url, attempt)
+            return
+        except requests.RequestException as error:
+            last_error = error
+            logger.warning(
+                "Webhook attempt %d/%d to %s failed: %s",
+                attempt,
+                max_attempts,
+                url,
+                error,
+            )
+            if attempt < max_attempts:
+                sleep(base_delay * (2 ** (attempt - 1)))
+
+    raise WebhookDeliveryError(
+        f"Failed to deliver webhook to {url} after {max_attempts} attempts: {last_error}"
+    )

tests/test_validation_tasks.py

@@ -11,6 +11,7 @@
 from app.tasks import validation_tasks
 from app.tasks.validation_tasks import run_validation_job
 from app.utils.config import Settings
+from app.utils.webhook_utils import WebhookDeliveryError
 from app.validation.results import ValidationOutcome, ValidationStatus
 
 RUNNER = "app.tasks.validation_tasks.validate_crate_path"
@@ -107,6 +108,23 @@ def get_bytes(self, key):
             run_validation_job(storage, "foo", _settings(), created_at="t")
 
 
+def test_webhook_failure_surfaces_but_result_is_already_persisted(storage):
+    """A terminal webhook failure propagates, yet the outcome was persisted first."""
+    storage.put_bytes("crates/foo.zip", b"PK")
+
+    with mock.patch(RUNNER) as run, mock.patch(WEBHOOK) as hook:
+        run.return_value = ValidationOutcome(status=ValidationStatus.VALID, created_at="t")
+        hook.side_effect = WebhookDeliveryError("gave up")
+
+        with pytest.raises(WebhookDeliveryError):
+            run_validation_job(
+                storage, "foo", _settings(), webhook_url="https://hook", created_at="t"
+            )
+
+    # Persisted before the webhook was attempted, so GET still works.
+    assert _stored_outcome(storage, "foo")["status"] == "valid"
+
+
 def test_created_at_is_persisted(storage):
     storage.put_bytes("crates/foo.zip", b"PK")
     with mock.patch(RUNNER) as run, mock.patch(WEBHOOK):

tests/test_webhooks.py

@@ -0,0 +1,58 @@
+"""Tests for webhook delivery with retry/backoff."""
+
+from unittest import mock
+
+import pytest
+import requests
+
+from app.utils import webhook_utils
+from app.utils.webhook_utils import send_webhook_notification, WebhookDeliveryError
+
+
+def _ok_response():
+    response = mock.Mock()
+    response.raise_for_status.return_value = None
+    return response
+
+
+def test_successful_delivery_posts_once():
+    with mock.patch.object(webhook_utils.requests, "post", return_value=_ok_response()) as post:
+        send_webhook_notification("https://hook", {"status": "valid"}, sleep=lambda _: None)
+    post.assert_called_once()
+    # The payload is sent as JSON and a timeout is set (no unbounded hang).
+    assert post.call_args.kwargs["json"] == {"status": "valid"}
+    assert "timeout" in post.call_args.kwargs
+
+
+def test_retries_then_succeeds():
+    flaky = [requests.ConnectionError("boom"), requests.ConnectionError("boom"), _ok_response()]
+    sleeps = []
+    with mock.patch.object(webhook_utils.requests, "post", side_effect=flaky) as post:
+        send_webhook_notification(
+            "https://hook", {"x": 1}, max_attempts=3, sleep=sleeps.append
+        )
+    assert post.call_count == 3
+    assert len(sleeps) == 2  # slept between the three attempts
+
+
+def test_terminal_failure_raises_after_exhausting_attempts():
+    with mock.patch.object(
+        webhook_utils.requests, "post", side_effect=requests.ConnectionError("down")
+    ) as post:
+        with pytest.raises(WebhookDeliveryError) as exc_info:
+            send_webhook_notification(
+                "https://hook", {"x": 1}, max_attempts=3, sleep=lambda _: None
+            )
+    assert post.call_count == 3
+    assert "https://hook" in str(exc_info.value)
+
+
+def test_http_error_status_is_retried():
+    bad = mock.Mock()
+    bad.raise_for_status.side_effect = requests.HTTPError("500")
+    with mock.patch.object(webhook_utils.requests, "post", return_value=bad) as post:
+        with pytest.raises(WebhookDeliveryError):
+            send_webhook_notification(
+                "https://hook", {"x": 1}, max_attempts=2, sleep=lambda _: None
+            )
+    assert post.call_count == 2