Browse Source

fix(tgbot): answer the callbacks the bot cannot route (#6493)

Telegram keeps a tapped button in its loading state until the callback is
answered, and three paths dropped the tap without answering: a payload that
matched no case in the email-scoped switch, one that matched nothing in the
switch that follows it, and a button whose hash had aged out of the 20-minute
storage, which replied with a chat message only.

All three answer now. The email-scoped switch is reached only by an admin's
payload carrying arguments, and today an unknown action there falls out of the
switch into a return that tells the operator nothing.

The expired-hash path keeps the chat message as well, because
sendCallbackAnswerTgBot is a single unretried call while SendMsgToTgbot retries
connection errors, and after a panel restart that notice is the only
explanation the admin gets.
BlindMaster24 13 hours ago
parent
commit
aaa5e61cad

+ 81 - 0
internal/web/service/tgbot/tgbot_declined_callback_test.go

@@ -0,0 +1,81 @@
+package tgbot
+
+import (
+	"testing"
+	"time"
+
+	"github.com/mhsanaei/3x-ui/v3/internal/web/global"
+
+	"github.com/mymmrac/telego"
+)
+
+func tapCallback(t *testing.T, tb *Tgbot, isAdmin bool, data string) {
+	t.Helper()
+	tb.answerCallback(&telego.CallbackQuery{
+		ID:      "q1",
+		From:    telego.User{ID: 1},
+		Data:    data,
+		Message: &telego.Message{Chat: telego.Chat{ID: 1}},
+	}, isAdmin)
+}
+
+// decliningServer answers both methods a declined callback can reach, and
+// leaves isRunning set so a notice would go out if the code decided to send one.
+func decliningServer(t *testing.T) func(string) int {
+	t.Helper()
+	mock, calls := staleButtonServer(t, map[string]any{
+		"answerCallbackQuery": map[string]any{"ok": true, "result": true},
+		"sendMessage": map[string]any{"ok": true, "result": map[string]any{
+			"message_id": 1,
+			"date":       0,
+			"chat":       map[string]any{"id": 1, "type": "private"},
+		}},
+	})
+	swapTestBot(t, mock.URL)
+	t.Cleanup(mock.Close)
+
+	origRunning := isRunning
+	t.Cleanup(func() { isRunning = origRunning })
+	isRunning = true
+
+	return calls
+}
+
+// Regression test: a payload the bot could not route was dropped without an
+// answer, so Telegram kept the button spinning until the callback timed out.
+func TestUnroutableCallbackIsAnswered(t *testing.T) {
+	for _, data := range []string{"no_such_callback", "get_backup_legacy", "add_client_legacy_step 5"} {
+		t.Run(data, func(t *testing.T) {
+			calls := decliningServer(t)
+
+			tapCallback(t, &Tgbot{}, true, data)
+
+			if n := calls("answerCallbackQuery"); n != 1 {
+				t.Errorf("answerCallbackQuery calls = %d, want 1: an unroutable tap must be answered", n)
+			}
+			if n := calls("sendMessage"); n != 0 {
+				t.Errorf("sendMessage calls = %d, want 0: an answer is not a posted message", n)
+			}
+		})
+	}
+}
+
+// Regression test: a button whose hash aged out was answered with nothing at all;
+// the answer clears it and the chat notice survives a failed send.
+func TestExpiredCallbackHashIsAnsweredAndReported(t *testing.T) {
+	calls := decliningServer(t)
+
+	origHash := hashStorage
+	hashStorage = global.NewHashStorage(time.Minute)
+	t.Cleanup(func() { hashStorage = origHash })
+
+	// 32 hex characters — the shape decodeQuery looks up, and never stored.
+	tapCallback(t, &Tgbot{}, true, "0123456789abcdef0123456789abcdef")
+
+	if n := calls("answerCallbackQuery"); n != 1 {
+		t.Errorf("answerCallbackQuery calls = %d, want 1: an expired button must be answered", n)
+	}
+	if n := calls("sendMessage"); n != 1 {
+		t.Errorf("sendMessage calls = %d, want 1: the notice is the durable half of the reply", n)
+	}
+}

+ 11 - 0
internal/web/service/tgbot/tgbot_router.go

@@ -316,6 +316,9 @@ func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool
 		// get query from hash storage
 		decodedQuery, err := t.decodeQuery(callbackQuery.Data)
 		if err != nil {
+			// A button older than the 20-minute hash window is the common case
+			// here; the answer clears it, the message outlives a failed send.
+			t.sendCallbackAnswerTgBot(callbackQuery.ID, t.I18nBot("tgbot.noQuery"))
 			t.SendMsgToTgbot(chatId, t.I18nBot("tgbot.noQuery"))
 			return
 		}
@@ -884,6 +887,10 @@ func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool
 					return
 				}
 				t.editMessageCallbackTgBot(callbackQuery.Message.GetChat().ID, callbackQuery.Message.GetMessageID(), picker)
+			default:
+				// An unknown action with arguments is still a tap, and an
+				// unanswered tap spins until Telegram times it out.
+				t.sendCallbackAnswerTgBot(callbackQuery.ID, t.I18nBot("tgbot.answers.errorOperation"))
 			}
 			return
 		} else {
@@ -1327,6 +1334,10 @@ func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool
 		case "client_qr_links":
 			t.sendClientQRLinks(chatId, email)
 		}
+
+		// Nothing matched: an unknown button still has to be answered, or it
+		// keeps spinning until Telegram times the callback out.
+		t.sendCallbackAnswerTgBot(callbackQuery.ID, t.I18nBot("tgbot.answers.errorOperation"))
 	}
 }