diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f5ff97..299e5b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,22 @@ # Changelog +## v1.0.0-rc.13 + +### Added +- `AsUserError(...)`, `AsInternalError(...)`, `IsUserError(...)`, and `IsInternalError(...)` for explicitly marking centralized handler errors as user-visible or internal-only without breaking the existing default error flow. + +### Changed +- Bot configuration mutators now treat the bot as configuration-frozen after the first run begins and ignore late mutation attempts for bot-level config such as prefixes, payload defaults, plugins, middleware, runners, localization, scene session wiring, and database context injection. +- `MsgContext` godoc and field comments now describe the normalized update contract more explicitly, including when `Msg`, `From`, callback target fields, `Text`, and `Args` are expected to be populated. +- `MsgContext.Error(...)` and returned handler errors now suppress the automatic user reply when the error is explicitly marked with `AsInternalError(...)`, while keeping the previous user-visible default for unclassified errors. + +### Tests +- Added regression coverage for the bot configuration freeze model, including ignored post-run mutations for core bot configuration methods and late registration paths. +- Added table-driven update-contract coverage for `prepareUpdateCtx(...)`, including message-backed, callback-backed, user-backed, and no-user update kinds. +- Added regression tests proving that `edited_message` and `edited_channel_post` stay out of command routing and continue through generic update handlers. +- Added callback-routing regression tests for both chat-message and inline-message callback targets, including `CallbackQueryId`, `CallbackMsgId`, `InlineMsgId`, and payload-argument guarantees. +- Added regression tests for the new error-visibility model in both message and callback flows, including silent internal-only errors and explicit user-visible callback replies. + ## v1.0.0-rc.12 ### Added diff --git a/TODO.md b/TODO.md index 4d22f15..70a59a3 100644 --- a/TODO.md +++ b/TODO.md @@ -12,12 +12,14 @@ Russian page: Current priority split: -- `Priority 1`: update schema contract, user-facing vs internal error model, configuration freeze model. -- `Priority 2`: webhook runtime model, authorization and policy model, observability model. -- `Priority 3`: service layer and dependency graph model, plugin composition contract. +- `Priority 1`: webhook runtime model, authorization and policy model, observability model. +- `Priority 2`: service layer and dependency graph model, plugin composition contract. Completed former high-priority items: +- `[v1.0.0-rc.13] Update schema contract`: documented and tested the normalized `MsgContext` update-routing contract, including routing categories and per-update field guarantees. +- `[v1.0.0-rc.13] User-facing vs internal error model`: added explicit user-visible vs internal-only error markers and updated centralized handler error routing accordingly. +- `[v1.0.0-rc.13] Configuration freeze model`: formalized bot configuration freeze after first run, documented lifecycle commit points, and added regression coverage for ignored late mutations. - `1. Conversation / Scene Model`: completed in `v1.0.0-rc.12`. - `2. Typed Handler Input Model`: completed in `v1.0.0-rc.12`. - `3. Request Context / Cancellation Model`: completed in `v1.0.0-rc.12`. diff --git a/bot.go b/bot.go index 8163819..70d47b3 100644 --- a/bot.go +++ b/bot.go @@ -118,6 +118,18 @@ type Bot[T DbContext] struct { ran bool } +func (bot *Bot[T]) configMutable(method string) bool { + bot.runStateMu.Lock() + defer bot.runStateMu.Unlock() + if !bot.ran { + return true + } + if bot.logger != nil { + bot.logger.Warnln(fmt.Sprintf("%s called after bot configuration was frozen; ignoring", method)) + } + return false +} + // NewBot creates and initializes a new Bot instance using the provided BotOpts. // // Automatically: @@ -339,6 +351,9 @@ func (bot *Bot[T]) L10n(lang, key string) string { // SetDraftProvider replaces the default DraftProvider with a custom one. // Useful for using LinearDraftIdGenerator to persist draft IDs across restarts. func (bot *Bot[T]) SetDraftProvider(p *DraftProvider) *Bot[T] { + if !bot.configMutable("SetDraftProvider") { + return bot + } bot.draftProvider = p return bot } @@ -350,6 +365,9 @@ func (bot *Bot[T]) GetDraftProvider() *DraftProvider { // SetSessionStore replaces the session store used for scene management. func (bot *Bot[T]) SetSessionStore(store SessionStore) *Bot[T] { + if !bot.configMutable("SetSessionStore") { + return bot + } if store == nil { bot.logger.Warn("SetSessionStore called with nil store; using default MemorySessionStore") return bot @@ -365,6 +383,9 @@ func (bot *Bot[T]) GetSessionStore() SessionStore { // SetSceneScopePriority sets the lookup order for resolving active scene sessions. func (bot *Bot[T]) SetSceneScopePriority(priority []SceneScope) *Bot[T] { + if !bot.configMutable("SetSceneScopePriority") { + return bot + } newPriority := make([]SceneScope, 0, 3) for _, scope := range priority { if scope != SceneScopeUser && scope != SceneScopeChat && scope != SceneScopeUserChat { @@ -391,6 +412,9 @@ func (bot *Bot[T]) SetSceneScopePriority(priority []SceneScope) *Bot[T] { // Value-typed contexts are supported, but the bot warns once because handlers // receive T by value. func (bot *Bot[T]) DatabaseContext(ctx T) *Bot[T] { + if !bot.configMutable("DatabaseContext") { + return bot + } if !bot.warnedValueDB && shouldWarnOnValueDBContext[T]() && bot.logger != nil { bot.logger.Warnln("database context uses a value type; shared dependencies should usually use a pointer type as T") bot.warnedValueDB = true @@ -403,6 +427,9 @@ func (bot *Bot[T]) DatabaseContext(ctx T) *Bot[T] { // UpdateTypes sets the list of update types the bot will request from Telegram. // Overwrites any previously set types. func (bot *Bot[T]) UpdateTypes(t ...tgapi.UpdateType) *Bot[T] { + if !bot.configMutable("UpdateTypes") { + return bot + } bot.updateTypes = make([]tgapi.UpdateType, 0) bot.updateTypes = append(bot.updateTypes, t...) return bot @@ -413,6 +440,9 @@ func (bot *Bot[T]) UpdateTypes(t ...tgapi.UpdateType) *Bot[T] { // Base64 stores the same JSON encoded as a Base64URL string. // InlineKeyboard.SetPayloadType may override this value for an individual keyboard. func (bot *Bot[T]) SetPayloadType(t BotPayloadType) *Bot[T] { + if !bot.configMutable("SetPayloadType") { + return bot + } bot.payloadType = t return bot } @@ -423,6 +453,9 @@ func (bot *Bot[T]) GetPayloadType() BotPayloadType { return bot.payloadType } // SetStrictPayloadType enables or disables strict callback payload decoding. // When enabled, callback payloads must match the bot's default payload type. func (bot *Bot[T]) SetStrictPayloadType(strict bool) *Bot[T] { + if !bot.configMutable("SetStrictPayloadType") { + return bot + } bot.strictPayloadType = strict return bot } @@ -430,6 +463,9 @@ func (bot *Bot[T]) SetStrictPayloadType(strict bool) *Bot[T] { // AddUpdateType adds one or more update types to the list. // Does not overwrite existing types. func (bot *Bot[T]) AddUpdateType(t ...tgapi.UpdateType) *Bot[T] { + if !bot.configMutable("AddUpdateType") { + return bot + } bot.updateTypes = append(bot.updateTypes, t...) return bot } @@ -437,6 +473,9 @@ func (bot *Bot[T]) AddUpdateType(t ...tgapi.UpdateType) *Bot[T] { // AddPrefixes adds one or more command prefixes (e.g., "/", "!"). // Must have at least one prefix before Run(). func (bot *Bot[T]) AddPrefixes(prefixes ...string) *Bot[T] { + if !bot.configMutable("AddPrefixes") { + return bot + } bot.prefixes = append(bot.prefixes, prefixes...) return bot } @@ -445,6 +484,9 @@ func (bot *Bot[T]) AddPrefixes(prefixes ...string) *Bot[T] { // Use "%s" to insert the error message. // Example: "❌ Error: %s" → "❌ Error: Command not found". func (bot *Bot[T]) ErrorTemplate(s string) *Bot[T] { + if !bot.configMutable("ErrorTemplate") { + return bot + } bot.errorTemplate = s return bot } @@ -478,6 +520,9 @@ func (bot *Bot[T]) Debug(debug bool) *Bot[T] { // 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] { + if !bot.configMutable("AddPlugins") { + return bot + } level := bot.GetLoggerLevel() for _, p := range plugin { if p == nil { @@ -514,6 +559,9 @@ func (bot *Bot[T]) AddPlugins(plugin ...*Plugin[T]) *Bot[T] { // // Middleware with an empty name are skipped with a warning. func (bot *Bot[T]) AddMiddleware(middleware ...Middleware[T]) *Bot[T] { + if !bot.configMutable("AddMiddleware") { + return bot + } for _, m := range middleware { if m.name == "" { bot.logger.Warnln("middleware must have a non-empty name") @@ -552,6 +600,9 @@ func (bot *Bot[T]) AddMiddleware(middleware ...Middleware[T]) *Bot[T] { // // Runners with an empty name are skipped with a warning. func (bot *Bot[T]) AddRunner(runner Runner[T]) *Bot[T] { + if !bot.configMutable("AddRunner") { + return bot + } if runner.name == "" { bot.logger.Warnln("runner must have a non-empty name") return bot @@ -575,6 +626,9 @@ func (bot *Bot[T]) AddRunner(runner Runner[T]) *Bot[T] { // // Replaces any previously set L10n instance. func (bot *Bot[T]) AddL10n(l *L10n) *Bot[T] { + if !bot.configMutable("AddL10n") { + return bot + } if l == nil { bot.logger.Warn("AddL10n called with nil L10n; localization will be disabled") return bot diff --git a/bot_test.go b/bot_test.go index e1028fb..712e0ab 100644 --- a/bot_test.go +++ b/bot_test.go @@ -215,3 +215,227 @@ func TestRunWithContextRejectsSecondRun(t *testing.T) { t.Fatalf("expected ErrBotAlreadyRun on second run, got %v", err) } } + +func TestBotConfigurationFreezesAfterRunStarts(t *testing.T) { + type testDB struct{ Name string } + + makeBot := func() *Bot[*testDB] { + return &Bot[*testDB]{ + logger: slog.CreateLogger(), + prefixes: []string{"/"}, + updateTypes: []tgapi.UpdateType{tgapi.UpdateTypeMessage}, + payloadType: BotPayloadBase64, + strictPayloadType: false, + errorTemplate: "%s", + l10n: &L10n{}, + draftProvider: &DraftProvider{}, + sessionStore: NewMemorySessionStore(), + sceneScopePriority: []SceneScope{SceneScopeUserChat, SceneScopeChat, SceneScopeUser}, + } + } + + tests := []struct { + name string + check func(t *testing.T, bot *Bot[*testDB]) + }{ + { + name: "DatabaseContext", + check: func(t *testing.T, bot *Bot[*testDB]) { + original := &testDB{Name: "before"} + bot.DatabaseContext(original) + if err := bot.beginRun(); err != nil { + t.Fatalf("beginRun returned error: %v", err) + } + t.Cleanup(bot.finishRun) + + later := &testDB{Name: "after"} + bot.DatabaseContext(later) + if bot.dbContext != original { + t.Fatal("DatabaseContext mutated after configuration freeze") + } + }, + }, + { + name: "UpdateTypes", + check: func(t *testing.T, bot *Bot[*testDB]) { + original := append([]tgapi.UpdateType(nil), bot.updateTypes...) + if err := bot.beginRun(); err != nil { + t.Fatalf("beginRun returned error: %v", err) + } + t.Cleanup(bot.finishRun) + + bot.UpdateTypes(tgapi.UpdateTypePoll) + if !reflect.DeepEqual(bot.updateTypes, original) { + t.Fatalf("UpdateTypes mutated after configuration freeze: got %v want %v", bot.updateTypes, original) + } + }, + }, + { + name: "AddUpdateType", + check: func(t *testing.T, bot *Bot[*testDB]) { + original := append([]tgapi.UpdateType(nil), bot.updateTypes...) + if err := bot.beginRun(); err != nil { + t.Fatalf("beginRun returned error: %v", err) + } + t.Cleanup(bot.finishRun) + + bot.AddUpdateType(tgapi.UpdateTypePoll) + if !reflect.DeepEqual(bot.updateTypes, original) { + t.Fatalf("AddUpdateType mutated after configuration freeze: got %v want %v", bot.updateTypes, original) + } + }, + }, + { + name: "SetPayloadType", + check: func(t *testing.T, bot *Bot[*testDB]) { + if err := bot.beginRun(); err != nil { + t.Fatalf("beginRun returned error: %v", err) + } + t.Cleanup(bot.finishRun) + + bot.SetPayloadType(BotPayloadJson) + if bot.payloadType != BotPayloadBase64 { + t.Fatalf("payloadType mutated after configuration freeze: got %q want %q", bot.payloadType, BotPayloadBase64) + } + }, + }, + { + name: "SetStrictPayloadType", + check: func(t *testing.T, bot *Bot[*testDB]) { + if err := bot.beginRun(); err != nil { + t.Fatalf("beginRun returned error: %v", err) + } + t.Cleanup(bot.finishRun) + + bot.SetStrictPayloadType(true) + if bot.strictPayloadType { + t.Fatal("strictPayloadType mutated after configuration freeze") + } + }, + }, + { + name: "AddPrefixes", + check: func(t *testing.T, bot *Bot[*testDB]) { + original := append([]string(nil), bot.prefixes...) + if err := bot.beginRun(); err != nil { + t.Fatalf("beginRun returned error: %v", err) + } + t.Cleanup(bot.finishRun) + + bot.AddPrefixes("!") + if !reflect.DeepEqual(bot.prefixes, original) { + t.Fatalf("prefixes mutated after configuration freeze: got %v want %v", bot.prefixes, original) + } + }, + }, + { + name: "ErrorTemplate", + check: func(t *testing.T, bot *Bot[*testDB]) { + if err := bot.beginRun(); err != nil { + t.Fatalf("beginRun returned error: %v", err) + } + t.Cleanup(bot.finishRun) + + bot.ErrorTemplate("changed") + if bot.errorTemplate != "%s" { + t.Fatalf("errorTemplate mutated after configuration freeze: got %q want %q", bot.errorTemplate, "%s") + } + }, + }, + { + name: "SetDraftProvider", + check: func(t *testing.T, bot *Bot[*testDB]) { + original := bot.draftProvider + if err := bot.beginRun(); err != nil { + t.Fatalf("beginRun returned error: %v", err) + } + t.Cleanup(bot.finishRun) + + bot.SetDraftProvider(&DraftProvider{}) + if bot.draftProvider != original { + t.Fatal("draftProvider mutated after configuration freeze") + } + }, + }, + { + name: "SetSessionStore", + check: func(t *testing.T, bot *Bot[*testDB]) { + original := bot.sessionStore + if err := bot.beginRun(); err != nil { + t.Fatalf("beginRun returned error: %v", err) + } + t.Cleanup(bot.finishRun) + + bot.SetSessionStore(NewMemorySessionStore()) + if bot.sessionStore != original { + t.Fatal("sessionStore mutated after configuration freeze") + } + }, + }, + { + name: "SetSceneScopePriority", + check: func(t *testing.T, bot *Bot[*testDB]) { + original := append([]SceneScope(nil), bot.sceneScopePriority...) + if err := bot.beginRun(); err != nil { + t.Fatalf("beginRun returned error: %v", err) + } + t.Cleanup(bot.finishRun) + + bot.SetSceneScopePriority([]SceneScope{SceneScopeUser}) + if !reflect.DeepEqual(bot.sceneScopePriority, original) { + t.Fatalf("sceneScopePriority mutated after configuration freeze: got %v want %v", bot.sceneScopePriority, original) + } + }, + }, + { + name: "AddL10n", + check: func(t *testing.T, bot *Bot[*testDB]) { + original := bot.l10n + if err := bot.beginRun(); err != nil { + t.Fatalf("beginRun returned error: %v", err) + } + t.Cleanup(bot.finishRun) + + bot.AddL10n(&L10n{}) + if bot.l10n != original { + t.Fatal("l10n mutated after configuration freeze") + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.check(t, makeBot()) + }) + } +} + +func TestAddPluginsAndRuntimeRegistrationsNoOpAfterRunStarts(t *testing.T) { + bot := &Bot[NoDB]{ + logger: slog.CreateLogger(), + prefixes: []string{"/"}, + middlewares: []Middleware[NoDB]{NewMiddleware("base", func(ctx *MsgContext, db NoDB) bool { return true })}, + runners: []Runner[NoDB]{NewRunner("base", func(bot *Bot[NoDB]) error { return nil })}, + } + plugin := NewPlugin[NoDB]("late") + + if err := bot.beginRun(); err != nil { + t.Fatalf("beginRun returned error: %v", err) + } + defer bot.finishRun() + + bot.AddPlugins(plugin) + bot.AddMiddleware(NewMiddleware("late", func(ctx *MsgContext, db NoDB) bool { return true })) + bot.AddRunner(NewRunner("late", func(bot *Bot[NoDB]) error { return nil })) + + if len(bot.plugins) != 0 { + t.Fatalf("expected AddPlugins to be ignored after configuration freeze, got %d plugins", len(bot.plugins)) + } + if len(bot.middlewares) != 1 { + t.Fatalf("expected AddMiddleware to be ignored after configuration freeze, got %d middlewares", len(bot.middlewares)) + } + if len(bot.runners) != 1 { + t.Fatalf("expected AddRunner to be ignored after configuration freeze, got %d runners", len(bot.runners)) + } +} diff --git a/error_model.go b/error_model.go new file mode 100644 index 0000000..940ba67 --- /dev/null +++ b/error_model.go @@ -0,0 +1,59 @@ +package laniakea + +import "errors" + +type classifiedError struct { + err error + userVisible bool + internalOnly bool +} + +func (e *classifiedError) Error() string { + if e == nil || e.err == nil { + return "" + } + return e.err.Error() +} + +func (e *classifiedError) Unwrap() error { + if e == nil { + return nil + } + return e.err +} + +// AsUserError marks err as safe to show to the user through the centralized +// handler error flow. +func AsUserError(err error) error { + if err == nil { + return nil + } + return &classifiedError{err: err, userVisible: true} +} + +// AsInternalError marks err as internal-only so it will be logged but not sent +// to the user through the centralized handler error flow. +func AsInternalError(err error) error { + if err == nil { + return nil + } + return &classifiedError{err: err, internalOnly: true} +} + +// IsUserError reports whether err was explicitly marked as user-visible. +func IsUserError(err error) bool { + var classified *classifiedError + if !errors.As(err, &classified) { + return false + } + return classified.userVisible +} + +// IsInternalError reports whether err was explicitly marked as internal-only. +func IsInternalError(err error) bool { + var classified *classifiedError + if !errors.As(err, &classified) { + return false + } + return classified.internalOnly +} diff --git a/handler_test.go b/handler_test.go index efed1cc..9bf901a 100644 --- a/handler_test.go +++ b/handler_test.go @@ -8,6 +8,10 @@ import ( "git.scuroneko.dev/scuroneko/slog" ) +func ptr[T any](v T) *T { + return &v +} + func TestCheckPrefixesSkipsEmptyPrefixes(t *testing.T) { bot := &Bot[NoDB]{prefixes: []string{"", "/"}} @@ -74,6 +78,275 @@ func TestAddUpdateHandlerRejectsReservedUpdateTypes(t *testing.T) { } } +func TestPrepareUpdateCtxContract(t *testing.T) { + tests := []struct { + name string + update *tgapi.Update + wantMsg bool + wantFrom bool + wantFromID int64 + wantCallbackID string + wantCallbackMsgID int + wantInlineMsgID string + }{ + { + name: "message", + update: &tgapi.Update{ + Type: tgapi.UpdateTypeMessage, + Message: &tgapi.Message{ + MessageID: 11, + From: &tgapi.User{ID: 101}, + Chat: &tgapi.Chat{ID: 1001}, + }, + }, + wantMsg: true, + wantFrom: true, + wantFromID: 101, + }, + { + name: "edited message", + update: &tgapi.Update{ + Type: tgapi.UpdateTypeEditedMessage, + EditedMessage: &tgapi.Message{ + MessageID: 12, + From: &tgapi.User{ID: 102}, + Chat: &tgapi.Chat{ID: 1002}, + }, + }, + wantMsg: true, + wantFrom: true, + wantFromID: 102, + }, + { + name: "channel post sender chat", + update: &tgapi.Update{ + Type: tgapi.UpdateTypeChannelPost, + ChannelPost: &tgapi.Message{ + MessageID: 13, + Chat: &tgapi.Chat{ID: -1003}, + }, + }, + wantMsg: true, + }, + { + name: "business message", + update: &tgapi.Update{ + Type: tgapi.UpdateTypeBusinessMessage, + BusinessMessage: &tgapi.Message{ + MessageID: 14, + From: &tgapi.User{ID: 103}, + Chat: &tgapi.Chat{ID: 1004}, + }, + }, + wantMsg: true, + wantFrom: true, + wantFromID: 103, + }, + { + name: "inline query", + update: &tgapi.Update{ + Type: tgapi.UpdateTypeInlineQuery, + InlineQuery: &tgapi.InlineQuery{ID: "iq", From: tgapi.User{ID: 104}}, + }, + wantFrom: true, + wantFromID: 104, + }, + { + name: "chosen inline result", + update: &tgapi.Update{ + Type: tgapi.UpdateTypeChosenInlineResult, + ChosenInlineResult: &tgapi.ChosenInlineResult{ResultID: "res", From: tgapi.User{ID: 105}}, + }, + wantFrom: true, + wantFromID: 105, + }, + { + name: "callback query with message", + update: &tgapi.Update{ + Type: tgapi.UpdateTypeCallbackQuery, + CallbackQuery: &tgapi.CallbackQuery{ + ID: "cb-1", + From: tgapi.User{ID: 106}, + Message: &tgapi.Message{ + MessageID: 77, + Chat: &tgapi.Chat{ID: 1005}, + }, + }, + }, + wantMsg: true, + wantFrom: true, + wantFromID: 106, + wantCallbackID: "cb-1", + wantCallbackMsgID: 77, + }, + { + name: "callback query with inline message", + update: &tgapi.Update{ + Type: tgapi.UpdateTypeCallbackQuery, + CallbackQuery: &tgapi.CallbackQuery{ + ID: "cb-2", + From: tgapi.User{ID: 107}, + InlineMessageID: ptr("inline-42"), + }, + }, + wantFrom: true, + wantFromID: 107, + wantCallbackID: "cb-2", + wantInlineMsgID:"inline-42", + }, + { + name: "shipping query", + update: &tgapi.Update{ + Type: tgapi.UpdateTypeShippingQuery, + ShippingQuery: &tgapi.ShippingQuery{ID: "ship", From: tgapi.User{ID: 108}}, + }, + wantFrom: true, + wantFromID: 108, + }, + { + name: "pre checkout query", + update: &tgapi.Update{ + Type: tgapi.UpdateTypePreCheckoutQuery, + PreCheckoutQuery: &tgapi.PreCheckoutQuery{ID: "pre", From: tgapi.User{ID: 109}}, + }, + wantFrom: true, + wantFromID: 109, + }, + { + name: "purchased paid media", + update: &tgapi.Update{ + Type: tgapi.UpdateTypePurchasedPaidMedia, + PurchasedPaidMedia: &tgapi.PaidMediaPurchased{From: tgapi.User{ID: 110}}, + }, + wantFrom: true, + wantFromID: 110, + }, + { + name: "my chat member", + update: &tgapi.Update{ + Type: tgapi.UpdateTypeMyChatMember, + MyChatMember: &tgapi.ChatMemberUpdated{From: tgapi.User{ID: 111}}, + }, + wantFrom: true, + wantFromID: 111, + }, + { + name: "chat member", + update: &tgapi.Update{ + Type: tgapi.UpdateTypeChatMember, + ChatMember: &tgapi.ChatMemberUpdated{From: tgapi.User{ID: 112}}, + }, + wantFrom: true, + wantFromID: 112, + }, + { + name: "chat join request", + update: &tgapi.Update{ + Type: tgapi.UpdateTypeChatJoinRequest, + ChatJoinRequest: &tgapi.ChatJoinRequest{From: tgapi.User{ID: 113}}, + }, + wantFrom: true, + wantFromID: 113, + }, + { + name: "business connection", + update: &tgapi.Update{ + Type: tgapi.UpdateTypeBusinessConnection, + BusinessConnection: &tgapi.BusinessConnection{User: tgapi.User{ID: 114}}, + }, + wantFrom: true, + wantFromID: 114, + }, + { + name: "poll answer", + update: &tgapi.Update{ + Type: tgapi.UpdateTypePollAnswer, + PollAnswer: &tgapi.PollAnswer{User: tgapi.User{ID: 115}}, + }, + wantFrom: true, + wantFromID: 115, + }, + { + name: "message reaction", + update: &tgapi.Update{ + Type: tgapi.UpdateTypeMessageReaction, + MessageReaction: &tgapi.MessageReactionUpdated{User: &tgapi.User{ID: 116}}, + }, + wantFrom: true, + wantFromID: 116, + }, + { + name: "chat boost", + update: &tgapi.Update{ + Type: tgapi.UpdateTypeChatBoost, + ChatBoost: &tgapi.ChatBoostUpdated{ + Boost: tgapi.ChatBoost{Source: tgapi.ChatBoostSource{User: tgapi.User{ID: 117}}}, + }, + }, + wantFrom: true, + wantFromID: 117, + }, + { + name: "removed chat boost", + update: &tgapi.Update{ + Type: tgapi.UpdateTypeRemovedChatBoost, + RemovedChatBoost: &tgapi.ChatBoostRemoved{ + Source: tgapi.ChatBoostSource{User: tgapi.User{ID: 118}}, + }, + }, + wantFrom: true, + wantFromID: 118, + }, + { + name: "poll", + update: &tgapi.Update{ + Type: tgapi.UpdateTypePoll, + Poll: &tgapi.Poll{ID: "poll"}, + }, + }, + { + name: "message reaction count", + update: &tgapi.Update{ + Type: tgapi.UpdateTypeMessageReactionCount, + MessageReactionCount: &tgapi.MessageReactionCountUpdated{}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + bot := &Bot[NoDB]{} + ctx := &MsgContext{} + bot.prepareUpdateCtx(tt.update, ctx) + + if got := ctx.Msg != nil; got != tt.wantMsg { + t.Fatalf("unexpected Msg presence: got %v want %v", got, tt.wantMsg) + } + if got := ctx.From != nil; got != tt.wantFrom { + t.Fatalf("unexpected From presence: got %v want %v", got, tt.wantFrom) + } + if ctx.FromID != tt.wantFromID { + t.Fatalf("unexpected FromID: got %d want %d", ctx.FromID, tt.wantFromID) + } + if ctx.CallbackQueryId != tt.wantCallbackID { + t.Fatalf("unexpected CallbackQueryId: got %q want %q", ctx.CallbackQueryId, tt.wantCallbackID) + } + if ctx.CallbackMsgId != tt.wantCallbackMsgID { + t.Fatalf("unexpected CallbackMsgId: got %d want %d", ctx.CallbackMsgId, tt.wantCallbackMsgID) + } + if ctx.InlineMsgId != tt.wantInlineMsgID { + t.Fatalf("unexpected InlineMsgId: got %q want %q", ctx.InlineMsgId, tt.wantInlineMsgID) + } + if ctx.Text != "" { + t.Fatalf("prepareUpdateCtx must not populate Text, got %q", ctx.Text) + } + if len(ctx.Args) != 0 { + t.Fatalf("prepareUpdateCtx must not populate Args, got %v", ctx.Args) + } + }) + } +} + func TestHandleUpdateHandlersPopulateFromContext(t *testing.T) { tests := []struct { name string @@ -323,3 +596,207 @@ func TestPayloadHandlerBindArgsEndToEnd(t *testing.T) { t.Fatalf("unexpected bound payload input: got %#v want %#v", got, want) } } + +func TestHandleEditedMessageStaysOutOfCommandFlow(t *testing.T) { + commandCalled := false + updateCalled := false + + plugin := NewPlugin[NoDB]("test") + plugin.NewCommand(func(ctx *MsgContext, db NoDB) error { + commandCalled = true + return nil + }, "ping") + plugin.AddUpdateHandler(tgapi.UpdateTypeEditedMessage, func(ctx *MsgContext, db NoDB) error { + updateCalled = true + if ctx.Msg == nil { + t.Fatal("expected ctx.Msg in edited message handler") + } + if ctx.Text != "" { + t.Fatalf("expected empty Text in edited_message update handler, got %q", ctx.Text) + } + if len(ctx.Args) != 0 { + t.Fatalf("expected empty Args in edited_message update handler, got %v", ctx.Args) + } + return nil + }) + + bot := &Bot[NoDB]{ + logger: slog.CreateLogger(), + prefixes: []string{"/"}, + plugins: []Plugin[NoDB]{clonePlugin(plugin)}, + } + + bot.handle(context.Background(), &tgapi.Update{ + UpdateID: 20, + Type: tgapi.UpdateTypeEditedMessage, + EditedMessage: &tgapi.Message{ + MessageID: 1, + Text: "/ping", + From: &tgapi.User{ID: 1}, + Chat: &tgapi.Chat{ID: 42}, + }, + }) + + if commandCalled { + t.Fatal("edited_message must not enter command flow") + } + if !updateCalled { + t.Fatal("expected edited_message update handler to be called") + } +} + +func TestHandleEditedChannelPostStaysOutOfCommandFlow(t *testing.T) { + commandCalled := false + updateCalled := false + + plugin := NewPlugin[NoDB]("test") + plugin.NewCommand(func(ctx *MsgContext, db NoDB) error { + commandCalled = true + return nil + }, "ping") + plugin.AddUpdateHandler(tgapi.UpdateTypeEditedChannelPost, func(ctx *MsgContext, db NoDB) error { + updateCalled = true + if ctx.Msg == nil { + t.Fatal("expected ctx.Msg in edited channel post handler") + } + return nil + }) + + bot := &Bot[NoDB]{ + logger: slog.CreateLogger(), + prefixes: []string{"/"}, + plugins: []Plugin[NoDB]{clonePlugin(plugin)}, + } + + bot.handle(context.Background(), &tgapi.Update{ + UpdateID: 21, + Type: tgapi.UpdateTypeEditedChannelPost, + EditedChannelPost: &tgapi.Message{ + MessageID: 1, + Text: "/ping", + Chat: &tgapi.Chat{ID: -10042}, + }, + }) + + if commandCalled { + t.Fatal("edited_channel_post must not enter command flow") + } + if !updateCalled { + t.Fatal("expected edited_channel_post update handler to be called") + } +} + +func TestHandleCallbackPopulatesMessageTargets(t *testing.T) { + called := false + plugin := NewPlugin[NoDB]("test") + plugin.NewPayload(func(ctx *MsgContext, db NoDB) error { + called = true + if ctx.CallbackQueryId != "cb-msg" { + t.Fatalf("unexpected CallbackQueryId: %q", ctx.CallbackQueryId) + } + if ctx.CallbackMsgId != 55 { + t.Fatalf("unexpected CallbackMsgId: %d", ctx.CallbackMsgId) + } + if ctx.InlineMsgId != "" { + t.Fatalf("did not expect InlineMsgId, got %q", ctx.InlineMsgId) + } + if ctx.Msg == nil { + t.Fatal("expected callback message context") + } + if ctx.From == nil || ctx.FromID != 7 { + t.Fatalf("unexpected callback sender: %#v / %d", ctx.From, ctx.FromID) + } + if ctx.Text != "" { + t.Fatalf("callback flow must not populate Text, got %q", ctx.Text) + } + if got, want := ctx.Args, []string{"7", "ok"}; len(got) != len(want) || got[0] != want[0] || got[1] != want[1] { + t.Fatalf("unexpected callback args: got %v want %v", got, want) + } + return nil + }, "approve") + + bot := &Bot[NoDB]{ + logger: slog.CreateLogger(), + payloadType: BotPayloadJson, + plugins: []Plugin[NoDB]{clonePlugin(plugin)}, + } + + data, err := encodeJsonPayload(CallbackData{Command: "approve", Args: []string{"7", "ok"}}) + if err != nil { + t.Fatalf("encodeJsonPayload returned error: %v", err) + } + + bot.handle(context.Background(), &tgapi.Update{ + UpdateID: 30, + Type: tgapi.UpdateTypeCallbackQuery, + CallbackQuery: &tgapi.CallbackQuery{ + ID: "cb-msg", + Data: data, + From: tgapi.User{ID: 7}, + Message: &tgapi.Message{ + MessageID: 55, + Chat: &tgapi.Chat{ID: 77}, + }, + }, + }) + + if !called { + t.Fatal("expected payload handler to be called") + } +} + +func TestHandleCallbackPopulatesInlineTargets(t *testing.T) { + called := false + plugin := NewPlugin[NoDB]("test") + plugin.NewPayload(func(ctx *MsgContext, db NoDB) error { + called = true + if ctx.CallbackQueryId != "cb-inline" { + t.Fatalf("unexpected CallbackQueryId: %q", ctx.CallbackQueryId) + } + if ctx.CallbackMsgId != 0 { + t.Fatalf("did not expect CallbackMsgId, got %d", ctx.CallbackMsgId) + } + if ctx.InlineMsgId != "inline-55" { + t.Fatalf("unexpected InlineMsgId: %q", ctx.InlineMsgId) + } + if ctx.Msg != nil { + t.Fatalf("did not expect callback chat message context, got %#v", ctx.Msg) + } + if ctx.From == nil || ctx.FromID != 8 { + t.Fatalf("unexpected callback sender: %#v / %d", ctx.From, ctx.FromID) + } + if ctx.Text != "" { + t.Fatalf("callback flow must not populate Text, got %q", ctx.Text) + } + if got, want := ctx.Args, []string{"9"}; len(got) != len(want) || got[0] != want[0] { + t.Fatalf("unexpected callback args: got %v want %v", got, want) + } + return nil + }, "inline.approve") + + bot := &Bot[NoDB]{ + logger: slog.CreateLogger(), + payloadType: BotPayloadJson, + plugins: []Plugin[NoDB]{clonePlugin(plugin)}, + } + + data, err := encodeJsonPayload(CallbackData{Command: "inline.approve", Args: []string{"9"}}) + if err != nil { + t.Fatalf("encodeJsonPayload returned error: %v", err) + } + + bot.handle(context.Background(), &tgapi.Update{ + UpdateID: 31, + Type: tgapi.UpdateTypeCallbackQuery, + CallbackQuery: &tgapi.CallbackQuery{ + ID: "cb-inline", + Data: data, + From: tgapi.User{ID: 8}, + InlineMessageID: ptr("inline-55"), + }, + }) + + if !called { + t.Fatal("expected inline payload handler to be called") + } +} diff --git a/msg_context.go b/msg_context.go index fd1c5fb..31f488f 100644 --- a/msg_context.go +++ b/msg_context.go @@ -13,27 +13,59 @@ import ( "git.scuroneko.dev/scuroneko/slog" ) -// MsgContext holds the context for handling a Telegram message or callback query. -// It provides methods to respond, edit, delete, and translate messages, as well as -// manage inline keyboards and message drafts. +// MsgContext holds the normalized per-update context passed to command, payload, +// scene, middleware, and generic update handlers. +// +// MsgContext is populated from the current Telegram update before handler routing. +// Not every field is guaranteed for every update kind. In particular: +// - Update is always present. +// - Msg is populated only for update kinds that carry a Telegram message object. +// - From and FromID are populated only when the update exposes a user identity. +// - Text, Args, and Prefix are populated only by command or scene command routing. +// - CallbackQueryId, CallbackMsgId, and InlineMsgId are populated only for +// callback query handling when the corresponding callback targets exist. +// +// Helper methods on MsgContext may require a message-backed context. For example, +// reply helpers need Msg, while inline callback edit helpers can work through +// InlineMsgId when there is no chat message. type MsgContext struct { Api *tgapi.API Update tgapi.Update - Msg *tgapi.Message + // Msg is the normalized Telegram message for message-backed update kinds. + // It is nil for updates that do not include a message object. + Msg *tgapi.Message + // From is the normalized Telegram user for update kinds that expose one. + // It stays nil for sender-chat-only updates and update kinds without a user. 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 + // InlineMsgId is the inline message identifier for callback queries that target + // an inline message instead of a chat message. + InlineMsgId string + // CallbackMsgId is the message ID targeted by the current callback query when + // the callback comes from a chat message. + CallbackMsgId int + // CallbackQueryId is the Telegram callback query ID for payload handlers and + // callback-backed scene handlers. CallbackQueryId string - FromID int64 - Prefix string - Text string - Args []string + // FromID is the normalized sender ID when the current update exposes a user. + // It is zero when the update has no user identity. + FromID int64 + // Prefix is the matched command prefix for command routing and scene-local + // command routing. It is empty outside those flows. + Prefix string + // Text is the parsed command tail for command routing, the parsed scene-command + // tail for scene-local command routing, or the trimmed message text seen by a + // scene step/message handler. It is empty when the current routing path does + // not derive text input. + Text string + // Args contains parsed command or payload arguments for the current routing + // path. It is nil or empty when no argument vector is derived. + Args []string errorTemplate string l10n *L10n @@ -478,6 +510,13 @@ func (ctx *MsgContext) SendAction(action tgapi.ChatActionType) { // Internal helper that formats, sends, and logs an error. func (ctx *MsgContext) error(err error) { + if err == nil { + return + } + ctx.Logger.Errorln(err) + if IsInternalError(err) { + return + } text := fmt.Sprintf(ctx.errorTemplate, err.Error()) if ctx.CallbackQueryId != "" { @@ -485,7 +524,6 @@ func (ctx *MsgContext) error(err error) { } else { ctx.answer(text, nil, tgapi.ParseNone) } - ctx.Logger.Errorln(err) } // Error is an alias for error(). diff --git a/msg_context_test.go b/msg_context_test.go index d69ddd4..f0465d6 100644 --- a/msg_context_test.go +++ b/msg_context_test.go @@ -166,6 +166,164 @@ func TestBindArgsRejectsUnsupportedFieldTypes(t *testing.T) { } } +func TestErrorDefaultRemainsUserVisibleForMessageFlow(t *testing.T) { + var requests int + var gotBody map[string]any + + client := &http.Client{ + Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + requests++ + body, err := io.ReadAll(req.Body) + if err != nil { + t.Fatalf("failed to read request body: %v", err) + } + if err := json.Unmarshal(body, &gotBody); err != nil { + t.Fatalf("failed to decode request body: %v", err) + } + return &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: io.NopCloser(strings.NewReader(`{"ok":true,"result":{"message_id":9,"date":1}}`)), + }, nil + }), + } + + api := tgapi.NewAPI( + tgapi.NewAPIOpts("token"). + SetAPIUrl("https://example.test"). + SetHTTPClient(client), + ) + defer func() { + if err := api.Close(); err != nil { + t.Fatalf("Close returned error: %v", err) + } + }() + + ctx := &MsgContext{ + Api: api, + Msg: &tgapi.Message{Chat: &tgapi.Chat{ID: 42, Type: string(tgapi.ChatTypePrivate)}}, + Logger: slog.CreateLogger(), + errorTemplate: "Error: %s", + } + + ctx.error(errors.New("boom")) + + if requests != 1 { + t.Fatalf("expected one user-facing error reply, got %d requests", requests) + } + if got := gotBody["text"]; got != "Error: boom" { + t.Fatalf("unexpected error reply text: %v", got) + } +} + +func TestErrorInternalSkipsUserReplyForMessageFlow(t *testing.T) { + client := &http.Client{ + Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + t.Fatal("unexpected HTTP request for internal-only error") + return nil, nil + }), + } + + api := tgapi.NewAPI( + tgapi.NewAPIOpts("token"). + SetAPIUrl("https://example.test"). + SetHTTPClient(client), + ) + defer func() { + if err := api.Close(); err != nil { + t.Fatalf("Close returned error: %v", err) + } + }() + + ctx := &MsgContext{ + Api: api, + Msg: &tgapi.Message{Chat: &tgapi.Chat{ID: 42, Type: string(tgapi.ChatTypePrivate)}}, + Logger: slog.CreateLogger(), + errorTemplate: "Error: %s", + } + + ctx.error(AsInternalError(errors.New("boom"))) +} + +func TestErrorInternalSkipsCallbackAnswer(t *testing.T) { + client := &http.Client{ + Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + t.Fatal("unexpected callback answer request for internal-only error") + return nil, nil + }), + } + + api := tgapi.NewAPI( + tgapi.NewAPIOpts("token"). + SetAPIUrl("https://example.test"). + SetHTTPClient(client), + ) + defer func() { + if err := api.Close(); err != nil { + t.Fatalf("Close returned error: %v", err) + } + }() + + ctx := &MsgContext{ + Api: api, + Logger: slog.CreateLogger(), + errorTemplate: "%s", + CallbackQueryId: "cb-1", + } + + ctx.error(AsInternalError(errors.New("boom"))) +} + +func TestErrorUserVisibleAnswersCallback(t *testing.T) { + var requests int + var gotBody map[string]any + + client := &http.Client{ + Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + requests++ + body, err := io.ReadAll(req.Body) + if err != nil { + t.Fatalf("failed to read request body: %v", err) + } + if err := json.Unmarshal(body, &gotBody); err != nil { + t.Fatalf("failed to decode request body: %v", err) + } + return &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: io.NopCloser(strings.NewReader(`{"ok":true,"result":true}`)), + }, nil + }), + } + + api := tgapi.NewAPI( + tgapi.NewAPIOpts("token"). + SetAPIUrl("https://example.test"). + SetHTTPClient(client), + ) + defer func() { + if err := api.Close(); err != nil { + t.Fatalf("Close returned error: %v", err) + } + }() + + ctx := &MsgContext{ + Api: api, + Logger: slog.CreateLogger(), + errorTemplate: "Oops: %s", + CallbackQueryId: "cb-1", + } + + ctx.error(AsUserError(errors.New("boom"))) + + if requests != 1 { + t.Fatalf("expected one callback error answer, got %d requests", requests) + } + if got := gotBody["text"]; got != "Oops: boom" { + t.Fatalf("unexpected callback error text: %v", got) + } +} + func TestAnswerRejectsEmptyMessage(t *testing.T) { ctx := &MsgContext{ Msg: &tgapi.Message{Chat: &tgapi.Chat{ID: 42, Type: string(tgapi.ChatTypePrivate)}}, diff --git a/plugins.go b/plugins.go index 75783aa..3d4067e 100644 --- a/plugins.go +++ b/plugins.go @@ -40,6 +40,11 @@ var ErrCmdArgCountMismatch = errors.New("command arg count mismatch") // ErrCmdArgRegexpMismatch is returned when an argument fails regex validation. var ErrCmdArgRegexpMismatch = errors.New("command arg regexp mismatch") +var ( + errCommandNotFound = errors.New("command not found") + errPayloadNotFound = errors.New("payload not found") +) + // CommandArg defines a single argument for a command, including type, regex, // and whether it is required. type CommandArg struct { @@ -314,7 +319,7 @@ func (p *Plugin[T]) Close() error { func (p *Plugin[T]) executeCmd(cmd string, ctx *MsgContext, db T) { command, exists := p.commands[cmd] if !exists { - ctx.error(errors.New("command not found")) + ctx.error(AsInternalError(errCommandNotFound)) return } @@ -340,7 +345,7 @@ func (p *Plugin[T]) executeCmd(cmd string, ctx *MsgContext, db T) { func (p *Plugin[T]) executePayload(payload string, ctx *MsgContext, db T) { command, exists := p.payloads[payload] if !exists { - ctx.error(errors.New("payload not found")) + ctx.error(AsInternalError(errPayloadNotFound)) return } diff --git a/utils/version.go b/utils/version.go index fb41955..40e5010 100644 --- a/utils/version.go +++ b/utils/version.go @@ -2,7 +2,7 @@ package utils const ( // VersionString is the module version string. - VersionString = "1.0.0-rc.12" + VersionString = "1.0.0-rc.13" // VersionMajor is the module major version. VersionMajor = 1 // VersionMinor is the module minor version. @@ -10,5 +10,5 @@ const ( // VersionPatch is the module patch version. VersionPatch = 0 // VersionBeta is the prerelease counter for the current version. - VersionBeta = 12 + VersionBeta = 13 )