Merge PR #716: fix: log exceptions instead of silently swallowing in cron scheduler
Authored by 0xbyt4. Replaces two except-Exception-pass blocks with logger.warning() calls and adds tests for both paths.
This commit is contained in:
@@ -138,8 +138,8 @@ def _deliver_result(job: dict, content: str) -> None:
|
||||
try:
|
||||
from gateway.mirror import mirror_to_session
|
||||
mirror_to_session(platform_name, chat_id, content, source_label="cron")
|
||||
except Exception:
|
||||
pass
|
||||
except Exception as e:
|
||||
logger.warning("Job '%s': mirror_to_session failed: %s", job["id"], e)
|
||||
|
||||
|
||||
def run_job(job: dict) -> tuple[bool, str, str, Optional[str]]:
|
||||
@@ -190,8 +190,8 @@ def run_job(job: dict) -> tuple[bool, str, str, Optional[str]]:
|
||||
model = _model_cfg
|
||||
elif isinstance(_model_cfg, dict):
|
||||
model = _model_cfg.get("default", model)
|
||||
except Exception:
|
||||
pass
|
||||
except Exception as e:
|
||||
logger.warning("Job '%s': failed to load config.yaml, using defaults: %s", job_id, e)
|
||||
|
||||
# Reasoning config from env or config.yaml
|
||||
reasoning_config = None
|
||||
|
||||
@@ -1,8 +1,12 @@
|
||||
"""Tests for cron/scheduler.py — origin resolution and delivery routing."""
|
||||
"""Tests for cron/scheduler.py — origin resolution, delivery routing, and error logging."""
|
||||
|
||||
import asyncio
|
||||
import logging
|
||||
from unittest.mock import patch, MagicMock, AsyncMock
|
||||
|
||||
import pytest
|
||||
|
||||
from cron.scheduler import _resolve_origin
|
||||
from cron.scheduler import _resolve_origin, _deliver_result, run_job
|
||||
|
||||
|
||||
class TestResolveOrigin:
|
||||
@@ -36,3 +40,60 @@ class TestResolveOrigin:
|
||||
def test_empty_origin(self):
|
||||
job = {"origin": {}}
|
||||
assert _resolve_origin(job) is None
|
||||
|
||||
|
||||
class TestDeliverResultMirrorLogging:
|
||||
"""Verify that mirror_to_session failures are logged, not silently swallowed."""
|
||||
|
||||
def test_mirror_failure_is_logged(self, caplog):
|
||||
"""When mirror_to_session raises, a warning should be logged."""
|
||||
from gateway.config import Platform
|
||||
|
||||
pconfig = MagicMock()
|
||||
pconfig.enabled = True
|
||||
mock_cfg = MagicMock()
|
||||
mock_cfg.platforms = {Platform.TELEGRAM: pconfig}
|
||||
|
||||
with patch("gateway.config.load_gateway_config", return_value=mock_cfg), \
|
||||
patch("asyncio.run", return_value=None), \
|
||||
patch("gateway.mirror.mirror_to_session", side_effect=ConnectionError("network down")):
|
||||
job = {
|
||||
"id": "test-job",
|
||||
"deliver": "origin",
|
||||
"origin": {"platform": "telegram", "chat_id": "123"},
|
||||
}
|
||||
with caplog.at_level(logging.WARNING, logger="cron.scheduler"):
|
||||
_deliver_result(job, "Hello!")
|
||||
|
||||
assert any("mirror_to_session failed" in r.message for r in caplog.records), \
|
||||
f"Expected 'mirror_to_session failed' warning in logs, got: {[r.message for r in caplog.records]}"
|
||||
|
||||
|
||||
class TestRunJobConfigLogging:
|
||||
"""Verify that config.yaml parse failures are logged, not silently swallowed."""
|
||||
|
||||
def test_bad_config_yaml_is_logged(self, caplog, tmp_path):
|
||||
"""When config.yaml is malformed, a warning should be logged."""
|
||||
# Create a bad config.yaml
|
||||
bad_yaml = tmp_path / "config.yaml"
|
||||
bad_yaml.write_text("invalid: yaml: [[[bad")
|
||||
|
||||
job = {
|
||||
"id": "test-job",
|
||||
"name": "test",
|
||||
"prompt": "hello",
|
||||
}
|
||||
|
||||
with patch("cron.scheduler._hermes_home", tmp_path), \
|
||||
patch("cron.scheduler._resolve_origin", return_value=None), \
|
||||
patch("dotenv.load_dotenv"), \
|
||||
patch("run_agent.AIAgent") as mock_agent_cls:
|
||||
mock_agent = MagicMock()
|
||||
mock_agent.run.return_value = ("output doc", "final response")
|
||||
mock_agent_cls.return_value = mock_agent
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="cron.scheduler"):
|
||||
run_job(job)
|
||||
|
||||
assert any("failed to load config.yaml" in r.message for r in caplog.records), \
|
||||
f"Expected 'failed to load config.yaml' warning in logs, got: {[r.message for r in caplog.records]}"
|
||||
|
||||
Reference in New Issue
Block a user