diff --git a/internal/delivery/telegramsink/telegramsink.go b/internal/delivery/telegramsink/telegramsink.go index 507f2a6..bbf157b 100644 --- a/internal/delivery/telegramsink/telegramsink.go +++ b/internal/delivery/telegramsink/telegramsink.go @@ -172,21 +172,49 @@ func (s *Sink) Send(ctx context.Context, d delivery.Sendable) error { return fmt.Errorf("telegramsink: sendMessage: %w", s.redact(err)) } defer resp.Body.Close() - rb, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) + rb, _ := io.ReadAll(io.LimitReader(resp.Body, maxRespBytes)) // telegram returns 200 with ok=true on success; non-2xx with ok=false + // error_code + description on failure. parse the body either way so a 200 // with ok=false (shouldn't happen, but the API reserves that) still surfaces. var tr telegramResp - if jsonErr := json.Unmarshal(rb, &tr); jsonErr == nil && !tr.Ok { + jsonErr := json.Unmarshal(rb, &tr) + if jsonErr == nil && !tr.Ok { return fmt.Errorf("telegramsink: telegram returned error %d: %s", tr.ErrorCode, strings.TrimSpace(tr.Description)) } if resp.StatusCode/100 != 2 { - return fmt.Errorf("telegramsink: telegram returned %d: %s", resp.StatusCode, strings.TrimSpace(string(rb))) + return fmt.Errorf("telegramsink: telegram returned %d: %s", resp.StatusCode, snippet(rb)) + } + // A 2xx whose body is not the bot API's envelope did not come from the bot + // API. The normal path here is the relay: this box reaches telegram through + // an HTTP/SOCKS5 proxy, and a proxy that is up but cannot reach + // api.telegram.org answers 200 with an HTML page of its own. Reading that as + // a delivered message is the worst outcome the sink has — the dispatcher + // writes a 'sent' outbox row, MarkSent restarts the repeat clock, and the + // sev4 alarm that never arrived goes quiet for a whole interval. Only + // ok=true is a send. + if jsonErr != nil { + return fmt.Errorf("telegramsink: telegram returned %d with a body that is not the bot API envelope (not a confirmed send): %s", resp.StatusCode, snippet(rb)) } return nil } +// maxRespBytes caps the response read — the body is wire-controlled and the +// relay in front of it is not telegram. It is far above any sendMessage +// envelope (a few hundred bytes; the result echoes one short away message), +// because a truncated body no longer parses and now reads as a failed send. +const maxRespBytes = 64 << 10 + +// snippet trims a response body down to something an error line can carry. A +// relay's HTML page is measured in kilobytes and none of it belongs in the log. +func snippet(rb []byte) string { + s := strings.TrimSpace(string(rb)) + if len(s) > 200 { + return s[:200] + "…" + } + return s +} + // sendMessageURL — the bot API path. the token is in the URL path // (https://api.telegram.org/bot/sendMessage); telegram does not accept // it anywhere else. the URL is built per-send from the resolved base and never diff --git a/internal/delivery/telegramsink/telegramsink_test.go b/internal/delivery/telegramsink/telegramsink_test.go index fae9536..d806fc4 100644 --- a/internal/delivery/telegramsink/telegramsink_test.go +++ b/internal/delivery/telegramsink/telegramsink_test.go @@ -291,6 +291,66 @@ func TestSendReturnsErrorOnTelegramError(t *testing.T) { } } +// A relay that is up but cannot reach api.telegram.org answers 200 with a page +// of its own. That is not a delivered message, and calling it one silences a +// sev4 alarm for a full repeat interval. +func TestSendRefusesA200ThatIsNotTheBotAPIEnvelope(t *testing.T) { + rs := newRecordingServer(t, http.StatusOK, `proxy: upstream unreachable`) + srv := httptest.NewServer(rs.handler()) + defer srv.Close() + + sink, _ := New(sinkCfg(srv.URL)) + err := sink.Send(context.Background(), nudgeSendable(loop.Sev4, "down")) + if err == nil { + t.Fatal("want error on a 200 that is not the bot API envelope") + } + if !strings.Contains(err.Error(), "not a confirmed send") { + t.Fatalf("error should say the send is unconfirmed, got: %v", err) + } +} + +// The success path must stay a success: ok=true on 200 is a send. +func TestSendAcceptsOkTrue(t *testing.T) { + rs := newRecordingServer(t, http.StatusOK, `{"ok":true,"result":{"message_id":7}}`) + srv := httptest.NewServer(rs.handler()) + defer srv.Close() + + sink, _ := New(sinkCfg(srv.URL)) + if err := sink.Send(context.Background(), nudgeSendable(loop.Sev4, "down")); err != nil { + t.Fatalf("want success on ok=true, got: %v", err) + } +} + +// A body long enough to have been truncated by the old 4096-byte cap still +// parses, so a real send is not reported as a failure. +func TestSendAcceptsAnOversizedButValidEnvelope(t *testing.T) { + rs := newRecordingServer(t, http.StatusOK, + `{"ok":true,"result":{"message_id":7,"text":"`+strings.Repeat("x", 8000)+`"}}`) + srv := httptest.NewServer(rs.handler()) + defer srv.Close() + + sink, _ := New(sinkCfg(srv.URL)) + if err := sink.Send(context.Background(), nudgeSendable(loop.Sev4, "down")); err != nil { + t.Fatalf("want success on a large ok=true envelope, got: %v", err) + } +} + +// The error line carries a snippet, not the relay's whole page. +func TestSendErrorDoesNotCarryTheWholeBody(t *testing.T) { + rs := newRecordingServer(t, http.StatusBadGateway, strings.Repeat("z", 5000)) + srv := httptest.NewServer(rs.handler()) + defer srv.Close() + + sink, _ := New(sinkCfg(srv.URL)) + err := sink.Send(context.Background(), nudgeSendable(loop.Sev4, "down")) + if err == nil { + t.Fatal("want error on 502") + } + if len(err.Error()) > 500 { + t.Fatalf("error line should be trimmed, got %d bytes", len(err.Error())) + } +} + func TestSendReturnsErrorOnNon2xx(t *testing.T) { rs := newRecordingServer(t, http.StatusBadGateway, "bad gateway") srv := httptest.NewServer(rs.handler())