fix(gateway): retry restart notification when Telegram send path is degraded - #72009
fix(gateway): retry restart notification when Telegram send path is degraded#72009kvadyan wants to merge 1 commit into
Conversation
…egraded On startup the Telegram adapter gates send() behind _send_path_degraded until the first successful getUpdates response. Lifecycle notifications fired right after connect race this gate and are dropped locally with 'send_path_degraded'; the .restart_notify.json marker is unlinked in finally, so the notification is lost permanently. With long polling the gate only opens on a getUpdates *response*, which on a quiet chat can take tens of seconds (observed >30s in production: the flag cleared only when the user sent a message). A fixed startup delay cannot cover this without blocking the whole startup sequence. Two-part fix: - _wait_for_send_paths_ready(timeout=3.0): short pre-notification wait covering the fast case (pending updates flush immediately). Kept short on purpose - it blocks the startup sequence. - _retry_restart_notification(attempts=6, interval=10s): background retry registered in self._background_tasks when send() reports send_path_degraded, covering slow long-poll openings. The adapter health gate itself is untouched - it protects against wedged PTB/httpx states after reconnect storms. Reproduced on hermes-gateway.service: restart at 13:52:19, first getUpdates response >30s later, notification lost. After the fix: 'Sent restart notification to telegram:<chat> (retry 4)'.
alt-glitch
added
type/bug
Duplicate of #66598: both retain the restart lifecycle marker after a Telegram send_path_degraded refusal and retry once polling readiness returns. #66598 is the earlier, broader implementation with focused coverage for both requester and home-channel notification paths. |
|
Production verification report Applied this PR's patch on top of Hermes v0.19.0 (commit Setup:
After applying this PR:
This is a clean, additive fix for a real production failure mode. Sharing in case it helps with review prioritization. |
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting a verified Telegram delivery race. The current-main path still sends during the degraded window and then deletes the requester marker (gateway/run.py:20123-20152); Telegram explicitly refuses that send while _send_path_degraded is true (plugins/platforms/telegram/adapter.py:4332-4334).
Problems
- The new retry task is not durable: PR-head
gateway/run.py:17154schedules it, but the existingfinallydeletes.restart_notify.jsonat line 17177. Current shutdown cancels tracked_background_tasks(gateway/run.py:12230-12240), so a shutdown before retry leaves no record to redeliver. - The same degraded-refusal/cleanup pattern remains for planned-restart home-channel notices (
gateway/run.py:10946-10952,20212-20219). This matches the broader requester-and-home-channel scope noted on #66598. - PR-head
gateway/run.py:8354waits unconditionally and the diff adds no regression tests.
Suggested changes
- Retain lifecycle markers only for retryable
send_path_degradedrefusals, then use a bounded deferred retry with explicit terminal cleanup. - Cover both requester and planned home-channel lifecycle paths, and make readiness waiting conditional on a pending marker.
- Add focused tests for retained markers, recovery delivery, terminal cleanup, and shutdown cancellation.
Automated hermes-sweeper review.
| # Long-poll health gate can stay closed for tens of | ||
| # seconds on a quiet chat — retry in the background | ||
| # instead of dropping the notification on the floor. | ||
| task = asyncio.create_task( |
There was a problem hiding this comment.
This retry is not durable: the existing finally below still unlinks .restart_notify.json, while current shutdown cancels every task in _background_tasks (gateway/run.py:12230-12240). Retain the marker for this retryable refusal and clear it only after a successful or explicitly terminal retry outcome.
| # blocks the startup sequence, and slow cases (long-poll hanging | ||
| # on a quiet chat) are covered by the background retry in | ||
| # _send_restart_notification(). | ||
| await self._wait_for_send_paths_ready(timeout=3.0) |
There was a problem hiding this comment.
Please gate this readiness wait on an actual pending lifecycle marker. As written, every startup may wait up to three seconds whenever an adapter exposes a degraded flag, even when there is no restart/startup notification to deliver.
teknium1
added
the
sweeper:blast-moderate
4 participants
Bug Description
After
/restart, the "Gateway restarted successfully" notification is silently lost when the Telegram adapter is in polling mode.journalctlshows:The
.restart_notify.jsonmarker is unlinked infinally, so there is no retry — the notification is gone permanently.Root Cause
The Telegram adapter gates
send()behind_send_path_degradeduntil the first successfulgetUpdatesresponse (_record_polling_progress). The gateway fires lifecycle notifications ~1s after connect, racing this gate.Critical detail: with long polling the gate only opens when a
getUpdatesrequest returns. On a quiet chat the poll hangs until an update arrives — measured in production: restart at 13:52:19, gate still closed 30s+ later, opened only when the user sent a message. So no fixed startup delay can reliably cover this, and a long blocking wait would stall the whole startup sequence (housekeeping, message intake).Fix
Two-part, in
gateway/run.py:_wait_for_send_paths_ready(timeout=3.0)— short pre-notification wait polling adapters'_send_path_degradedflag (duck-typed; adapters without the gate pass instantly). Covers the fast case where pending updates flush immediately. Kept deliberately short because it blocks startup._retry_restart_notification(attempts=6, interval=10s)— whensend()still reportssend_path_degraded, a background task (registered inself._background_tasks, so it is cancelled cleanly on shutdown) retries until the gate opens.The adapter health gate itself is untouched — it protects against wedged PTB/httpx states after reconnect storms.
How to Verify
/restartwas not delivered: send_path_degraded, no message ever arrivesSent restart notification to telegram:<chat> (retry N)in the log, message arrives within ~10–60s of bootTest Plan
python3 -m py_compile gateway/run.pyRisk Assessment
Low — additive: one short wait + one background retry task. Failure mode on timeout is identical to current behavior (warning logged, send attempted anyway). No changes to the adapter or the health gate.