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 9086fdc45f42b082e4f50fef2d5f74dd84606279..d02aff04b539507225a0ab858822abb98fd38adf 100644 (file)
--- a/botapi.go
+++ b/botapi.go
@@ -21,6 +21,13 @@ const (
        // take before the bot gives it up.
        pollSlack = 20 * time.Second
 
        // 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
 )
        // 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
 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 {
 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
 }
 
 // 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{
 // 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
 }
 
        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 {
 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)
 }
 
        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 {
 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",
        return b.call(ctx, "sendChatAction", map[string]any{
                "chat_id": chat,
                "action":  "typing",

diff --git a/botapi_test.go b/botapi_test.go
index b571c6ae0e625fc1d6b1704ad40f9f60be901a82..e1827471b425478352ce0200897b84f22e026dc7 100644 (file)
--- a/botapi_test.go
+++ b/botapi_test.go
@@ -1,6 +1,7 @@
 package main
 
 import (
 package main
 
 import (

+       "context"

        "encoding/json"
        "fmt"
        "io"
        "encoding/json"
        "fmt"
        "io"


@@ -56,6 +57,33 @@ func TestSendMessageSendsPlainText(t *testing.T) {
        }, server.Calls()[0].params)
 }
 
        }, 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
 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
 // 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
 
 type fakeBot struct {
        *httptest.Server
 


@@ -151,7 +180,9 @@ type fakeBot struct {
        updates []botUpdate
        calls   []fakeBotCall
        fail    func(call fakeBotCall) (int, string)
        updates []botUpdate
        calls   []fakeBotCall
        fail    func(call fakeBotCall) (int, string)

+       stall   func(call fakeBotCall) bool

        added   chan struct{}
        added   chan struct{}

+       stopped chan struct{}

 }
 
 // fakeBotCall is one call of the fake Bot API. OK tells if the call got a
 }
 
 // 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{
        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)
        }
 
        f.Server = httptest.NewServer(http.HandlerFunc(f.serve))
        t.Cleanup(f.Close)

+       t.Cleanup(func() {
+               close(f.stopped)
+       })

 
        return f
 }
 
        return f
 }


@@ -233,8 +268,19 @@ func (f *fakeBot) serve(w http.ResponseWriter, r *http.Request) {
                status, description = f.fail(call)
        }
 
                status, description = f.fail(call)
        }
 

+       stall := f.stall != nil && f.stall(call)

        f.mutex.Unlock()
 
        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)
        if status != 0 {
                f.record(call)
                w.WriteHeader(status)

diff --git a/telegram_test.go b/telegram_test.go
index d18e7948e2b27f430b4fec071a6fe7a1391aa6ae..b1cd6ae7348b9c9aedccb77678330fa55a12d54a 100644 (file)
--- a/telegram_test.go
+++ b/telegram_test.go
@@ -202,6 +202,29 @@ func TestBotTriesAgainAfterFailures(t *testing.T) {
        require.NotContains(t, logs.String(), testBotToken)
 }
 
        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()
 func TestBotLogsNoTokenWhenTheNetworkFails(t *testing.T) {
        telegram := newFakeBot(t)
        telegram.Close()