feat(cron): configurable media-send timeout + non-empty failure reasons
Follow-up on the salvaged commits from PRs #87965 and #87967 (@AiwendilInTheWoods): - Promote the media-send timeout to the standard resolution pattern: HERMES_CRON_MEDIA_SEND_TIMEOUT env var, then cron.media_send_timeout_seconds in config.yaml, then 300s default (mirrors script_timeout_seconds; .env stays secrets-only). - Register the config key in DEFAULT_CONFIG and document both surfaces (environment-variables reference + cron user guide). - Fold the empty-str() exception fallback into the error string recorded in delivery_errors (post-#88631 the reason reaches the run status, not just the log line). - Tests: timeout resolution precedence + TimeoutError reason fallback.
This commit is contained in:
@@ -2404,10 +2404,10 @@ def _send_media_via_adapter(
|
||||
try:
|
||||
# Large attachments (long TTS audio, concatenated recordings,
|
||||
# big exports) can legitimately exceed a fixed 30s upload
|
||||
# window. Configurable, matching the other cron timeouts.
|
||||
result = future.result(
|
||||
timeout=int(os.getenv("HERMES_CRON_MEDIA_SEND_TIMEOUT", "300"))
|
||||
)
|
||||
# window. Configurable, matching the other cron timeouts
|
||||
# (cron.media_send_timeout_seconds in config.yaml, or the
|
||||
# HERMES_CRON_MEDIA_SEND_TIMEOUT env override).
|
||||
result = future.result(timeout=_get_media_send_timeout())
|
||||
except TimeoutError:
|
||||
future.cancel()
|
||||
raise
|
||||
@@ -3256,6 +3256,44 @@ def _get_script_timeout() -> int:
|
||||
return _DEFAULT_SCRIPT_TIMEOUT
|
||||
|
||||
|
||||
_DEFAULT_MEDIA_SEND_TIMEOUT = 300
|
||||
|
||||
|
||||
def _get_media_send_timeout() -> int:
|
||||
"""Resolve the per-attachment media-send timeout from env/config.
|
||||
|
||||
Mirrors the ``script_timeout_seconds`` resolution pattern: the
|
||||
HERMES_CRON_MEDIA_SEND_TIMEOUT env var wins, then
|
||||
``cron.media_send_timeout_seconds`` in config.yaml, then the default
|
||||
(300s — large attachments like long TTS audio can legitimately exceed
|
||||
the old fixed 30s upload window).
|
||||
"""
|
||||
env_value = os.getenv("HERMES_CRON_MEDIA_SEND_TIMEOUT", "").strip()
|
||||
if env_value:
|
||||
try:
|
||||
timeout = int(float(env_value))
|
||||
if timeout > 0:
|
||||
return timeout
|
||||
except Exception:
|
||||
logger.warning(
|
||||
"Invalid HERMES_CRON_MEDIA_SEND_TIMEOUT=%r; using config/default",
|
||||
env_value,
|
||||
)
|
||||
|
||||
try:
|
||||
cfg = load_config() or {}
|
||||
cron_cfg = cfg.get("cron", {}) if isinstance(cfg, dict) else {}
|
||||
configured = cron_cfg.get("media_send_timeout_seconds")
|
||||
if configured is not None:
|
||||
timeout = int(float(configured))
|
||||
if timeout > 0:
|
||||
return timeout
|
||||
except Exception as exc:
|
||||
logger.debug("Failed to load cron media-send timeout from config: %s", exc)
|
||||
|
||||
return _DEFAULT_MEDIA_SEND_TIMEOUT
|
||||
|
||||
|
||||
def _read_windows_pyvenv_cfg(venv_dir: Path) -> dict[str, str]:
|
||||
cfg_path = venv_dir / "pyvenv.cfg"
|
||||
try:
|
||||
|
||||
@@ -2502,6 +2502,12 @@ DEFAULT_CONFIG = {
|
||||
# wedges the job's dispatch guard forever. Also overridable via
|
||||
# HERMES_CRON_SESSION_DB_TIMEOUT env var. 0 = unlimited (skip the bound).
|
||||
"session_db_timeout_seconds": 10,
|
||||
# Timeout (seconds) for each media attachment send during cron
|
||||
# delivery via a live gateway adapter. Large attachments (long TTS
|
||||
# audio, big exports) can exceed the old fixed 30s window. Also
|
||||
# overridable via HERMES_CRON_MEDIA_SEND_TIMEOUT env var. Keep in
|
||||
# sync with cron.scheduler._DEFAULT_MEDIA_SEND_TIMEOUT.
|
||||
"media_send_timeout_seconds": 300,
|
||||
},
|
||||
|
||||
# Kanban multi-agent coordination — controls the dispatcher loop that
|
||||
|
||||
98
tests/cron/test_media_send_timeout.py
Normal file
98
tests/cron/test_media_send_timeout.py
Normal file
@@ -0,0 +1,98 @@
|
||||
"""Cron media-send timeout resolution and failure-reason formatting.
|
||||
|
||||
Covers two salvaged fixes:
|
||||
|
||||
- PR #87965 (@AiwendilInTheWoods): an argument-less exception (notably
|
||||
TimeoutError from ``future.result(timeout=...)``) has an empty ``str()``,
|
||||
which used to render "failed to send media <path>: " with no reason at
|
||||
all — in both the log line and the delivery error recorded on the run.
|
||||
- PR #87967 (@AiwendilInTheWoods): the per-attachment send timeout was a
|
||||
hardcoded 30s; large attachments (long TTS audio, big exports) failed on
|
||||
slow uplinks with no way to raise it. Now resolved via
|
||||
HERMES_CRON_MEDIA_SEND_TIMEOUT → cron.media_send_timeout_seconds → 300s.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
from cron.scheduler import (
|
||||
_DEFAULT_MEDIA_SEND_TIMEOUT,
|
||||
_get_media_send_timeout,
|
||||
_send_media_via_adapter,
|
||||
)
|
||||
|
||||
|
||||
class TestMediaSendTimeoutResolution:
|
||||
def test_default(self, monkeypatch):
|
||||
monkeypatch.delenv("HERMES_CRON_MEDIA_SEND_TIMEOUT", raising=False)
|
||||
monkeypatch.setattr("cron.scheduler.load_config", lambda: {})
|
||||
assert _get_media_send_timeout() == _DEFAULT_MEDIA_SEND_TIMEOUT == 300
|
||||
|
||||
def test_env_wins(self, monkeypatch):
|
||||
monkeypatch.setenv("HERMES_CRON_MEDIA_SEND_TIMEOUT", "45")
|
||||
monkeypatch.setattr(
|
||||
"cron.scheduler.load_config",
|
||||
lambda: {"cron": {"media_send_timeout_seconds": 900}},
|
||||
)
|
||||
assert _get_media_send_timeout() == 45
|
||||
|
||||
def test_config_value(self, monkeypatch):
|
||||
monkeypatch.delenv("HERMES_CRON_MEDIA_SEND_TIMEOUT", raising=False)
|
||||
monkeypatch.setattr(
|
||||
"cron.scheduler.load_config",
|
||||
lambda: {"cron": {"media_send_timeout_seconds": 900}},
|
||||
)
|
||||
assert _get_media_send_timeout() == 900
|
||||
|
||||
@pytest.mark.parametrize("bad", ["abc", "-5", "0", ""])
|
||||
def test_invalid_env_falls_back(self, monkeypatch, bad):
|
||||
monkeypatch.setenv("HERMES_CRON_MEDIA_SEND_TIMEOUT", bad)
|
||||
monkeypatch.setattr("cron.scheduler.load_config", lambda: {})
|
||||
assert _get_media_send_timeout() == _DEFAULT_MEDIA_SEND_TIMEOUT
|
||||
|
||||
def test_invalid_config_falls_back(self, monkeypatch):
|
||||
monkeypatch.delenv("HERMES_CRON_MEDIA_SEND_TIMEOUT", raising=False)
|
||||
monkeypatch.setattr(
|
||||
"cron.scheduler.load_config",
|
||||
lambda: {"cron": {"media_send_timeout_seconds": "nope"}},
|
||||
)
|
||||
assert _get_media_send_timeout() == _DEFAULT_MEDIA_SEND_TIMEOUT
|
||||
|
||||
|
||||
class TestEmptyReasonFallback:
|
||||
def _run(self, tmp_path, monkeypatch, exc):
|
||||
"""Drive _send_media_via_adapter into its generic except handler."""
|
||||
media = tmp_path / "clip.mp3"
|
||||
media.write_bytes(b"x")
|
||||
|
||||
monkeypatch.setattr(
|
||||
"gateway.platforms.base.BasePlatformAdapter.filter_media_delivery_paths",
|
||||
staticmethod(lambda files: [(str(media), False)]),
|
||||
)
|
||||
|
||||
def boom(coro, loop):
|
||||
coro.close()
|
||||
raise exc
|
||||
|
||||
monkeypatch.setattr("agent.async_utils.safe_schedule_threadsafe", boom)
|
||||
|
||||
class _Adapter:
|
||||
async def send_voice(self, **kw): # pragma: no cover - never awaited
|
||||
pass
|
||||
|
||||
errors = _send_media_via_adapter(
|
||||
_Adapter(), "C123", [(str(media), False)], None, loop=object(),
|
||||
job={"id": "job-x"},
|
||||
)
|
||||
assert len(errors) == 1
|
||||
return errors[0]
|
||||
|
||||
def test_timeout_error_names_the_class(self, tmp_path, monkeypatch):
|
||||
# TimeoutError() has an empty str() — the recorded reason must not
|
||||
# be blank (the trailing-colon-nothing log from the field report).
|
||||
err = self._run(tmp_path, monkeypatch, TimeoutError())
|
||||
assert err.rstrip() != f"failed to send media {tmp_path / 'clip.mp3'}:"
|
||||
assert "TimeoutError" in err
|
||||
|
||||
def test_exception_with_message_keeps_it(self, tmp_path, monkeypatch):
|
||||
err = self._run(tmp_path, monkeypatch, RuntimeError("bridge closed"))
|
||||
assert "bridge closed" in err
|
||||
@@ -775,6 +775,7 @@ Advanced per-platform knobs for throttling the outbound message batcher. Most us
|
||||
| `HERMES_FILE_MUTATION_VERIFIER` | Enable the per-turn file-mutation verifier footer (default: `true`). When enabled, Hermes appends an advisory listing any `write_file` / `patch` calls that failed during the turn and were not superseded by a successful write. Set to `0`, `false`, `no`, or `off` to suppress. Mirrors `display.file_mutation_verifier` in `config.yaml`; the env var wins when set. |
|
||||
| `HERMES_CRON_TIMEOUT` | Inactivity timeout for cron job agent runs in seconds (default: `600`). The agent can run indefinitely while actively calling tools or receiving stream tokens — this only triggers when idle. Set to `0` for unlimited. |
|
||||
| `HERMES_CRON_SCRIPT_TIMEOUT` | Timeout for pre-run scripts attached to cron jobs in seconds (default: `3600`). Bounds the script only — skill/agent jobs use the separate `HERMES_CRON_TIMEOUT` inactivity budget. Also configurable via `cron.script_timeout_seconds` in `config.yaml`. |
|
||||
| `HERMES_CRON_MEDIA_SEND_TIMEOUT` | Timeout for each media attachment send during cron delivery via a live gateway adapter, in seconds (default: `300`). Raise it if large attachments (long TTS audio, big exports) time out during upload. Also configurable via `cron.media_send_timeout_seconds` in `config.yaml`. |
|
||||
| `HERMES_CRON_MAX_PARALLEL` | Max cron jobs run in parallel per tick (default: `4`). |
|
||||
|
||||
## Agent Behavior
|
||||
|
||||
@@ -541,6 +541,18 @@ cron:
|
||||
|
||||
Set `cleanup_timeout_seconds: 0` only to restore the legacy unbounded cleanup behavior.
|
||||
|
||||
## Media send timeout
|
||||
|
||||
When a cron delivery includes media attachments (a generated PDF, TTS audio, an exported report) sent through a live gateway adapter, each attachment upload is bounded by a timeout — 300 seconds by default. Large files on slow uplinks can need more:
|
||||
|
||||
```yaml
|
||||
# ~/.hermes/config.yaml
|
||||
cron:
|
||||
media_send_timeout_seconds: 600 # 10 minutes per attachment
|
||||
```
|
||||
|
||||
Or set the `HERMES_CRON_MEDIA_SEND_TIMEOUT` environment variable. The resolution order is: env var → config.yaml → 300s default. A timed-out attachment is recorded in the job's run status as a partial delivery failure (the text still delivers).
|
||||
|
||||
## No-agent mode (script-only jobs)
|
||||
|
||||
For recurring jobs that don't need LLM reasoning — classic watchdogs, disk/memory alerts, heartbeats, CI pings — pass `no_agent=True` at creation time. The scheduler runs your script on schedule and delivers its stdout directly, skipping the agent entirely:
|
||||
|
||||
Reference in New Issue
Block a user