From 7776acaf124b1cad358b47ab5470a9832c74fa28 Mon Sep 17 00:00:00 2001 From: ScuroNeko Date: Tue, 24 Mar 2026 13:45:19 +0300 Subject: [PATCH] refactor logging setup and split local/remote close APIs --- .golangci.yml | 14 ++++++ .pre-commit-config.yaml | 41 +++++++++++++++++ Makefile | 15 ------ bot.go | 93 +++++++++++++++++++++++++++++--------- cmd_generator_test.go | 4 +- handler.go | 9 ++++ msg_context.go | 4 ++ msg_context_test.go | 4 +- plugins.go | 61 ++++++++++++++++++++++++- tgapi/api.go | 9 ++-- tgapi/api_test.go | 4 +- tgapi/methods.go | 8 ++-- tgapi/methods_test.go | 12 ++--- tgapi/uploader_api.go | 3 +- tgapi/uploader_api_test.go | 4 +- utils/utils.go | 26 +++++++++++ utils/utils_test.go | 34 ++++++++++++++ utils/version.go | 4 +- 18 files changed, 283 insertions(+), 66 deletions(-) create mode 100644 .golangci.yml create mode 100644 .pre-commit-config.yaml delete mode 100644 Makefile create mode 100644 utils/utils_test.go diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..a7dcbc3 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,14 @@ +version: "2" +run: + timeout: 5m +linters: + disable-all: true + enable: + - errcheck + - govet + - ineffassign + - staticcheck + - unused +issues: + max-issues-per-linter: 0 + max-same-issues: 0 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..21e64ed --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,41 @@ +repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v6.0.0 + hooks: + - id: trailing-whitespace + - id: end-of-file-fixer + - id: check-merge-conflict + - id: check-yaml + - id: check-json + - id: mixed-line-ending + args: ["--fix=lf"] + + - repo: local + hooks: + - id: gofmt + name: gofmt + entry: gofmt -w + language: system + types: [go] + + - id: go-vet + name: go vet + entry: go vet ./... + language: system + pass_filenames: false + types: [go] + + - id: golangci-lint + name: golangci-lint + entry: golangci-lint run + language: system + pass_filenames: false + types: [go] + + - id: go-test + name: go test + entry: go test ./... + language: system + pass_filenames: false + stages: [pre-push] + types: [go] diff --git a/Makefile b/Makefile deleted file mode 100644 index ef3b6eb..0000000 --- a/Makefile +++ /dev/null @@ -1,15 +0,0 @@ -# Проверка наличия golangci-lint -GO_LINT := $(shell command -v golangci-lint 2>/dev/null) - -# Цель: запуск всех проверок кода -check: - @echo "🔍 Running code checks..." - @go mod tidy -v - @go vet ./... - @if [ -n "$(GO_LINT)" ]; then \ - echo "✅ golangci-lint found, running..." && \ - golangci-lint run --timeout=5m --verbose; \ - else \ - echo "⚠️ golangci-lint not installed. Install with: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.57.2"; \ - fi - @go test -race -v ./... 2>/dev/null || echo "⚠️ Tests skipped or failed (run manually with 'go test -race ./...')" diff --git a/bot.go b/bot.go index 6b677a6..f36bb71 100644 --- a/bot.go +++ b/bot.go @@ -7,7 +7,6 @@ import ( "sort" "strings" "sync" - "time" "git.nix13.pw/scuroneko/extypes" "git.nix13.pw/scuroneko/laniakea/tgapi" @@ -163,9 +162,7 @@ func NewBot[T any](opts *BotOpts) *Bot[T] { // Fetch bot info to validate token and get username u, err := api.GetMe() if err != nil { - closeCtx, cancel := context.WithTimeout(context.Background(), time.Second*10) - defer cancel() - _ = bot.Close(closeCtx) + _ = bot.Close() bot.logger.Fatal(err) } bot.username = Val(u.Username, "") @@ -179,12 +176,9 @@ func NewBot[T any](opts *BotOpts) *Bot[T] { // Close gracefully shuts down bot-owned resources. // -// The provided context is used to close the API client's long-polling request. -// Upload shutdown is not context-aware and still waits for pending uploads. -// // Close shuts down, in order: +// - Registered plugins via Plugin.Close // - Uploader (waits for pending uploads) -// - API client long-poll request via ctx // - API client internals // - RequestLogger (if enabled) // - Main logger @@ -193,18 +187,19 @@ func NewBot[T any](opts *BotOpts) *Bot[T] { // for invoking Close after RunWithContext returns to release these resources. // // Close returns a joined error containing all shutdown failures, if any. -func (bot *Bot[T]) Close(ctx context.Context) error { +func (bot *Bot[T]) Close() error { var e []error + for _, p := range bot.plugins { + if err := p.Close(); err != nil { + e = append(e, err) + } + } if err := bot.uploader.Close(); err != nil { bot.logger.Errorln(err) e = append(e, err) } - if _, err := bot.api.CloseWithContext(ctx); err != nil { - bot.logger.Errorln(err) - e = append(e, err) - } - if err := bot.api.CloseApi(); err != nil { + if err := bot.api.Close(); err != nil { bot.logger.Errorln(err) e = append(e, err) } @@ -220,6 +215,17 @@ func (bot *Bot[T]) Close(ctx context.Context) error { return errors.Join(e...) } +// CloseRemote sends Telegram Bot API "close" request for the current bot +// instance using ctx for cancellation and deadlines. +// +// This is separate from Bot.Close(), which only releases local resources. +func (bot *Bot[T]) CloseRemote(ctx context.Context) error { + if _, err := bot.api.CloseRemoteWithContext(ctx); err != nil { + return err + } + return nil +} + // initLoggers configures the main and optional request loggers. // // Uses DEBUG flag to set log level (DEBUG if true, FATAL otherwise). @@ -231,27 +237,25 @@ func (bot *Bot[T]) initLoggers(opts *BotOpts) { level = slog.DEBUG } - bot.logger = slog.CreateLogger().Level(level).Prefix("BOT") - bot.logger.AddWriter(bot.logger.CreateJsonStdoutWriter()) + bot.logger = utils.CreateLogger("BOT", level) if opts.WriteToFile { path := fmt.Sprintf("%s/main.log", strings.TrimRight(opts.LoggerBasePath, "/")) - fileWriter, err := bot.logger.CreateTextFileWriter(path) + logger, err := utils.CreateFileLogger("BOT", level, path) if err != nil { bot.logger.Fatal(err) } - bot.logger.AddWriter(fileWriter) + bot.logger = logger } if opts.UseRequestLogger { - bot.RequestLogger = slog.CreateLogger().Level(level).Prefix("REQUESTS") - bot.RequestLogger.AddWriter(bot.RequestLogger.CreateJsonStdoutWriter()) + bot.RequestLogger = utils.CreateLogger("REQUESTS", level) if opts.WriteToFile { path := fmt.Sprintf("%s/requests.log", strings.TrimRight(opts.LoggerBasePath, "/")) - fileWriter, err := bot.RequestLogger.CreateTextFileWriter(path) + logger, err := utils.CreateFileLogger("REQUESTS", level, path) if err != nil { bot.logger.Fatal(err) } - bot.RequestLogger.AddWriter(fileWriter) + bot.RequestLogger = logger } } } @@ -280,6 +284,16 @@ func (bot *Bot[T]) GetLogger() *slog.Logger { return bot.logger } // Returns nil if not set via DatabaseContext(). func (bot *Bot[T]) GetDBContext() *T { return bot.dbContext } +// GetLoggerLevel returns the effective log level derived from the bot's debug +// flag. +func (bot *Bot[T]) GetLoggerLevel() slog.LogLevel { + level := slog.FATAL + if bot.debug { + level = slog.DEBUG + } + return level +} + // L10n translates a key in the given language. // Returns empty string if translation not found. func (bot *Bot[T]) L10n(lang, key string) string { @@ -341,13 +355,38 @@ func (bot *Bot[T]) ErrorTemplate(s string) *Bot[T] { // Debug enables or disables debug logging. func (bot *Bot[T]) Debug(debug bool) *Bot[T] { bot.debug = debug + level := slog.FATAL + if debug { + level = slog.DEBUG + } + + bot.logger.Level(level) + if bot.RequestLogger != nil { + bot.RequestLogger.Level(level) + } + for _, p := range bot.plugins { + if p.logger == nil { + continue + } + p.logger.Level(level) + } return bot } // AddPlugins registers one or more plugins. // Plugins are executed in registration order unless filtered by middleware. +// +// Registration is a commit point for plugin configuration. The Bot stores +// plugin metadata internally, so plugins must be fully configured before they +// are passed here. Post-registration mutation through the original *Plugin is +// not a supported API, even if some changes appear to work due to shared maps. func (bot *Bot[T]) AddPlugins(plugin ...*Plugin[T]) *Bot[T] { + level := bot.GetLoggerLevel() for _, p := range plugin { + if p.logger == nil { + logger := utils.CreateLogger(p.name, level) + p.SetLogger(logger) + } bot.plugins = append(bot.plugins, *p) bot.logger.Debugln(fmt.Sprintf("plugins with name \"%s\" registered", p.name)) } @@ -443,6 +482,11 @@ func (bot *Bot[T]) AddL10n(l *L10n) *Bot[T] { // - Main bot logger // - Request logger (if enabled) // - API and Uploader loggers +// - Already registered plugin loggers +// +// Call this after AddPlugins if plugin loggers should also receive the writer. +// Plugins registered later do not automatically inherit previously added +// database writers; call AddDatabaseLoggerWriter again after adding them. // // Example: // @@ -458,6 +502,11 @@ func (bot *Bot[T]) AddDatabaseLoggerWriter(writer DbLogger[T]) *Bot[T] { for _, l := range bot.extraLoggers { l.AddWriter(w) } + for _, p := range bot.plugins { + if p.logger != nil { + p.logger.AddWriter(w) + } + } return bot } diff --git a/cmd_generator_test.go b/cmd_generator_test.go index 2931170..c2130aa 100644 --- a/cmd_generator_test.go +++ b/cmd_generator_test.go @@ -37,8 +37,8 @@ func TestAutoGenerateCommandsChecksLimitBeforeDelete(t *testing.T) { SetHTTPClient(client), ) defer func() { - if err := api.CloseApi(); err != nil { - t.Fatalf("CloseApi returned error: %v", err) + if err := api.Close(); err != nil { + t.Fatalf("Close returned error: %v", err) } }() diff --git a/handler.go b/handler.go index b07228d..fd1ccd7 100644 --- a/handler.go +++ b/handler.go @@ -97,6 +97,11 @@ func (bot *Bot[T]) handleMessage(update *tgapi.Update, ctx *MsgContext) { if !plugin.executeMiddlewares(ctx, bot.dbContext) { return } + + ctx.Logger = plugin.logger + if ctx.Logger == nil { + ctx.Logger = ctx.botLogger + } plugin.executeCmd(cmd, ctx, bot.dbContext) return } @@ -131,6 +136,10 @@ func (bot *Bot[T]) handleCallback(update *tgapi.Update, ctx *MsgContext) { if !plugin.executeMiddlewares(ctx, bot.dbContext) { return } + ctx.Logger = plugin.logger + if ctx.Logger == nil { + ctx.Logger = ctx.botLogger + } plugin.executePayload(data.Command, ctx, bot.dbContext) return } diff --git a/msg_context.go b/msg_context.go index 725b59e..3b36870 100644 --- a/msg_context.go +++ b/msg_context.go @@ -19,6 +19,10 @@ type MsgContext struct { Msg *tgapi.Message From *tgapi.User + // Logger is the logger assigned by the matched plugin for the current handler call. + // It may fall back to the bot logger when the plugin has no dedicated logger. + Logger *slog.Logger + InlineMsgId string CallbackMsgId int CallbackQueryId string diff --git a/msg_context_test.go b/msg_context_test.go index f6cb124..e1742c7 100644 --- a/msg_context_test.go +++ b/msg_context_test.go @@ -37,8 +37,8 @@ func TestAnswerPhotoIncludesDirectMessagesTopicID(t *testing.T) { SetHTTPClient(client), ) defer func() { - if err := api.CloseApi(); err != nil { - t.Fatalf("CloseApi returned error: %v", err) + if err := api.Close(); err != nil { + t.Fatalf("Close returned error: %v", err) } }() diff --git a/plugins.go b/plugins.go index 134c900..903718f 100644 --- a/plugins.go +++ b/plugins.go @@ -5,6 +5,7 @@ import ( "regexp" "git.nix13.pw/scuroneko/extypes" + "git.nix13.pw/scuroneko/slog" ) // CommandValueType defines the expected type of a command argument. @@ -151,19 +152,30 @@ func (c *Command[T]) validateArgs(args []string) error { // Plugin represents a collection of commands and payloads (e.g., callback handlers), // with shared middleware and configuration. +// +// A Plugin is intended to be fully configured before it is passed to Bot.AddPlugins. +// After registration, treat the plugin as committed and do not mutate it further. +// Post-registration changes through the original *Plugin are not a supported API. type Plugin[T DbContext] struct { name string // Name of the plugin (e.g., "admin", "user") commands map[string]*Command[T] // Registered commands (triggered by message) payloads map[string]*Command[T] // Registered payloads (triggered by callback data) middlewares extypes.Slice[Middleware[T]] // Shared middlewares for all commands/payloads skipAutoCmd bool // If true, all commands in this plugin are excluded from auto-help + logger *slog.Logger + + onClose func() error } // NewPlugin creates a new Plugin with the given name. func NewPlugin[T DbContext](name string) *Plugin[T] { return &Plugin[T]{ - name, make(map[string]*Command[T]), - make(map[string]*Command[T]), extypes.Slice[Middleware[T]]{}, false, + name: name, + commands: make(map[string]*Command[T]), + payloads: make(map[string]*Command[T]), + middlewares: make(extypes.Slice[Middleware[T]], 0), + skipAutoCmd: false, + logger: nil, } } @@ -210,6 +222,51 @@ func (p *Plugin[T]) SkipCommandAutoGen() *Plugin[T] { return p } +// SetLogger sets the logger used for this plugin's handlers. +// +// Call this before Bot.AddPlugins. If the plugin is already registered, changing +// the original *Plugin does not update the Bot's internal copy. +func (p *Plugin[T]) SetLogger(l *slog.Logger) *Plugin[T] { + p.logger = l + return p +} + +// RemoveLogger clears the custom logger for this plugin. +// +// Call this before Bot.AddPlugins. If the plugin is already registered, changing +// the original *Plugin does not update the Bot's internal copy. +func (p *Plugin[T]) RemoveLogger() *Plugin[T] { + p.logger = nil + return p +} + +// SetOnClose registers a callback invoked from Plugin.Close after the plugin +// logger is closed. +// +// Call this before Bot.AddPlugins. If the plugin is already registered, changing +// the original *Plugin does not update the Bot's internal copy. +func (p *Plugin[T]) SetOnClose(f func() error) *Plugin[T] { + p.onClose = f + return p +} + +// Close releases plugin-owned resources such as its logger and optional +// OnClose callback. +func (p *Plugin[T]) Close() error { + var e []error + if p.logger != nil { + if err := p.logger.Close(); err != nil { + e = append(e, err) + } + } + if p.onClose != nil { + if err := p.onClose(); err != nil { + e = append(e, err) + } + } + return errors.Join(e...) +} + // executeCmd finds and executes a command by its trigger string. // Validates arguments and runs middlewares before executor. // On error, sends an error message to the user via ctx.error(). diff --git a/tgapi/api.go b/tgapi/api.go index 30a16c6..c251218 100644 --- a/tgapi/api.go +++ b/tgapi/api.go @@ -95,10 +95,9 @@ type API struct { } // NewAPI creates a new API client from options. -// Always call CloseApi() when done to release resources. +// Always call Close() when done to release resources. func NewAPI(opts *APIOpts) *API { - l := slog.CreateLogger().Level(utils.GetLoggerLevel()).Prefix("API") - l.AddWriter(l.CreateJsonStdoutWriter()) + l := utils.CreateLogger("API", utils.GetLoggerLevel()) client := opts.client if client == nil { @@ -120,10 +119,10 @@ func NewAPI(opts *APIOpts) *API { } } -// CloseApi shuts down the internal worker pool and closes the logger. +// Close shuts down the internal worker pool and closes the logger. // Must be called to avoid resource leaks. // See https://core.telegram.org/bots/api -func (api *API) CloseApi() error { +func (api *API) Close() error { api.pool.stop() return api.logger.Close() } diff --git a/tgapi/api_test.go b/tgapi/api_test.go index 2f3db3d..5dc4491 100644 --- a/tgapi/api_test.go +++ b/tgapi/api_test.go @@ -35,8 +35,8 @@ func TestAPILeavesAcceptEncodingToHTTPTransport(t *testing.T) { SetHTTPClient(client), ) defer func() { - if err := api.CloseApi(); err != nil { - t.Fatalf("CloseApi returned error: %v", err) + if err := api.Close(); err != nil { + t.Fatalf("Close returned error: %v", err) } }() diff --git a/tgapi/methods.go b/tgapi/methods.go index e4a2f9c..948575d 100644 --- a/tgapi/methods.go +++ b/tgapi/methods.go @@ -49,18 +49,18 @@ func (api *API) LogOutWithContext(ctx context.Context) (bool, error) { return req.DoWithContext(ctx, api) } -// Close closes the bot instance on the local server. +// CloseRemote closes the bot instance on the local server. // Returns true on success. // See https://core.telegram.org/bots/api#close -func (api *API) Close() (bool, error) { +func (api *API) CloseRemote() (bool, error) { req := NewRequest[bool, EmptyParams]("close", NoParams) return req.Do(api) } -// CloseWithContext is the context-aware variant of Close. +// CloseRemoteWithContext is the context-aware variant of CloseRemote. // It executes the same request but uses ctx for cancellation and deadlines. // See https://core.telegram.org/bots/api#close -func (api *API) CloseWithContext(ctx context.Context) (bool, error) { +func (api *API) CloseRemoteWithContext(ctx context.Context) (bool, error) { req := NewRequest[bool, EmptyParams]("close", NoParams) return req.DoWithContext(ctx, api) } diff --git a/tgapi/methods_test.go b/tgapi/methods_test.go index 649ff50..86f1aba 100644 --- a/tgapi/methods_test.go +++ b/tgapi/methods_test.go @@ -27,8 +27,8 @@ func TestGetFileByLinkUsesConfiguredAPIURL(t *testing.T) { SetHTTPClient(client), ) defer func() { - if err := api.CloseApi(); err != nil { - t.Fatalf("CloseApi returned error: %v", err) + if err := api.Close(); err != nil { + t.Fatalf("Close returned error: %v", err) } }() @@ -60,8 +60,8 @@ func TestGetFileByLinkReturnsHTTPStatusError(t *testing.T) { SetHTTPClient(client), ) defer func() { - if err := api.CloseApi(); err != nil { - t.Fatalf("CloseApi returned error: %v", err) + if err := api.Close(); err != nil { + t.Fatalf("Close returned error: %v", err) } }() @@ -97,8 +97,8 @@ func TestGetUpdatesOmitsAllowedUpdatesWhenEmpty(t *testing.T) { SetHTTPClient(client), ) defer func() { - if err := api.CloseApi(); err != nil { - t.Fatalf("CloseApi returned error: %v", err) + if err := api.Close(); err != nil { + t.Fatalf("Close returned error: %v", err) } }() diff --git a/tgapi/uploader_api.go b/tgapi/uploader_api.go index 732b894..c244020 100644 --- a/tgapi/uploader_api.go +++ b/tgapi/uploader_api.go @@ -69,8 +69,7 @@ type Uploader struct { // NewUploader creates a multipart uploader bound to an API client. func NewUploader(api *API) *Uploader { - logger := slog.CreateLogger().Level(utils.GetLoggerLevel()).Prefix("UPLOADER") - logger.AddWriter(logger.CreateJsonStdoutWriter()) + logger := utils.CreateLogger("UPLOADER", utils.GetLoggerLevel()) return &Uploader{api, logger} } diff --git a/tgapi/uploader_api_test.go b/tgapi/uploader_api_test.go index 7cf55dd..cbbfebc 100644 --- a/tgapi/uploader_api_test.go +++ b/tgapi/uploader_api_test.go @@ -44,8 +44,8 @@ func TestUploaderEncodesJSONFieldsAndLeavesAcceptEncodingToHTTPTransport(t *test SetHTTPClient(client), ) defer func() { - if err := api.CloseApi(); err != nil { - t.Fatalf("CloseApi returned error: %v", err) + if err := api.Close(); err != nil { + t.Fatalf("Close returned error: %v", err) } }() diff --git a/utils/utils.go b/utils/utils.go index 889cc1f..144f161 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -14,3 +14,29 @@ func GetLoggerLevel() slog.LogLevel { } return level } + +// CreateLogger creates a logger with the shared default policy: +// JSON stdout output, provided prefix, and provided level. +func CreateLogger(prefix string, level slog.LogLevel) *slog.Logger { + logger := slog.CreateLogger().Level(level) + if prefix != "" { + logger.Prefix(prefix) + } + logger.AddWriter(logger.CreateJsonStdoutWriter()) + return logger +} + +// CreateFileLogger creates a logger with the shared default policy and appends +// file output to the provided path. +// +// The returned logger is always non-nil. When file writer creation fails, the +// logger still writes to stdout and the error is returned to the caller. +func CreateFileLogger(prefix string, level slog.LogLevel, filePath string) (*slog.Logger, error) { + logger := CreateLogger(prefix, level) + fileWriter, err := logger.CreateTextFileWriter(filePath) + if err != nil { + return logger, err + } + logger.AddWriter(fileWriter) + return logger, nil +} diff --git a/utils/utils_test.go b/utils/utils_test.go new file mode 100644 index 0000000..4df68fb --- /dev/null +++ b/utils/utils_test.go @@ -0,0 +1,34 @@ +package utils + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "git.nix13.pw/scuroneko/slog" +) + +func TestCreateFileLoggerWritesToConfiguredFile(t *testing.T) { + logPath := filepath.Join(t.TempDir(), "main.log") + + logger, err := CreateFileLogger("TEST", slog.DEBUG, logPath) + if err != nil { + t.Fatalf("CreateFileLogger returned error: %v", err) + } + logger.Infoln("hello from file logger") + if err := logger.Close(); err != nil { + t.Fatalf("Close returned error: %v", err) + } + + data, err := os.ReadFile(logPath) + if err != nil { + t.Fatalf("ReadFile returned error: %v", err) + } + if !strings.Contains(string(data), "hello from file logger") { + t.Fatalf("expected log message in file, got %q", string(data)) + } + if !strings.Contains(string(data), "[TEST]") { + t.Fatalf("expected prefix in file, got %q", string(data)) + } +} diff --git a/utils/version.go b/utils/version.go index e261a40..0a0b5d1 100644 --- a/utils/version.go +++ b/utils/version.go @@ -1,9 +1,9 @@ package utils const ( - VersionString = "1.0.0-rc.5" + VersionString = "1.0.0-rc.7" VersionMajor = 1 VersionMinor = 0 VersionPatch = 0 - VersionBeta = 5 + VersionBeta = 7 )