Conversation
[pull] origin from krau:main
[pull] origin from krau:main
Reviewer's GuideThis PR bumps numerous dependencies, integrates a full internationalization layer, adds a userbot-based fallback for file retrieval, refines dispatcher handler registration, and internationalizes logging and configuration messages. Sequence Diagram for File Retrieval with Userbot FallbacksequenceDiagram
actor User
participant BotApp as SaveAny-Bot Application
participant BotClient as Primary Telegram Client
participant UserClient as Fallback User Client
participant TelegramAPI
User->>BotApp: Send message with file link
BotApp->>BotApp: parseLink(link)
BotApp->>BotApp: tryFetchFileFromMessage(chatID, messageID)
activate BotApp
BotApp->>BotClient: FileFromMessage(chatID, messageID)
activate BotClient
BotClient->>TelegramAPI: Get file info/download
TelegramAPI-->>BotClient: Respond (e.g., success or error)
deactivate BotClient
alt File fetch failed with BotClient (e.g., peer not found) AND UserClient enabled
BotApp->>UserClient: FileFromMessage(chatID, messageID)
activate UserClient
UserClient->>TelegramAPI: Get file info/download
TelegramAPI-->>UserClient: Respond
deactivate UserClient
UserClient-->>BotApp: File object / error
else BotClient succeeded or UserClient not used
BotClient-->>BotApp: File object / error
end
deactivate BotApp
BotApp->>BotApp: Process file (e.g., add to queue, save)
BotApp->>User: Notify status (e.g., "File processing started")
Class Diagram for the New Internationalization (i18n) ModuleclassDiagram
class I18nModule {
<<Module>>
+Init(lang: string)
+T(key: string, templateData: map) string
+TWithLang(lang: string, key: string, templateData: map) string
+TWithoutInit(lang: string, key: string, templateData: map) string
}
class Config {
+Lang: string
}
class CmdRun {
+InitAll()
}
class Logger {
+Error(message: string)
+Info(message: string)
}
Config --|> I18nModule : provides lang for Init
CmdRun ..> I18nModule : calls Init()
Logger ..> I18nModule : uses T() for messages
note for I18nModule "Loads translations from locale/*.toml files"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. WalkthroughThis update introduces internationalization (i18n) support, adds a Telegram userbot client with authentication and middleware, and enhances file handling logic to optionally use the userbot for Telegram file retrieval. Numerous log and error messages are now localized. Dependency versions are updated, and new tooling for i18n key generation is included. Changes
Sequence Diagram(s)Telegram File Retrieval with Optional UserbotsequenceDiagram
participant User
participant Bot
participant UserClient
participant TelegramAPI
User->>Bot: Sends message with file
Bot->>Bot: Check UseUserClient flag
alt UseUserClient is true and UserClient available
Bot->>UserClient: Request file via userbot context
UserClient->>TelegramAPI: Fetch file
TelegramAPI-->>UserClient: Return file
UserClient-->>Bot: File data
else
Bot->>TelegramAPI: Fetch file via bot context
TelegramAPI-->>Bot: Return file
end
Bot->>Bot: Store UseUserClient flag in task
Bot-->>User: Continue processing
Userbot Login FlowsequenceDiagram
participant App
participant User
participant UserClient
participant TelegramAPI
App->>UserClient: Initialize with config
UserClient->>User: Prompt for phone/code/password
User->>UserClient: Provide auth info
UserClient->>TelegramAPI: Authenticate
TelegramAPI-->>UserClient: Auth result
UserClient->>App: Success or error
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (21)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @Silentely, 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!
Summary of Changes
Hello team, Gemini here with a summary of this pull request titled "Dev". Based on the changes, it appears this PR introduces significant enhancements by integrating a Telegram user client (userbot) for potentially more robust file fetching and adds internationalization (i18n) support to the bot's messages. It also includes dependency updates and minor logging improvements.
Highlights
- User Client Integration: Adds support for logging in and using a Telegram user client (userbot) alongside the main bot client. This user client can be configured to handle fetching files from messages, potentially bypassing limitations of the standard bot API for certain chat types or large files. A new configuration section
telegram.userbotis introduced. - Internationalization (i18n): Introduces internationalization capabilities using the
go-i18nlibrary. Message strings are moved to TOML locale files (i18n/locale/zh-Hans.tomlis added), and the bot is updated to use translated messages based on the configured language (langconfig option). A command-line tool (cmd/geni18n) is added to generate Go constants for message keys. - Dependency Updates: Updates several dependencies, including
gotd/td,gorm.io/gorm, and adds new dependencies for i18n (go-i18n), terminal UI for userbot auth (charmbracelet/huh,log,color), and custom client middlewares (cenkalti/backoff,gotd/contrib). - Logging and Error Handling Improvements: Updates log messages in several places to use the new i18n system. Custom middlewares for user client connection recovery and RPC retries are added to improve robustness.
- Bot Handler Filtering: Adds filters to the main bot handlers to ignore messages from channels and group chats by default, suggesting a focus on private chat interactions unless explicitly configured otherwise (though the filter is on chat type, not mentions).
Changelog
Click here to see the changelog
- bot/handle_add_task.go
- Imported
userclientpackage. - Modified
AddToQueueto conditionally use the user client's API for fetching files ifrecord.UseUserClientis true and the user client is initialized (lines 157-164). - Added
UseUserClientfield to thetypes.Taskstruct when creating a new task (line 179).
- Imported
- bot/handle_link.go
- Imported
userclientpackage. - Added
tryFetchFileFromMessagefunction to attempt fetching a file using the provided context's client, and fall back to the user client if specific errors occur (peer not found,unexpected message type) and the user client is enabled (lines 57-72). - Added
tryFetchMessagefunction as a wrapper aroundGetTGMessage(lines 75-76). - Modified
handleLinkMessageto usetryFetchFileFromMessageto get the file and theuseUserClientflag (line 105). - Updated file name generation in
handleLinkMessageto usetryFetchMessageto get the message object if needed (lines 112-117). - Added
UseUserClientflag to thedao.ReceivedFilerecord when saving (line 127). - Added
UseUserClientflag to thetypes.Taskstruct when creating a new task (line 145).
- Imported
- bot/handle_start.go
- Changed the
helpcommand to display the full Git commit hash instead of a truncated version (line 38).
- Changed the
- bot/handlers.go
- Imported
extpackage. - Added new message handlers to filter out messages from channels and chats (groups) before the main
checkPermissionhandler (lines 12-17). - Renamed the dispatcher variable from
dispatchertodisp.
- Imported
- cmd/geni18n/main.go
- Added a new command-line tool to generate Go constants for i18n message keys from TOML locale files.
- Reads TOML files in
./i18n/locale, extracts keys from[[...]]sections using a regex (line 23). - Generates a Go file (
i18n/i18nk/keys.go) with PascalCase constants for the extracted keys (lines 65-71).
- cmd/run.go
- Imported
context,i18n,i18nk, anduserclientpackages. - Modified shutdown log messages to use i18n translated strings (lines 32-33).
- Modified cache cleaning log messages and error messages to use i18n translated strings with template data (lines 39-65).
- Added
i18n.Initcall inInitAllto initialize the i18n bundle (line 76). - Added conditional
userclient.Logincall inInitAllif userbot is enabled in config (lines 81-90).
- Imported
- common/logger.go
- Removed
slog.DefaultChannelNamesetting (line 16). - Added a custom log template string (line 26).
- Applied the custom log template to the console handler (line 28).
- Applied the custom log template to the file handler if configured (line 37).
- Removed
- common/os.go
- Imported
i18nandi18nkpackages. - Modified log messages in
RmFileAfterto use i18n translated strings with template data (lines 15-18, 21-24, 27-30).
- Imported
- config/viper.go
- Imported
errors,i18n, andi18nkpackages. - Added
Langfield to theConfigstruct (line 17). - Added
Userbotstruct totelegramConfigwithEnableandSessionfields (lines 64-67). - Set default values for
lang,telegram.userbot.enable, andtelegram.userbot.session(lines 95, 106-107). - Changed error messages and info messages in
Initto use i18n translated strings, including using the newTWithoutInitfunction for errors that occur before full i18n initialization (lines 145-148, 152-154, 160-163).
- Imported
- core/download.go
- Imported
userclientpackage. - Modified the download logic in
processPendingTaskto use the user client's API (userclient.UC.API()) iftask.UseUserClientis true and the user client is initialized (lines 66-69).
- Imported
- dao/model.go
- Added
UseUserClientboolean field to theReceivedFilestruct to track if the userbot was used to fetch the file (line 19).
- Added
- go.mod
- Updated various dependencies, including
gotd/td,gorm.io/gorm,golang.org/x/net,golang.org/x/time,golang.org/x/exp,golang.org/x/tools,minio/minio-go,duke-git/lancet. - Added new dependencies:
github.com/nicksnyder/go-i18n/v2,github.com/charmbracelet/huh,github.com/charmbracelet/log,github.com/fatih/color,github.com/cenkalti/backoff/v4,github.com/gotd/contrib, and severalcharmbracelet/xpackages.
- Updated various dependencies, including
- go.sum
- Updated checksums corresponding to the
go.modchanges.
- Updated checksums corresponding to the
- i18n/i18n.go
- Added a new package for internationalization.
- Uses
go-i18nto load locale files embedded in the binary. - Provides
Initfunction to load messages and set the default language. - Provides
Tfunction for translating message keys using the initialized localizer. - Provides
TWithLangfor translating with a specific language. - Provides
TWithoutInitfor translating beforeInitis called (used in config loading).
- i18n/i18nk/keys.go
- Added a new file containing Go constants for i18n message keys.
- This file is generated by the
cmd/geni18ntool.
- i18n/locale/zh-Hans.toml
- Added a new TOML file containing Simplified Chinese translations for various bot messages and log strings.
- types/task.go
- Added
UseUserClientboolean field to theTaskstruct to indicate if the task should use the user client for downloading (line 19).
- Added
- userclient/auth.go
- Added a new package and file for user client authentication.
- Defines
termialAuthConversatorwhich implementsgotgproto.AuthConversatorusingcharmbracelet/huhfor interactive phone number, code, and password input in the terminal.
- userclient/middlewares/middlewares.go
- Added a new package and file for user client Telegram client middlewares.
- Provides
NewDefaultMiddlewaresfunction to create a slice of default middlewares including custom recovery and retry logic.
- userclient/middlewares/recovery/recovery.go
- Added a new package and file implementing a custom
gotd/tdmiddleware. - This middleware uses
cenkalti/backofffor exponential backoff and retries requests if non-Telegram business errors occur.
- Added a new package and file implementing a custom
- userclient/middlewares/retry/retry.go
- Added a new package and file implementing a custom
gotd/tdmiddleware. - This middleware retries requests up to a maximum number of attempts if specific internal or RPC errors occur.
- Added a new package and file implementing a custom
- userclient/userclient.go
- Added a new package and file for managing the user client.
- Provides a global
UCvariable to hold the user client instance. - Provides
GetCtxfunction to get anext.Contextfrom the user client. - Provides
Loginfunction to log in the user client using the configured session path and thetermialAuthConversatorfor interactive authentication. - Uses the custom recovery and retry middlewares during client initialization.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and 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 to provide feedback.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces significant and valuable features, including internationalization (i18n) support, an optional Telegram userbot fallback for fetching files, and various enhancements to logging, configuration, and task handling. The integration of the userbot client for authentication and message/file fetching is a notable improvement for robustness. The i18n implementation is comprehensive, complete with locale files and a generation tool for keys.
Overall, the changes are well-structured. I've identified a few areas for improvement, primarily concerning context management for the user client, error handling robustness, and clarification on some handler logic. Addressing these points will further enhance the quality and reliability of the new features.
Summary of Findings
- Context Management for User Client: The
userclient.GetCtx()function incorrectly caches and reuses a single globalext.Context. This is a high-severity issue that can lead to race conditions and incorrect behavior. Each operation should obtain a fresh context. - Error Handling Robustness: In
bot/handle_link.go, error checking intryFetchFileFromMessagerelies on string matching, which is brittle. It should ideally check for specific error types/codes. - Userbot Fallback Consistency: The
tryFetchMessagefunction inbot/handle_link.golacks a userbot fallback mechanism, unliketryFetchFileFromMessage. This could be an area for enhancing robustness. - Handler Scope Clarification: The new
ChatTypeChatfilter inbot/handlers.goneeds confirmation regarding its intended scope, particularly concerning "regular" Telegram groups. - i18n Key Generation Regex (Not Commented): The regex in
cmd/geni18n/main.gofor extracting TOML keys is functional for current simple keys but might not handle more complex TOML key specifications if used in the future. (Severity: low, not commented directly per settings).
Merge Readiness
This pull request introduces substantial and valuable enhancements. However, there is a high-severity issue related to ext.Context management in the user client (userclient.GetCtx()) that needs to be addressed to prevent potential race conditions and incorrect behavior. Additionally, there are a few medium-severity items concerning error handling robustness and handler logic clarity that would benefit from review and potential adjustment.
I recommend addressing these issues, particularly the high-severity context management problem, before merging. As an AI reviewer, I am not authorized to approve pull requests; please ensure further review and approval from other maintainers.
| var ectx *ext.Context | ||
|
|
||
| func GetCtx() *ext.Context { | ||
| if ectx != nil { | ||
| return ectx | ||
| } | ||
| ectx = UC.CreateContext() | ||
| return ectx |
There was a problem hiding this comment.
The GetCtx() function caches and reuses a single global ext.Context instance (ectx). This is generally unsafe because ext.Context instances are often intended to be short-lived and specific to a single operation or update. Reusing a single instance across multiple concurrent operations can lead to race conditions, incorrect state, or issues with context cancellation.
It's recommended that each logical operation requiring a user client context obtains a fresh one by calling UC.CreateContext(). Consider removing the global ectx variable and modifying GetCtx() to always return a new context from UC.CreateContext(). You might also want to rename GetCtx to something like CreateUserClientContext to better reflect its behavior if it always creates a new one.
var UC *gotgproto.Client
// var ectx *ext.Context // Remove global cached context
// GetCtx should return a new context for each call.
// Consider renaming to CreateUserClientContext or similar.
func GetCtx() *ext.Context {
if UC == nil {
// This case should ideally not happen if Login is always called first.
// Panic or return an error to indicate UC is not initialized.
common.Log.Panic("User client (UC) is not initialized when calling GetCtx")
return nil // Or handle error more gracefully
}
return UC.CreateContext()
}| if err == nil { | ||
| return file, false, nil | ||
| } | ||
| if (strings.Contains(err.Error(), "peer not found") || strings.Contains(err.Error(), "unexpected message type")) && userclient.UC != nil { |
There was a problem hiding this comment.
The current method of checking for specific errors by matching substrings (e.g., "peer not found", "unexpected message type") can be brittle if the underlying error messages change in future library updates. Could we make this more robust, perhaps by checking for specific error types or codes if the Telegram library exposes them (e.g., using tgerr.Is or checking *tgerr.Error fields)? If not, defining these substrings as constants might be a small improvement for maintainability.
| func tryFetchMessage(ctx *ext.Context, chatID int64, messageID int) (*tg.Message, error) { | ||
| return GetTGMessage(ctx, chatID, messageID) | ||
| } |
There was a problem hiding this comment.
The tryFetchMessage function currently only wraps GetTGMessage and doesn't seem to implement a userbot fallback, unlike tryFetchFileFromMessage. If GetTGMessage fails (e.g., due to errors like "peer not found"), could it also benefit from an attempt with the userbot client, especially when this function is used to generate a filename if file.FileName is initially empty? This would enhance consistency and robustness.
| disp.AddHandler(handlers.NewMessage(filters.Message.ChatType(filters.ChatTypeChannel), func(ctx *ext.Context, u *ext.Update) error { | ||
| return dispatcher.EndGroups | ||
| })) | ||
| disp.AddHandler(handlers.NewMessage(filters.Message.ChatType(filters.ChatTypeChat), func(ctx *ext.Context, u *ext.Update) error { | ||
| return dispatcher.EndGroups | ||
| })) |
There was a problem hiding this comment.
These new handlers are designed to skip events from channels and certain private chats by returning dispatcher.EndGroups. The handler for filters.Message.ChatType(filters.ChatTypeChat) will affect 1-on-1 private chats (not with the bot) and also "regular" Telegram groups (distinct from supergroups).
Could you confirm if the intent is to ignore messages from regular groups as well? If the bot is expected to function in regular groups, this filter might prevent that. If it's intended only for supergroups and DMs with the bot, then this setup (along with the subsequent filters.Message.All handler) should work as expected.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Silentely - I've reviewed your changes - here's some feedback:
- In userclient.Login's goroutine, add an early return after sending the error to the res channel to avoid sending two responses and potential deadlocks.
- Global
ectxin userclient.GetCtx is reused across all calls and may introduce context reuse or concurrency issues—consider returning a fresh context per operation or synchronizing access. - TWithoutInit rebuilds the i18n bundle and reloads locale files on every invocation, which can be expensive—cache the parsed bundle or localizer to avoid repeated file reads.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| Middlewares: middlewares.NewDefaultMiddlewares(ctx, 5*time.Minute), | ||
| }, | ||
| ) | ||
| if err != nil { |
There was a problem hiding this comment.
issue (bug_risk): Missing return after sending error result in Login goroutine
After sending the error to the res channel, add a return to avoid a second send and prevent the goroutine from blocking.
| var UC *gotgproto.Client | ||
| var ectx *ext.Context | ||
|
|
||
| func GetCtx() *ext.Context { | ||
| if ectx != nil { | ||
| return ectx | ||
| } | ||
| ectx = UC.CreateContext() | ||
| return ectx | ||
| } |
There was a problem hiding this comment.
suggestion (bug_risk): Caching a single ext.Context globally is unsafe
Storing a package-level *ext.Context can cause stale context reuse and race conditions. It's safer to return a new context from UC.CreateContext() each time.
| var UC *gotgproto.Client | |
| var ectx *ext.Context | |
| func GetCtx() *ext.Context { | |
| if ectx != nil { | |
| return ectx | |
| } | |
| ectx = UC.CreateContext() | |
| return ectx | |
| } | |
| var UC *gotgproto.Client | |
| func GetCtx() *ext.Context { | |
| return UC.CreateContext() | |
| } |
2 participants
Summary by Sourcery
Introduce comprehensive i18n support and an optional Telegram userbot fallback for file fetching, update dependencies, adjust handler registration, and enhance logging and task models for userbot usage
New Features:
Enhancements:
Summary by CodeRabbit
New Features
Improvements
Dependency Updates
Bug Fixes
Chores