Compare commits
4 Commits
fix/issue-
...
fix/744
| Author | SHA1 | Date | |
|---|---|---|---|
| 82926709b8 | |||
| cd0108d853 | |||
| 0b2d67e0c7 | |||
| 5067ff6842 |
109
cron/pending_deliveries.py
Normal file
109
cron/pending_deliveries.py
Normal file
@@ -0,0 +1,109 @@
|
||||
"""
|
||||
Pending Cron Deliveries — Buffer and retry failed deliveries
|
||||
|
||||
When gateway reconnects, in-flight cron job notifications may be lost.
|
||||
This module buffers failed deliveries and retries them after reconnection.
|
||||
|
||||
Issue: #744
|
||||
"""
|
||||
|
||||
import json
|
||||
import logging
|
||||
from pathlib import Path
|
||||
from typing import Any, Dict, List, Optional
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
HERMES_HOME = Path.home() / ".hermes"
|
||||
PENDING_FILE = HERMES_HOME / "cron" / "pending_deliveries.json"
|
||||
|
||||
|
||||
def _load_pending() -> List[Dict[str, Any]]:
|
||||
if not PENDING_FILE.exists():
|
||||
return []
|
||||
try:
|
||||
return json.loads(PENDING_FILE.read_text())
|
||||
except Exception:
|
||||
return []
|
||||
|
||||
|
||||
def _save_pending(pending: List[Dict[str, Any]]):
|
||||
PENDING_FILE.parent.mkdir(parents=True, exist_ok=True)
|
||||
PENDING_FILE.write_text(json.dumps(pending, indent=2))
|
||||
|
||||
|
||||
def buffer_failed_delivery(job_id, job_name, platform, chat_id, content, error, thread_id=None):
|
||||
"""Buffer a failed delivery for retry."""
|
||||
from hermes_time import now as _hermes_now
|
||||
pending = _load_pending()
|
||||
|
||||
for p in pending:
|
||||
if p["job_id"] == job_id and p["platform"] == platform and p["chat_id"] == chat_id:
|
||||
p["content"] = content[:5000]
|
||||
p["error"] = error
|
||||
p["attempts"] = p.get("attempts", 1) + 1
|
||||
p["last_attempt"] = _hermes_now().isoformat()
|
||||
_save_pending(pending)
|
||||
return
|
||||
|
||||
now = _hermes_now().isoformat()
|
||||
pending.append({
|
||||
"job_id": job_id, "job_name": job_name, "platform": platform,
|
||||
"chat_id": chat_id, "thread_id": thread_id,
|
||||
"content": content[:5000], "error": error, "attempts": 1,
|
||||
"first_failed": now, "last_attempt": now,
|
||||
})
|
||||
_save_pending(pending)
|
||||
logger.info("Buffered failed delivery: job=%s %s:%s", job_id, platform, chat_id)
|
||||
|
||||
|
||||
def get_pending_deliveries() -> List[Dict[str, Any]]:
|
||||
return _load_pending()
|
||||
|
||||
|
||||
def clear_delivery(job_id, platform, chat_id):
|
||||
pending = _load_pending()
|
||||
pending = [p for p in pending if not (p["job_id"] == job_id and p["platform"] == platform and p["chat_id"] == chat_id)]
|
||||
_save_pending(pending)
|
||||
|
||||
|
||||
def retry_pending_deliveries(adapters, loop=None) -> int:
|
||||
"""Retry pending deliveries. Returns count of successful retries."""
|
||||
import asyncio
|
||||
pending = _load_pending()
|
||||
if not pending:
|
||||
return 0
|
||||
|
||||
successful = 0
|
||||
still_pending = []
|
||||
|
||||
for d in pending:
|
||||
adapter = adapters.get(d["platform"])
|
||||
if not adapter or not adapter.connected:
|
||||
still_pending.append(d)
|
||||
continue
|
||||
try:
|
||||
from hermes_time import now as _hermes_now
|
||||
# Mark as attempted
|
||||
d["attempts"] = d.get("attempts", 1) + 1
|
||||
d["last_attempt"] = _hermes_now().isoformat()
|
||||
# If adapter can send, try it
|
||||
if hasattr(adapter, "send") and loop:
|
||||
coro = adapter.send(d["chat_id"], d["content"][:4000])
|
||||
fut = asyncio.run_coroutine_threadsafe(coro, loop)
|
||||
result = fut.result(timeout=30)
|
||||
if result and not result.get("error"):
|
||||
successful += 1
|
||||
logger.info("Retry OK: job=%s %s:%s", d["job_id"], d["platform"], d["chat_id"])
|
||||
continue
|
||||
still_pending.append(d)
|
||||
except Exception as e:
|
||||
d["error"] = str(e)
|
||||
still_pending.append(d)
|
||||
|
||||
_save_pending(still_pending)
|
||||
return successful
|
||||
|
||||
|
||||
def get_pending_count() -> int:
|
||||
return len(_load_pending())
|
||||
@@ -967,7 +967,24 @@ def tick(verbose: bool = True, adapters=None, loop=None) -> int:
|
||||
delivery_error = _deliver_result(job, deliver_content, adapters=adapters, loop=loop)
|
||||
except Exception as de:
|
||||
delivery_error = str(de)
|
||||
logger.error("Delivery failed for job %s: %s", job["id"], de)
|
||||
logger.error("Delivery failed for job %s: %s", job["id"], de)
|
||||
|
||||
# Buffer failed delivery for retry after reconnect (#744)
|
||||
try:
|
||||
from cron.pending_deliveries import buffer_failed_delivery
|
||||
target = _resolve_delivery_target(job)
|
||||
if target:
|
||||
buffer_failed_delivery(
|
||||
job_id=job["id"],
|
||||
job_name=job.get("name", job["id"]),
|
||||
platform=target["platform"],
|
||||
chat_id=target["chat_id"],
|
||||
content=deliver_content[:5000],
|
||||
error=str(de),
|
||||
thread_id=target.get("thread_id")
|
||||
)
|
||||
except Exception as _buf_err:
|
||||
logger.debug("Failed to buffer delivery: %s", _buf_err)
|
||||
|
||||
mark_job_run(job["id"], success, error, delivery_error=delivery_error)
|
||||
executed += 1
|
||||
|
||||
@@ -1938,6 +1938,16 @@ class GatewayRunner:
|
||||
error_message=None,
|
||||
)
|
||||
logger.info("✓ %s reconnected successfully", platform.value)
|
||||
|
||||
# Retry pending cron deliveries after reconnect (#744)
|
||||
try:
|
||||
from cron.pending_deliveries import retry_pending_deliveries
|
||||
loop = asyncio.get_event_loop()
|
||||
retried = retry_pending_deliveries(self.adapters, loop=loop)
|
||||
if retried:
|
||||
logger.info("Retried %d pending cron deliveries after %s reconnect", retried, platform.value)
|
||||
except Exception as _retry_err:
|
||||
logger.debug("Pending delivery retry failed: %s", _retry_err)
|
||||
|
||||
# Rebuild channel directory with the new adapter
|
||||
try:
|
||||
|
||||
58
tests/test_pending_deliveries.py
Normal file
58
tests/test_pending_deliveries.py
Normal file
@@ -0,0 +1,58 @@
|
||||
"""
|
||||
Tests for pending delivery buffer
|
||||
|
||||
Issue: #744
|
||||
"""
|
||||
|
||||
import json
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
from cron.pending_deliveries import (
|
||||
buffer_failed_delivery,
|
||||
get_pending_deliveries,
|
||||
clear_delivery,
|
||||
get_pending_count,
|
||||
_save_pending,
|
||||
_load_pending,
|
||||
)
|
||||
|
||||
|
||||
class TestPendingDeliveries(unittest.TestCase):
|
||||
|
||||
def setUp(self):
|
||||
self.tmp = tempfile.mkdtemp()
|
||||
self.patch_path = patch("cron.pending_deliveries.PENDING_FILE",
|
||||
Path(self.tmp) / "pending.json")
|
||||
self.patch_path.start()
|
||||
|
||||
def tearDown(self):
|
||||
self.patch_path.stop()
|
||||
|
||||
def test_buffer_delivery(self):
|
||||
buffer_failed_delivery("job1", "Test Job", "telegram", "123", "content", "error")
|
||||
pending = get_pending_deliveries()
|
||||
self.assertEqual(len(pending), 1)
|
||||
self.assertEqual(pending[0]["job_id"], "job1")
|
||||
self.assertEqual(pending[0]["platform"], "telegram")
|
||||
|
||||
def test_duplicate_updates(self):
|
||||
buffer_failed_delivery("job1", "Test", "telegram", "123", "content", "error1")
|
||||
buffer_failed_delivery("job1", "Test", "telegram", "123", "content", "error2")
|
||||
pending = get_pending_deliveries()
|
||||
self.assertEqual(len(pending), 1)
|
||||
self.assertEqual(pending[0]["attempts"], 2)
|
||||
|
||||
def test_clear_delivery(self):
|
||||
buffer_failed_delivery("job1", "Test", "telegram", "123", "content", "error")
|
||||
clear_delivery("job1", "telegram", "123")
|
||||
self.assertEqual(get_pending_count(), 0)
|
||||
|
||||
def test_empty_returns_zero(self):
|
||||
self.assertEqual(get_pending_count(), 0)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user