feat(telegram): add error policy for suppressing repetitive error messages#51914
Conversation
Greptile SummaryThis PR introduces a per-account Telegram error policy ( Issues found:
Confidence Score: 2/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/telegram/bot-message-dispatch.ts
Line: 672-680
Comment:
**`"always"` policy still suppresses errors via cooldown**
`shouldSuppressTelegramError` is called unconditionally for both `"always"` and `"once"` policies. When policy is `"always"`, the intent is that every error surfaces — but because the cooldown check runs regardless, the first error for a given `chatId` gets recorded, and all subsequent errors within the 4-hour window are silently dropped even though the user explicitly asked for `"always"`.
The guard should only apply when policy is `"once"`:
```suggestion
if (
errorPolicy.policy === "once" &&
shouldSuppressTelegramError({
chatId,
cooldownMs: errorPolicy.cooldownMs,
errorMessage: String(err),
})
) {
return;
}
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/telegram/error-policy.ts
Line: 122-126
Comment:
**`errorMessage` stored but never used for identity comparison**
The PR description promises that only "repeated identical errors" are suppressed. However, `shouldSuppressTelegramError` never compares the incoming `errorMessage` against `entry.errorMessage` — it suppresses **any** error that arrives within the cooldown window, regardless of type.
In practice: if a chat first sees a `429` rate-limit error, then shortly afterward encounters an auth/connection failure, that second (different, potentially more actionable) error is silently dropped for up to 4 hours.
To match the documented semantics, the comparison needs to incorporate error identity:
```suggestion
// Check if we're still within cooldown period for the same error
if (
now - entry.lastErrorTime < cooldownMs &&
entry.errorMessage === errorMessage
) {
// Within cooldown and same error - suppress
return true;
}
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/telegram/error-policy.ts
Line: 12
Comment:
**Unbounded in-memory growth in `errorCooldownStore`**
`errorCooldownStore` is a module-level `Map` that is never pruned. Every unique `chatId` that ever triggers an error is permanently retained for the lifetime of the process. In a long-running bot serving many chats this will accumulate silently.
Consider evicting stale entries — the information needed is already present (`lastErrorTime`). A simple approach is to delete the entry when the cooldown has expired and the error is being let through:
```ts
// After the "cooldown expired" branch:
errorCooldownStore.set(chatIdStr, { lastErrorTime: now, errorMessage });
// or, at the start of shouldSuppressTelegramError, evict all expired entries
// periodically (e.g. every N calls) to bound memory usage.
```
Alternatively, cap the Map size or use a TTL-based structure.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/config/zod-schema.providers-core.ts
Line: 82
Comment:
**`TelegramTopicSchema` has `errorPolicy` but no `errorCooldownMs`**
`TelegramGroupSchema` and `TelegramDirectSchema` both add `errorCooldownMs` alongside `errorPolicy`, but `TelegramTopicSchema` receives only `errorPolicy`. A user who sets `errorPolicy: "once"` at the topic level has no way to tune the cooldown window.
Additionally, `resolveTelegramErrorPolicy` in `error-policy.ts` does not read topic-level configs at all — so even the `errorPolicy` field on `TelegramTopicSchema` is silently ignored at runtime. This should either be wired up in the resolver or the field should be removed from the schema to avoid misleading users.
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: "feat(telegram): add ..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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: 206c70404e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd7872e6d3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@obviyus could you kindly review this when you have a moment? Thank you! |
…sages Introduces per-account error policy configuration that can suppress repetitive error messages (e.g., 429 rate limit, ECONNRESET) to prevent noisy error floods in Telegram channels. Closes openclaw#34498
obviyus
force-pushed
the
fix/telegram-error-policy-v2
branch
from
35d91d3 to
71b2b0d
Compare
|
Patched this on the PR branch. Simple version: the original feature added new config knobs, but runtime was reading them through a separate partial resolver, so DM/topic overrides and multi-account behavior were wrong. I rewired it to use the Telegram config that is already resolved for the message path, so account/group/direct/topic inheritance now follows the normal code path instead of a second special case. I also keyed the suppression cache by account + chat + thread, so one bot or topic cannot suppress another. Verified with a focused test for the policy logic and a full |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71b2b0dcb9
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1eeb63e9fb
ℹ️ 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".
obviyus
left a comment
obviyus
left a comment
There was a problem hiding this comment.
Reviewed latest changes; landing now.
|
Landed on main. Thanks @chinar-amrutkar. |
|
@obviyus thank you for reviewing 😊 |
|
@chinar-amrutkar thanks for the PR! Feel free to join the Discord server for reporting any other Telegram issues. |
2 participants
Summary
errorPolicyfield in Telegram provider config withsuppressRepeatErrorsanderrorWindowSecoptions.Change Type
Scope
Linked Issue/PR
User-visible / Behavior Changes
New optional
errorPolicyconfiguration for Telegram providers. When enabled, repeated identical errors within a configurable time window are suppressed from user-facing output.Security Impact
Repro + Verification
Steps
errorPolicy: { suppressRepeatErrors: true, errorWindowSec: 60 }Expected
Repeated identical errors suppressed within configured window.
Actual (before fix)
All errors surface to user, creating noise.
Compatibility / Migration
Failure Recovery
git revert HEADon branch