fix(cron): configurable media-send timeout + non-empty failure reasons (salvages #87965, #87967) - #88787
Merged
Merged
Conversation
TimeoutError carries no message and str(TimeoutError()) is the empty string, so the media-send warning rendered with nothing after the colon. Fall back to the exception class name when str(e) is empty.
The media delivery path used a hardcoded future.result(timeout=30). Large attachments legitimately exceed it with no way to raise the limit. Read HERMES_CRON_MEDIA_SEND_TIMEOUT, matching the existing HERMES_CRON_SCRIPT_TIMEOUT / HERMES_CRON_TIMEOUT / HERMES_CRON_SESSION_DB_TIMEOUT convention in the same module.
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.
Contributor
૮ >ﻌ< ა ci reviewran on 72c8fa5 — feat(cron): configurable media-send timeout + non-empty fail
|
alt-glitch
added
type/bug
This was referenced Aug 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cron media attachment sends now have a configurable timeout (env var → config.yaml → 300s default) and always record a non-empty failure reason — a timed-out attachment used to log "failed to send media : " with nothing after the colon and no way to raise the fixed 30s window.
Salvages PR #87965 and PR #87967 by @AiwendilInTheWoods (authorship preserved) onto current main, plus a follow-up commit integrating both with the post-#88631 delivery-error surfacing.
Changes
cron/scheduler.py:_send_media_via_adapterbounds each send with_get_media_send_timeout()— resolution mirrorsscript_timeout_seconds:HERMES_CRON_MEDIA_SEND_TIMEOUT→cron.media_send_timeout_seconds→ 300s. Argument-less exceptions (TimeoutError) fall back to the class name in both the log line and the delivery error recorded on the run.hermes_cli/config_defaults.py:cron.media_send_timeout_seconds: 300registered.tests/cron/test_media_send_timeout.py: resolution precedence (env wins, config, invalid-input fallbacks) + empty-reason fallback (TimeoutError names its class, real messages preserved).Validation
Infographic