From: Anderson A. Giacomolli Date: Sat, 7 Nov 2026 13:51:13 +0000 (-0300) Subject: Limit the time of each Telegram send call X-Git-Url: http://git.giacomolli.uk/onechat/commitdiff_plain/a345df4e4bbf9dbaf3da1ce548896eef8f515de3?ds=inline Limit the time of each Telegram send call Each sendMessage and sendChatAction call gets its own limit of 30 s, through a child context of the call. Before, only getUpdates had a limit, so a connection that stopped without a close, such as after a network change, held the sender until TCP gave up, and no reply, error or typing action reached the chat in that time. A call that times out fails with the same network error as the other network failures, so the sender waits and tries again, and the error text holds no token and no URL. The HTTP client keeps no timeout of its own, because the long poll of getUpdates needs a longer limit. A stop of the daemon still ends a call in progress at once, because the limit is a child of the daemon context. Issue: a143bd8 --- diff --git a/botapi.go b/botapi.go index 9086fdc..d02aff0 100644 --- a/botapi.go +++ b/botapi.go @@ -21,6 +21,13 @@ const ( // take before the bot gives it up. pollSlack = 20 * time.Second + // sendTimeout is how long a sendMessage or sendChatAction call can + // take. A connection that stops without a close, such as after a + // network change, then fails the call, and the sender tries again, + // instead of a wait until TCP gives up. The client has no timeout of + // its own, because the long poll of getUpdates needs a longer limit. + sendTimeout = 30 * time.Second + // botAnswerBytes is the most bytes of one answer of the Bot API. botAnswerBytes = 16 << 20 ) @@ -30,11 +37,13 @@ const ( var errBotNetwork = errors.New("no valid answer from the Bot API") // botAPI calls the Telegram Bot API. The bot token is a part of each URL, so no -// error of botAPI holds a URL, a net/http error, or the token. +// error of botAPI holds a URL, a net/http error, or the token. SendTimeout +// limits each sendMessage and sendChatAction call. type botAPI struct { - base string - token string - http *http.Client + base string + token string + http *http.Client + sendTimeout time.Duration } // botError is an answer of the Bot API that tells of a failure. RetryAfter is @@ -86,9 +95,10 @@ type botUser struct { // newBotAPI returns the client of the Bot API at base with token. func newBotAPI(base, token string) *botAPI { return &botAPI{ - base: base, - token: token, - http: &http.Client{}, + base: base, + token: token, + http: &http.Client{}, + sendTimeout: sendTimeout, } } @@ -109,16 +119,24 @@ func (b *botAPI) GetUpdates(ctx context.Context, offset int64) ([]botUpdate, err return updates, err } -// SendMessage sends text to chat as plain text, with no parse mode. +// SendMessage sends text to chat as plain text, with no parse mode. A call that +// takes longer than sendTimeout fails with errBotNetwork. func (b *botAPI) SendMessage(ctx context.Context, chat int64, text string) error { + ctx, cancel := context.WithTimeout(ctx, b.sendTimeout) + defer cancel() + return b.call(ctx, "sendMessage", map[string]any{ "chat_id": chat, "text": text, }, nil) } -// SendTyping shows the typing action in chat for about five seconds. +// SendTyping shows the typing action in chat for about five seconds. A call +// that takes longer than sendTimeout fails with errBotNetwork. func (b *botAPI) SendTyping(ctx context.Context, chat int64) error { + ctx, cancel := context.WithTimeout(ctx, b.sendTimeout) + defer cancel() + return b.call(ctx, "sendChatAction", map[string]any{ "chat_id": chat, "action": "typing", diff --git a/botapi_test.go b/botapi_test.go index b571c6a..e182747 100644 --- a/botapi_test.go +++ b/botapi_test.go @@ -1,6 +1,7 @@ package main import ( + "context" "encoding/json" "fmt" "io" @@ -56,6 +57,33 @@ func TestSendMessageSendsPlainText(t *testing.T) { }, server.Calls()[0].params) } +func TestSendMessageTimesOutOnAStalledConnection(t *testing.T) { + server := newFakeBot(t) + server.stall = func(call fakeBotCall) bool { + return call.method == "sendMessage" + } + + api := newBotAPI(server.URL, testBotToken) + api.sendTimeout = 50 * time.Millisecond + start := time.Now() + err := api.SendMessage(t.Context(), testOwner, "hi") + require.ErrorIs(t, err, errBotNetwork) + require.Less(t, time.Since(start), 5*time.Second) + require.NotContains(t, err.Error(), testBotToken) + require.NotContains(t, err.Error(), server.URL) + + retry, _ := retryable(err) + require.True(t, retry) + + api.sendTimeout = time.Hour + ctx, cancel := context.WithCancel(t.Context()) + time.AfterFunc(50*time.Millisecond, cancel) + start = time.Now() + err = api.SendMessage(ctx, testOwner, "hi") + require.Error(t, err) + require.Less(t, time.Since(start), 5*time.Second) +} + func TestBotErrorsDoNotHoldTheToken(t *testing.T) { testCases := []struct { name string @@ -142,8 +170,9 @@ func TestBotErrorsDoNotHoldTheToken(t *testing.T) { // fakeBot is a fake Bot API server for testBotToken. getUpdates returns the // added updates from the offset on, or waits a short time when there are none. // The other methods succeed, unless fail gives an error answer for a call. An -// answer with HTTP 429 asks for a wait of one second. Fail runs with the mutex -// held. +// answer with HTTP 429 asks for a wait of one second. When stall is true for a +// call, the call gets no answer until the client gives it up or the test ends. +// Fail and stall run with the mutex held. type fakeBot struct { *httptest.Server @@ -151,7 +180,9 @@ type fakeBot struct { updates []botUpdate calls []fakeBotCall fail func(call fakeBotCall) (int, string) + stall func(call fakeBotCall) bool added chan struct{} + stopped chan struct{} } // fakeBotCall is one call of the fake Bot API. OK tells if the call got a @@ -167,11 +198,15 @@ func newFakeBot(t *testing.T) *fakeBot { t.Helper() f := &fakeBot{ - added: make(chan struct{}, 1), + added: make(chan struct{}, 1), + stopped: make(chan struct{}), } f.Server = httptest.NewServer(http.HandlerFunc(f.serve)) t.Cleanup(f.Close) + t.Cleanup(func() { + close(f.stopped) + }) return f } @@ -233,8 +268,19 @@ func (f *fakeBot) serve(w http.ResponseWriter, r *http.Request) { status, description = f.fail(call) } + stall := f.stall != nil && f.stall(call) f.mutex.Unlock() + if stall { + f.record(call) + select { + case <-r.Context().Done(): + case <-f.stopped: + } + + return + } + if status != 0 { f.record(call) w.WriteHeader(status) diff --git a/telegram_test.go b/telegram_test.go index d18e794..b1cd6ae 100644 --- a/telegram_test.go +++ b/telegram_test.go @@ -202,6 +202,29 @@ func TestBotTriesAgainAfterFailures(t *testing.T) { require.NotContains(t, logs.String(), testBotToken) } +func TestDeliverTriesAgainAfterATimeout(t *testing.T) { + telegram := newFakeBot(t) + stalls := 1 + telegram.stall = func(call fakeBotCall) bool { + if call.method != "sendMessage" || stalls == 0 { + return false + } + + stalls-- + + return true + } + + bot, logs := startTestBot(t, telegram) + bot.api.sendTimeout = 50 * time.Millisecond + bot.deliver(t.Context(), "hello") + + require.Equal(t, []string{"hello"}, telegram.Sent()) + require.Len(t, telegram.Calls(), 2) + require.Contains(t, logs.String(), "no valid answer") + require.NotContains(t, logs.String(), testBotToken) +} + func TestBotLogsNoTokenWhenTheNetworkFails(t *testing.T) { telegram := newFakeBot(t) telegram.Close()