fix(tgbot): recover menus when cached artwork is rejected (#6968)

When Telegram rejected a cached decorative photo, /start returned without a screen and edits failed the same way. A Bot API 400 on cached artwork now retries once with the generated PNG, keeping caption, keyboard and target, and the stale cached id is dropped only after the fallback succeeds. Refs #6943.
This commit is contained in:
Hamed
2026-11-02 17:44:03 +03:30
committed by GitHub
parent d2450e103c
commit 4d583cebf3
3 changed files with 447 additions and 51 deletions
+54 -49
View File
@@ -2,6 +2,7 @@ package tgbot
import (
"context"
"errors"
"strings"
"sync"
"time"
@@ -9,6 +10,7 @@ import (
"github.com/mhsanaei/3x-ui/v3/internal/logger"
"github.com/mymmrac/telego"
"github.com/mymmrac/telego/telegoapi"
tu "github.com/mymmrac/telego/telegoutil"
)
@@ -185,37 +187,13 @@ func (t *Tgbot) editScreen(chatID int64, prev *screen, next *screen) bool {
var err error
switch {
case next.hasPhoto && next.photo != "":
_, err = liveBot().EditMessageMedia(ctx, &telego.EditMessageMediaParams{
ChatID: tu.ID(chatID),
MessageID: prev.msgID,
Media: &telego.InputMediaPhoto{
Type: "photo", Media: tu.FileFromID(next.photo), Caption: next.text, ParseMode: "HTML",
},
ReplyMarkup: next.markup,
})
case next.attachment != nil:
_, err = liveBot().EditMessageMedia(ctx, &telego.EditMessageMediaParams{
ChatID: tu.ID(chatID),
MessageID: prev.msgID,
Media: &telego.InputMediaPhoto{
Type: "photo", Media: *next.attachment, Caption: next.text, ParseMode: "HTML",
},
ReplyMarkup: next.markup,
})
case next.hasPhoto:
// No file_id yet (the avatar was never read): upload the fallback tile.
upload, ok := artUpload(next.kind)
if !ok {
return false
}
_, err = liveBot().EditMessageMedia(ctx, &telego.EditMessageMediaParams{
ChatID: tu.ID(chatID),
MessageID: prev.msgID,
Media: &telego.InputMediaPhoto{
Type: "photo", Media: upload, Caption: next.text, ParseMode: "HTML",
},
ReplyMarkup: next.markup,
case next.hasPhoto || next.attachment != nil:
_, err = requestScreenPhoto(next, func(photo telego.InputFile) (*telego.Message, error) {
return liveBot().EditMessageMedia(ctx, &telego.EditMessageMediaParams{
ChatID: tu.ID(chatID), MessageID: prev.msgID,
Media: &telego.InputMediaPhoto{Type: "photo", Media: photo, Caption: next.text, ParseMode: "HTML"},
ReplyMarkup: next.markup,
})
})
case prev.hasPhoto:
// A photo message cannot become a text one: drop the picture and let
@@ -250,24 +228,12 @@ func (t *Tgbot) replaceScreen(chatID int64, sc *screen) int {
var sent *telego.Message
var err error
switch {
case sc.hasPhoto && sc.photo != "":
sent, err = liveBot().SendPhoto(ctx, &telego.SendPhotoParams{
ChatID: tu.ID(chatID), MessageThreadID: adminTopic(chatID), Photo: tu.FileFromID(sc.photo),
Caption: sc.text, ParseMode: "HTML", ReplyMarkup: sc.markup,
})
case sc.attachment != nil:
sent, err = liveBot().SendPhoto(ctx, &telego.SendPhotoParams{
ChatID: tu.ID(chatID), MessageThreadID: adminTopic(chatID), Photo: *sc.attachment,
Caption: sc.text, ParseMode: "HTML", ReplyMarkup: sc.markup,
})
case sc.hasPhoto:
upload, ok := artUpload(sc.kind)
if !ok {
return 0
}
sent, err = liveBot().SendPhoto(ctx, &telego.SendPhotoParams{
ChatID: tu.ID(chatID), MessageThreadID: adminTopic(chatID), Photo: upload,
Caption: sc.text, ParseMode: "HTML", ReplyMarkup: sc.markup,
case sc.hasPhoto || sc.attachment != nil:
sent, err = requestScreenPhoto(sc, func(photo telego.InputFile) (*telego.Message, error) {
return liveBot().SendPhoto(ctx, &telego.SendPhotoParams{
ChatID: tu.ID(chatID), MessageThreadID: adminTopic(chatID), Photo: photo,
Caption: sc.text, ParseMode: "HTML", ReplyMarkup: sc.markup,
})
})
default:
sent, err = liveBot().SendMessage(ctx, &telego.SendMessageParams{
@@ -292,6 +258,45 @@ func (t *Tgbot) replaceScreen(chatID int64, sc *screen) int {
return sent.MessageID
}
// Only a rejected cached decoration gets one upload retry; QR pictures remain intact.
// The cache is invalidated only after the same caption and keyboard succeed without that id.
func requestScreenPhoto(sc *screen, request func(telego.InputFile) (*telego.Message, error)) (*telego.Message, error) {
var photo telego.InputFile
switch {
case sc.hasPhoto && sc.photo != "":
photo = tu.FileFromID(sc.photo)
case sc.attachment != nil:
photo = *sc.attachment
default:
var ok bool
photo, ok = artUpload(sc.kind)
if !ok {
return nil, errors.New("screen artwork is unavailable")
}
}
sent, err := request(photo)
var apiErr *telegoapi.Error
if err == nil || sc.photo == "" || sc.qrPicture != nil || sc.attachment != nil ||
!errors.As(err, &apiErr) || apiErr.ErrorCode != 400 || isTelegramNotModifiedError(err) {
return sent, err
}
upload, ok := artUpload(sc.kind)
if !ok {
return sent, err
}
sent, err = request(upload)
if err != nil {
return sent, err
}
rejectArtFileID(sc.photo)
sc.photo = ""
if sent != nil && len(sent.Photo) > 0 {
sc.photo = sent.Photo[len(sent.Photo)-1].FileID
cacheArtFileID(sc.kind, sc.photo)
}
return sent, nil
}
// ensureBackRow guarantees every screen can be left (a screen with only its own
// actions traps the user) and checks by callback, not label.
@@ -0,0 +1,362 @@
package tgbot
import (
"encoding/json"
"fmt"
"image/png"
"net/http"
"net/http/httptest"
"reflect"
"strings"
"sync"
"testing"
"time"
"github.com/mymmrac/telego"
"github.com/nicksnyder/go-i18n/v2/i18n"
)
type screenMediaRequest struct {
Method string
ChatID int64 `json:"chat_id"`
MessageID int `json:"message_id"`
Caption string `json:"caption"`
Photo string `json:"photo"`
Media struct {
Media string `json:"media"`
Caption string `json:"caption"`
} `json:"media"`
ReplyMarkup telego.InlineKeyboardMarkup `json:"reply_markup"`
Uploaded bool
UploadName string
}
type screenMediaAPI struct {
mu sync.Mutex
avatar string
code int
failUpload bool
failEdit bool
badResponse bool
uploadStarted chan struct{}
releaseUpload <-chan struct{}
requests []screenMediaRequest
}
func newScreenMediaAPI(t *testing.T) (*Tgbot, *screenMediaAPI) {
t.Helper()
api := &screenMediaAPI{avatar: "rejected-avatar", code: 400}
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
method := r.URL.Path[strings.LastIndex(r.URL.Path, "/")+1:]
w.Header().Set("Content-Type", "application/json")
api.mu.Lock()
avatar := api.avatar
api.mu.Unlock()
switch method {
case "getMe":
_ = json.NewEncoder(w).Encode(map[string]any{"ok": true, "result": map[string]any{
"id": 42, "is_bot": true, "first_name": "test", "username": "test_bot", "supports_inline_queries": true,
}})
return
case "getUserProfilePhotos":
if avatar == "" {
_ = json.NewEncoder(w).Encode(map[string]any{"ok": true, "result": map[string]any{"total_count": 0, "photos": [][]telego.PhotoSize{}}})
return
}
_ = json.NewEncoder(w).Encode(map[string]any{"ok": true, "result": map[string]any{
"total_count": 1, "photos": [][]telego.PhotoSize{{{FileID: avatar, FileUniqueID: "avatar", Width: 640, Height: 640}}},
}})
return
}
request := screenMediaRequest{Method: method}
if strings.HasPrefix(r.Header.Get("Content-Type"), "multipart/form-data") {
if err := r.ParseMultipartForm(1 << 20); err != nil {
t.Errorf("parse upload: %v", err)
return
}
defer r.MultipartForm.RemoveAll()
request.ChatID = parseInt64(r.FormValue("chat_id"))
request.MessageID = int(parseInt64(r.FormValue("message_id")))
request.Caption, request.Photo = r.FormValue("caption"), r.FormValue("photo")
_ = json.Unmarshal([]byte(r.FormValue("reply_markup")), &request.ReplyMarkup)
_ = json.Unmarshal([]byte(r.FormValue("media")), &request.Media)
for _, files := range r.MultipartForm.File {
for _, file := range files {
reader, err := file.Open()
if err != nil {
t.Errorf("open uploaded photo: %v", err)
return
}
config, err := png.DecodeConfig(reader)
_ = reader.Close()
if err != nil || (file.Filename == "screen.png" && (config.Width != 512 || config.Height != 512)) {
t.Errorf("fallback is not the generated 512x512 PNG: %+v, %v", config, err)
}
request.Uploaded = true
request.UploadName = file.Filename
}
}
} else if err := json.NewDecoder(r.Body).Decode(&request); err != nil {
t.Errorf("decode %s: %v", method, err)
return
}
if method == "editMessageMedia" {
request.Caption = request.Media.Caption
request.Photo = request.Media.Media
}
api.mu.Lock()
api.requests = append(api.requests, request)
code, failUpload, failEdit := api.code, api.failUpload, api.failEdit
badResponse, started, release := api.badResponse, api.uploadStarted, api.releaseUpload
id := 100 + len(api.requests)
api.mu.Unlock()
if request.Uploaded && started != nil {
select {
case started <- struct{}{}:
default:
}
select {
case <-release:
case <-r.Context().Done():
return
}
}
if badResponse {
_, _ = w.Write([]byte("{"))
return
}
if (request.Photo == "rejected-avatar" && !request.Uploaded) || (failUpload && request.Uploaded) || (failEdit && method == "editMessageMedia") {
_ = json.NewEncoder(w).Encode(map[string]any{"ok": false, "error_code": code, "description": "Bad Request: wrong file identifier/HTTP URL specified"})
return
}
if method == "deleteMessage" {
_ = json.NewEncoder(w).Encode(map[string]any{"ok": true, "result": true})
return
}
if request.MessageID != 0 {
id = request.MessageID
}
photoID := request.Photo
if request.Uploaded {
photoID = "fallback-photo"
}
_ = json.NewEncoder(w).Encode(map[string]any{"ok": true, "result": map[string]any{
"message_id": id, "date": 0, "chat": map[string]any{"id": request.ChatID, "type": "private"},
"photo": []telego.PhotoSize{{FileID: photoID, FileUniqueID: photoID, Width: 512, Height: 512}},
}})
}))
t.Cleanup(srv.Close)
swapTestBot(t, srv.URL)
tb := &Tgbot{}
tb.resolveScreenArt()
return tb, api
}
func (api *screenMediaAPI) snapshot() []screenMediaRequest {
api.mu.Lock()
defer api.mu.Unlock()
return append([]screenMediaRequest(nil), api.requests...)
}
func TestStartRecoversWhenTelegramRejectsAvatar(t *testing.T) {
tb, api := newScreenMediaAPI(t)
initReportDB(t)
setTestAdmins(t, 1)
draftLocalizer(t,
&i18n.Message{ID: "tgbot.buttons.catServer", Other: "Server"},
&i18n.Message{ID: "tgbot.buttons.catClients", Other: "Clients"},
&i18n.Message{ID: "tgbot.buttons.catTraffic", Other: "Traffic"},
&i18n.Message{ID: "tgbot.buttons.catMaintenance", Other: "Maintenance"},
&i18n.Message{ID: "tgbot.buttons.refresh", Other: "Refresh"},
&i18n.Message{ID: "tgbot.messages.hostname", Other: "Server: {{.Hostname}}"},
)
tb.answerCommand(&telego.Message{Chat: telego.Chat{ID: 500, Type: "private"}, From: &telego.User{ID: 1}, Text: "/start"}, 500, true)
screen, ok := tb.screens().get(500)
if !ok || screen.msgID == 0 {
t.Fatal("/start left the chat without a menu after Telegram rejected its avatar")
}
requests := api.snapshot()
if len(requests) != 2 || !requests[1].Uploaded {
t.Fatalf("requests = %+v, want one failed file-id send and one fallback upload", requests)
}
if requests[0].Caption != requests[1].Caption || !reflect.DeepEqual(requests[0].ReplyMarkup, requests[1].ReplyMarkup) || requests[1].ChatID != 500 {
t.Fatalf("fallback changed the menu content or destination: %+v", requests)
}
if len(requests[1].ReplyMarkup.InlineKeyboard) != 3 || requests[1].ReplyMarkup.InlineKeyboard[0][0].CallbackData != cbCatServer {
t.Fatalf("fallback did not carry the admin menu: %+v", requests[1].ReplyMarkup)
}
if screen.photo != "fallback-photo" {
t.Errorf("tracked photo = %q, want reusable fallback-photo", screen.photo)
}
tb.resolveScreenArt()
tb.renderScreen(500, tb.newScreen("main", "next", tb.homeRows()...))
tb.renderScreen(501, tb.newScreen("status", "another screen", tb.homeRows()...))
for _, request := range api.snapshot()[2:] {
if request.Photo == "rejected-avatar" && !request.Uploaded {
t.Errorf("the unchanged rejected avatar was reused: %+v", request)
}
}
api.mu.Lock()
api.avatar = "new-avatar"
api.mu.Unlock()
tb.resolveScreenArt()
tb.renderScreen(500, tb.newScreen("main", "new avatar", tb.homeRows()...))
last := api.snapshot()
if last[len(last)-1].Photo != "new-avatar" || last[len(last)-1].Uploaded {
t.Errorf("a changed avatar was not adopted: %+v", last[len(last)-1])
}
}
func TestScreenMediaEditRecoversWithoutReplacingMessage(t *testing.T) {
tb, api := newScreenMediaAPI(t)
tb.screens().put(1, &screen{msgID: 9, kind: "main", hasPhoto: true})
tb.renderScreen(1, tb.newScreen("main", "updated", tb.homeRows()...))
requests := api.snapshot()
if len(requests) != 2 || requests[0].Method != "editMessageMedia" || requests[1].Method != "editMessageMedia" || !requests[1].Uploaded {
t.Fatalf("requests = %+v, want a bounded media-edit fallback on the existing message", requests)
}
tracked, _ := tb.screens().get(1)
if tracked.msgID != 9 || tracked.photo != "fallback-photo" {
t.Errorf("recovered screen = %+v, want message 9 with fallback-photo", tracked)
}
}
func TestScreenMediaFailureKeepsThePreviousMenuAndCache(t *testing.T) {
tb, api := newScreenMediaAPI(t)
api.mu.Lock()
api.failUpload, api.failEdit = true, true
api.mu.Unlock()
previous := &screen{msgID: 9, kind: "main", hasPhoto: true, photo: "rejected-avatar"}
tb.screens().put(1, previous)
tb.renderScreen(1, tb.newScreen("main", "update", tb.homeRows()...))
tracked, _ := tb.screens().get(1)
if tracked != previous || tracked.msgID != 9 {
t.Errorf("failed recovery replaced the previous menu: %+v", tracked)
}
if id, _ := artFileID("main"); id != "rejected-avatar" {
t.Errorf("failed upload poisoned the artwork cache: %q", id)
}
requests := api.snapshot()
if len(requests) > 4 {
t.Fatalf("requests = %+v, want at most one upload retry per edit/send operation", requests)
}
for _, request := range requests {
if request.Method == "deleteMessage" {
t.Fatal("the previous menu was deleted before a replacement succeeded")
}
}
}
func TestScreenMediaHealthyBotsKeepTheirExistingRendering(t *testing.T) {
for _, avatar := range []string{"valid-avatar", ""} {
t.Run(avatar, func(t *testing.T) {
tb, api := newScreenMediaAPI(t)
api.mu.Lock()
api.avatar = avatar
api.mu.Unlock()
resetScreenArt()
tb.resolveScreenArt()
tb.renderScreen(1, tb.newScreen("main", "first", tb.homeRows()...))
tb.renderScreen(1, tb.newScreen("main", "second", tb.homeRows()...))
requests := api.snapshot()
if len(requests) != 2 || requests[0].Method != "sendPhoto" || requests[1].Method != "editMessageMedia" {
t.Fatalf("healthy bot changed its send/edit behavior: %+v", requests)
}
wantPhoto := avatar
if avatar == "" {
wantPhoto = "fallback-photo"
if !requests[0].Uploaded {
t.Error("an avatar-less bot did not upload its usual fallback tile")
}
} else if requests[0].Uploaded {
t.Error("a valid avatar was unnecessarily replaced")
}
if requests[1].Photo != wantPhoto || requests[1].Uploaded || requests[1].Caption != "second" {
t.Errorf("healthy menu was not edited normally: %+v", requests[1])
}
})
}
}
func TestScreenMediaDoesNotRetryUncertainOrForbiddenFailures(t *testing.T) {
for _, code := range []int{403, 429, 500, 0} {
t.Run(fmt.Sprint(code), func(t *testing.T) {
tb, api := newScreenMediaAPI(t)
api.mu.Lock()
api.code, api.badResponse = code, code == 0
api.mu.Unlock()
tb.renderScreen(1, tb.newScreen("main", "menu", tb.homeRows()...))
requests := api.snapshot()
if len(requests) != 1 || requests[0].Uploaded {
t.Errorf("non-recoverable failure retried or uploaded art: %+v", requests)
}
if _, ok := tb.screens().get(1); ok {
t.Error("an unconfirmed send was recorded as a live screen")
}
if id, _ := artFileID("main"); id != "rejected-avatar" {
t.Errorf("non-media failure invalidated the avatar: %q", id)
}
})
}
}
func TestRejectedQRIsNeverReplacedByDecorativeArt(t *testing.T) {
for _, cached := range []bool{false, true} {
t.Run(fmt.Sprint(cached), func(t *testing.T) {
tb, api := newScreenMediaAPI(t)
api.mu.Lock()
api.failUpload = true
api.mu.Unlock()
sc := tb.newScreen("qr", "QR", tb.backRow())
sc.qrPicture = qrSheetPicture([]subscriptionQRCode{{kind: "sub", content: "https://example.invalid/sub"}})
if cached {
sc.photo = "rejected-avatar"
}
tb.renderScreen(1, sc)
requests := api.snapshot()
if len(requests) != 1 || requests[0].UploadName == "screen.png" {
t.Errorf("QR rejection substituted decorative artwork: %+v", requests)
}
if id, _ := artFileID("main"); id != "rejected-avatar" {
t.Errorf("QR failure invalidated unrelated menu artwork: %q", id)
}
})
}
}
func TestScreenMediaRecoveryPreservesAConcurrentlyRefreshedAvatar(t *testing.T) {
tb, api := newScreenMediaAPI(t)
started, release := make(chan struct{}, 1), make(chan struct{})
var once sync.Once
releaseUpload := func() { once.Do(func() { close(release) }) }
t.Cleanup(releaseUpload)
api.mu.Lock()
api.uploadStarted, api.releaseUpload = started, release
api.mu.Unlock()
done := make(chan struct{})
go func() {
defer close(done)
tb.renderScreen(1, tb.newScreen("main", "menu", tb.homeRows()...))
}()
select {
case <-started:
case <-time.After(5 * time.Second):
t.Fatal("fallback upload never started")
}
api.mu.Lock()
api.avatar = "new-avatar"
api.mu.Unlock()
tb.resolveScreenArt()
releaseUpload()
select {
case <-done:
case <-time.After(5 * time.Second):
t.Fatal("screen recovery never completed")
}
if id, _ := artFileID("main"); id != "new-avatar" {
t.Errorf("late recovery discarded the newly refreshed avatar: %q", id)
}
}
+31 -2
View File
@@ -28,8 +28,9 @@ var (
artOverrides = map[string]string{}
// artFromAvatar records whether the cached ids came from the bot's avatar: a
// tile cached from an upload must never win over an avatar set afterwards.
artFromAvatar bool
artProbedAt time.Time
artFromAvatar bool
artProbedAt time.Time
rejectedAvatarID string
// artProbeRunning keeps a burst of renders from starting a burst of probes.
artProbeRunning atomic.Bool
// artProbes lets resetScreenArt outlast a probe still running against a previous bot.
@@ -81,12 +82,17 @@ func (t *Tgbot) resolveScreenArt() {
// Remember the miss so the next render does not probe again, but keep
// any tile already cached: an avatar-less bot still shows a picture.
artFromAvatar = false
rejectedAvatarID = ""
logger.Debug("Bot has no avatar; screens will use the black fallback")
return
}
// The last entry is the largest size Telegram keeps.
sizes := photos.Photos[0]
fileID := sizes[len(sizes)-1].FileID
if fileID == rejectedAvatarID {
return
}
rejectedAvatarID = ""
screenArtIDs = map[string]string{}
for _, kind := range screenKinds {
screenArtIDs[kind] = fileID
@@ -163,6 +169,28 @@ func cacheArtFileID(kind, fileID string) {
screenArtMu.Unlock()
}
// Compare before eviction: an asynchronous probe may already have installed a new avatar.
func rejectArtFileID(fileID string) {
screenArtMu.Lock()
defer screenArtMu.Unlock()
for kind, id := range screenArtIDs {
if id == fileID {
delete(screenArtIDs, kind)
if artFromAvatar {
rejectedAvatarID = fileID
}
}
}
if rejectedAvatarID == fileID {
artFromAvatar = false
}
for kind, id := range artOverrides {
if id == fileID {
delete(artOverrides, kind)
}
}
}
// resetScreenArt drops the cached file_ids; tests use it between mock servers.
func resetScreenArt() {
artProbes.Wait()
@@ -170,6 +198,7 @@ func resetScreenArt() {
screenArtIDs = map[string]string{}
artOverrides = map[string]string{}
artFromAvatar = false
rejectedAvatarID = ""
artProbedAt = time.Time{}
screenArtMu.Unlock()
}