Notify Telegram on runtime entry failures - #128
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2330665a74
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return False | ||
| message = _runtime_error_notification_message(exc, route_label=route_label) | ||
| for token, chat_id in targets: | ||
| send_telegram_message( |
There was a problem hiding this comment.
When a service relies on CRISIS_ALERT_TELEGRAM_API_BASE_URL (the workflow syncs it into Cloud Run in .github/workflows/sync-cloud-run-env.yml:767-768), this fallback bypasses that setting and calls send_telegram_message, which hard-codes https://api.telegram.org in notifications/telegram.py:322. In those egress/proxy deployments, runtime-entry failures now post to the wrong endpoint and the intended alert is not delivered; route this through the configured crisis Telegram settings/adapter instead of the generic sender.
Useful? React with 👍 / 👎.
| def _split_env_list(value: str | None) -> tuple[str, ...]: | ||
| return tuple( | ||
| item.strip() | ||
| for item in str(value or "").replace(";", ",").split(",") |
There was a problem hiding this comment.
When CRISIS_ALERT_TELEGRAM_CHAT_IDS is supplied as newline-separated values, the existing runtime config parser (runtime_config_support.split_env_list) normalizes those into separate recipients, but this new helper only replaces semicolons before splitting on commas. That makes the fallback send one Telegram request with a single chat ID containing newlines, so runtime-entry failure alerts miss all intended crisis chats in that supported configuration; reuse the existing splitter or RUNTIME_SETTINGS.crisis_alert_telegram_chat_ids.
Useful? React with 👍 / 👎.
1 participant
Summary
Tests