fix: prevent Telegram 409 Conflict on webhook re-registration - #447
Conversation
Delete any existing webhook before calling setWebhook in on_start(), matching the defensive cleanup that polling mode already does. As a safety net, register_webhook() now retries once on 409 after calling delete_webhook(). Closes nearai#440 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the robustness of Telegram webhook registration by addressing potential 409 Conflict errors. It introduces both a pre-emptive deletion of existing webhooks when a channel starts and a retry mechanism specifically for 409 Conflict responses during webhook registration, ensuring smoother and more reliable activation of Telegram channels. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
github-actions
Bot
added
size: S
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the robustness of Telegram webhook registration by proactively deleting any existing webhook on startup and adding a retry mechanism for 409 Conflict errors. The changes are logical and address the described issue. My main feedback is to refactor the retry logic to avoid significant code duplication, improving the maintainability of the register_webhook function, and to consider using specific error variants for clearer error handling.
| // On 409 Conflict, delete the existing webhook and retry once | ||
| if response.status == 409 { | ||
| channel_host::log( | ||
| channel_host::LogLevel::Warn, | ||
| "409 Conflict -- deleting existing webhook and retrying", | ||
| ); | ||
| let _ = delete_webhook(); | ||
|
|
||
| let retry = channel_host::http_request( | ||
| "POST", | ||
| "https://api.telegram.org/bot{TELEGRAM_BOT_TOKEN}/setWebhook", | ||
| &headers.to_string(), | ||
| Some(&body_bytes), | ||
| None, | ||
| ); | ||
| match retry { | ||
| Ok(retry_resp) => { | ||
| if retry_resp.status != 200 { | ||
| let body_str = String::from_utf8_lossy(&retry_resp.body); | ||
| return Err(format!( | ||
| "HTTP {} (after 409 retry): {}", | ||
| retry_resp.status, body_str | ||
| )); | ||
| } | ||
| let api_response: TelegramApiResponse<serde_json::Value> = | ||
| serde_json::from_slice(&retry_resp.body) | ||
| .map_err(|e| format!("Failed to parse response: {}", e))?; | ||
| if !api_response.ok { | ||
| return Err(format!( | ||
| "Telegram API error (after 409 retry): {}", | ||
| api_response | ||
| .description | ||
| .unwrap_or_else(|| "unknown".to_string()) | ||
| )); | ||
| } | ||
| channel_host::log( | ||
| channel_host::LogLevel::Info, | ||
| &format!("Webhook registered successfully (after retry): {}", webhook_url), | ||
| ); | ||
| return Ok(()); | ||
| } | ||
| Err(e) => return Err(format!("HTTP request failed (after 409 retry): {}", e)), | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic for handling the retry after a 409 Conflict introduces significant code duplication. The block for handling the retry response (lines 917-944) is nearly identical to the success path for the initial request (lines 947-972). This duplication makes the code harder to maintain, as future changes to response handling will need to be made in two places. This violates the principle of consolidating related operations to improve consistency and maintainability.
To improve maintainability, consider refactoring this to eliminate the duplicated logic. You could handle the 409 case by deleting the webhook and retrying the request, then process the result of either the original or the retried request in a single, shared code path.
Additionally, consider creating specific error variants for different failure modes (e.g., HttpRequestFailed, ApiResponseParseFailed) instead of using format! to construct error strings. This would provide semantically correct and clearer error messages, improving error handling.
Here is an example of how you could restructure the match block to avoid this duplication:
let mut response = match result {
Ok(response) => response,
Err(e) => return Err(format!("HTTP request failed: {}", e)),
};
let mut retried = false;
if response.status == 409 {
channel_host::log(
channel_host::LogLevel::Warn,
"409 Conflict -- deleting existing webhook and retrying",
);
let _ = delete_webhook();
retried = true;
response = match channel_host::http_request(
"POST",
"https://api.telegram.org/bot{TELEGRAM_BOT_TOKEN}/setWebhook",
&headers.to_string(),
Some(&body_bytes),
None,
) {
Ok(resp) => resp,
Err(e) => return Err(format!("HTTP request failed (after 409 retry): {}", e)),
};
}
if response.status != 200 {
let body_str = String::from_utf8_lossy(&response.body);
let retry_str = if retried { " (after 409 retry)" } else { "" };
return Err(format!("HTTP {}{}: {}", response.status, retry_str, body_str));
}
// Parse Telegram API response
let api_response: TelegramApiResponse<serde_json::Value> =
serde_json::from_slice(&response.body)
.map_err(|e| format!("Failed to parse response: {}", e))?;
if !api_response.ok {
let retry_str = if retried { " (after 409 retry)" } else { "" };
return Err(format!(
"Telegram API error{}: {}",
retry_str,
api_response
.description
.unwrap_or_else(|| "unknown".to_string())
));
}
let retry_str = if retried { " (after retry)" } else { "" };
channel_host::log(
channel_host::LogLevel::Info,
&format!("Webhook registered successfully{}: {}", retry_str, webhook_url),
);
return Ok(());This example would replace the entire match result { ... } block (lines 900-975).
References
- Consolidate related sequences of operations, such as creating, persisting, and scheduling a job, into a single reusable method to improve code consistency and maintainability.
- Create specific error variants for different failure modes (e.g.,
DownloadFailedwith a URL string vs.ManifestReadwith a file path) to provide semantically correct and clear error messages.
There was a problem hiding this comment.
Fixed -- restructured to use a shared response-handling path for both initial and retry requests. Skipped the error variant suggestion since this is a WASM module that communicates errors as plain strings.
Restructure the match block so the initial request and retry share a single response-handling code path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
github-actions
Bot
added
size: M
…#447) * fix: prevent Telegram 409 Conflict on webhook re-registration Delete any existing webhook before calling setWebhook in on_start(), matching the defensive cleanup that polling mode already does. As a safety net, register_webhook() now retries once on 409 after calling delete_webhook(). Closes nearai#440 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: deduplicate 409 retry logic in register_webhook Restructure the match block so the initial request and retry share a single response-handling code path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
Summary
setWebhookinon_start()webhook mode, matching the defensive cleanup polling mode already doesregister_webhook()as a safety net -- on 409, callsdelete_webhook()then retries onceCloses #440
Relationship to other open PRs
PR #381 (
fix/telegram-webhook-secret-header) fixes a different webhook failure: Telegram sends the secret inX-Telegram-Bot-Api-Secret-Tokenbut the capabilities file was missing the webhook config, so the router defaulted toX-Webhook-Secretand rejected all updates with 401. That PR fixes message delivery after webhook registration. This PR fixes webhook registration itself (409 Conflict when re-registering). Both are needed for reliable webhook mode, and they touch different files (#381 touchestelegram.capabilities.json, this PR toucheslib.rs) so there are no merge conflicts.PRs #289 (audio pipeline), #395 (broadcast fix), and #409 (file attachments) also touch the Telegram channel but in message parsing/sending code, far from the
on_start()/register_webhook()changes here. No conflict risk.Test plan
cargo component buildin channels-src/telegram/ with wasm32-wasip1 target)cargo clippy --all --all-features-- zero warningsGenerated with Claude Code