From 66e09717bd8832169d01884f115f251cfd334088 Mon Sep 17 00:00:00 2001 From: Alexander Trakhimenok Date: Wed, 15 Jul 2026 19:49:50 +0100 Subject: [PATCH] feat(botsfw): add SendGate so a platform can refuse a send MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bots-fw currently sends unconditionally, from code a platform cannot intercept. That is safe on Telegram, where a bot may message any chat it knows at any time - but that is a Telegram property, not a universal one. WhatsApp permits free-form messages only within 24 hours of the recipient's last reply. Outside that window a send fails with error 131047, and only a pre-approved template may be delivered. Today a platform has no way to say "not now", so the router would spend an API call to earn a rejection - or worse, deliver a billable template the app never intended. SendGate is an OPTIONAL interface on WebhookResponder: type SendGate interface { CanSend(c context.Context, m botmsg.MessageFromBot) error } A responder that does not implement it is treated as always permitting, so this is additive rather than breaking. Verified: bots-fw-telegram builds and its tests pass unchanged against this commit (via a temporary local replace). This deliberately does NOT change WebhookResponder.SendMessage's signature, which was the original proposal in the gap analysis. An optional interface achieves the same refusal without breaking every existing responder, and gates BEFORE the send rather than reporting after it - which matters when the attempt itself costs money. Routed the three unconditional send sites through the seam: - router.go processCommandResponse - every command response - router.go - the "Unknown Type=%d" message - driver.go - the panic handler, which pushes up to 3KB of stack trace into the user's chat. Best-effort: the panic is already logged and sent to analytics, so a refusal here is a warning, not an error. Refusals log at Warning, not Error: a gated platform declining an unsolicited message is the system working as designed, not a failure. Tests: 8 new, covering the compatibility guarantee (ungated responders always permit), that a refused send reaches no platform at all, and that refusals stay classifiable through wrapping via errors.Is. Background: spec/research/bots-fw-platform-neutrality-gap-analysis.md §1.1 in bots-go-framework/backstage. Co-Authored-By: Claude Opus 4.8 --- botsfw/send_gate.go | 83 ++++++++++++++++++++++++ botsfw/send_gate_test.go | 136 +++++++++++++++++++++++++++++++++++++++ botswebhook/driver.go | 13 +++- botswebhook/router.go | 18 ++++-- 4 files changed, 244 insertions(+), 6 deletions(-) create mode 100644 botsfw/send_gate.go create mode 100644 botsfw/send_gate_test.go diff --git a/botsfw/send_gate.go b/botsfw/send_gate.go new file mode 100644 index 0000000..be479f4 --- /dev/null +++ b/botsfw/send_gate.go @@ -0,0 +1,83 @@ +package botsfw + +import ( + "context" + "errors" + "fmt" + + "github.com/bots-go-framework/bots-fw/botmsg" +) + +// ErrSendNotPermitted is the base for refusals returned by a SendGate. +// +// Wrap it so callers can classify a refusal with errors.Is without depending on +// a specific platform's package: +// +// fmt.Errorf("outside the 24h window: %w", botsfw.ErrSendNotPermitted) +var ErrSendNotPermitted = errors.New("sending is not permitted right now") + +// SendGate is an optional interface a WebhookResponder may implement to refuse +// sends its platform does not currently permit. +// +// It exists because "a bot may message any chat it knows, at any time" is a +// Telegram property, not a universal one. Telegram's responder therefore does +// not implement SendGate, and nothing about its behaviour changes. +// +// Other platforms gate sending. WhatsApp permits free-form messages only within +// 24 hours of the recipient's last reply; outside that window a send fails, and +// only a pre-approved template may be delivered. Without this seam a platform has +// no way to say "not now" — the router would send unconditionally, spending an +// API call to earn a rejection, or worse, delivering a billable template message +// the app never intended. +// +// A responder that does not implement SendGate is treated as always permitting, +// so this is additive: existing responders keep working untouched. +type SendGate interface { + // CanSend reports whether m may be sent right now. + // + // A nil error means the send may proceed. A non-nil error means it may not, + // and describes why; implementations should wrap ErrSendNotPermitted so the + // refusal is classifiable. + // + // CanSend must not perform the send, and should avoid network calls: it is + // consulted on every outbound message. + CanSend(c context.Context, m botmsg.MessageFromBot) error +} + +// CanSend reports whether responder permits sending m right now. +// +// Responders that do not implement SendGate always permit, so this is safe to +// call on any responder. Returns nil when the send may proceed. +func CanSend(c context.Context, responder WebhookResponder, m botmsg.MessageFromBot) error { + if responder == nil { + return nil + } + gate, ok := responder.(SendGate) + if !ok { + return nil + } + return gate.CanSend(c, m) +} + +// IsSendNotPermitted reports whether err is a SendGate refusal. +func IsSendNotPermitted(err error) bool { + return errors.Is(err, ErrSendNotPermitted) +} + +// SendMessageThroughGate consults responder's SendGate, if any, and sends only if +// the send is permitted. +// +// This is the single seam every outbound send should route through, so a platform +// gets one place to refuse rather than one per call site. On refusal it returns a +// zero response and the refusal error, having attempted no send. +func SendMessageThroughGate( + c context.Context, + responder WebhookResponder, + m botmsg.MessageFromBot, + channel botmsg.BotAPISendMessageChannel, +) (OnMessageSentResponse, error) { + if err := CanSend(c, responder, m); err != nil { + return OnMessageSentResponse{}, fmt.Errorf("refused by platform send gate: %w", err) + } + return responder.SendMessage(c, m, channel) +} diff --git a/botsfw/send_gate_test.go b/botsfw/send_gate_test.go new file mode 100644 index 0000000..a242179 --- /dev/null +++ b/botsfw/send_gate_test.go @@ -0,0 +1,136 @@ +package botsfw + +import ( + "context" + "errors" + "fmt" + "testing" + + "github.com/bots-go-framework/bots-fw/botmsg" +) + +// ungatedResponder does not implement SendGate — the Telegram shape. +type ungatedResponder struct { + sent int +} + +func (r *ungatedResponder) SendMessage( + _ context.Context, _ botmsg.MessageFromBot, _ botmsg.BotAPISendMessageChannel, +) (OnMessageSentResponse, error) { + r.sent++ + return OnMessageSentResponse{}, nil +} + +func (r *ungatedResponder) DeleteMessage(_ context.Context, _ string) error { return nil } + +// gatedResponder implements SendGate — the WhatsApp shape. +type gatedResponder struct { + ungatedResponder + refuse error + asked int +} + +func (r *gatedResponder) CanSend(_ context.Context, _ botmsg.MessageFromBot) error { + r.asked++ + return r.refuse +} + +var ( + _ WebhookResponder = (*ungatedResponder)(nil) + _ WebhookResponder = (*gatedResponder)(nil) + _ SendGate = (*gatedResponder)(nil) +) + +// TestCanSend_ungatedResponderAlwaysPermits pins the compatibility guarantee: +// a responder that does not implement SendGate is unaffected by this seam. +func TestCanSend_ungatedResponderAlwaysPermits(t *testing.T) { + if err := CanSend(context.Background(), &ungatedResponder{}, botmsg.MessageFromBot{}); err != nil { + t.Errorf("an ungated responder must always permit, got: %v", err) + } +} + +func TestCanSend_nilResponderPermits(t *testing.T) { + if err := CanSend(context.Background(), nil, botmsg.MessageFromBot{}); err != nil { + t.Errorf("a nil responder must not error, got: %v", err) + } +} + +func TestCanSend_gatePermits(t *testing.T) { + r := &gatedResponder{} + if err := CanSend(context.Background(), r, botmsg.MessageFromBot{}); err != nil { + t.Errorf("expected permit, got: %v", err) + } + if r.asked != 1 { + t.Errorf("expected the gate to be consulted once, got %d", r.asked) + } +} + +func TestCanSend_gateRefuses(t *testing.T) { + want := fmt.Errorf("outside the 24h window: %w", ErrSendNotPermitted) + r := &gatedResponder{refuse: want} + + err := CanSend(context.Background(), r, botmsg.MessageFromBot{}) + if err == nil { + t.Fatal("expected a refusal") + } + if !IsSendNotPermitted(err) { + t.Errorf("refusal must be classifiable via IsSendNotPermitted, got: %v", err) + } +} + +// TestSendMessageThroughGate_refusalSendsNothing is the point of the whole seam: +// a refused send must cost no API call. On WhatsApp an attempted out-of-window +// send earns a rejection, and an attempted template send costs real money. +func TestSendMessageThroughGate_refusalSendsNothing(t *testing.T) { + r := &gatedResponder{refuse: fmt.Errorf("outside the 24h window: %w", ErrSendNotPermitted)} + + _, err := SendMessageThroughGate( + context.Background(), r, botmsg.MessageFromBot{}, BotAPISendMessageOverHTTPS, + ) + if err == nil { + t.Fatal("expected a refusal") + } + if !IsSendNotPermitted(err) { + t.Errorf("refusal must survive wrapping, got: %v", err) + } + if r.sent != 0 { + t.Errorf("a refused send must not reach the platform, but SendMessage ran %d time(s)", r.sent) + } +} + +func TestSendMessageThroughGate_permittedSendProceeds(t *testing.T) { + r := &gatedResponder{} + if _, err := SendMessageThroughGate( + context.Background(), r, botmsg.MessageFromBot{}, BotAPISendMessageOverHTTPS, + ); err != nil { + t.Fatalf("expected the send to proceed, got: %v", err) + } + if r.sent != 1 { + t.Errorf("expected exactly 1 send, got %d", r.sent) + } +} + +// TestSendMessageThroughGate_ungatedResponderProceeds pins that existing +// responders keep sending exactly as before. +func TestSendMessageThroughGate_ungatedResponderProceeds(t *testing.T) { + r := &ungatedResponder{} + if _, err := SendMessageThroughGate( + context.Background(), r, botmsg.MessageFromBot{}, BotAPISendMessageOverHTTPS, + ); err != nil { + t.Fatalf("expected the send to proceed, got: %v", err) + } + if r.sent != 1 { + t.Errorf("expected exactly 1 send, got %d", r.sent) + } +} + +// TestIsSendNotPermitted_unrelatedError pins that ordinary send failures are not +// mistaken for refusals. +func TestIsSendNotPermitted_unrelatedError(t *testing.T) { + if IsSendNotPermitted(errors.New("connection reset")) { + t.Error("an unrelated error must not classify as a refusal") + } + if IsSendNotPermitted(nil) { + t.Error("nil must not classify as a refusal") + } +} diff --git a/botswebhook/driver.go b/botswebhook/driver.go index 8b71962..924210f 100644 --- a/botswebhook/driver.go +++ b/botswebhook/driver.go @@ -158,8 +158,17 @@ func (d webhookDriver) processWebhookInput( var chatID string if chatID, err = whc.Input().BotChatID(); err == nil && chatID != "" { if responder := whc.Responder(); responder != nil { - if _, err = responder.SendMessage(ctx, whc.NewMessage(ErrorIcon+" "+messageText), botsfw.BotAPISendMessageOverResponse); err != nil { - log.Errorf(ctx, fmt.Errorf("failed to report error to user: %w", err).Error()) + m := whc.NewMessage(ErrorIcon + " " + messageText) + if _, err = botsfw.SendMessageThroughGate(ctx, responder, m, botsfw.BotAPISendMessageOverResponse); err != nil { + if botsfw.IsSendNotPermitted(err) { + // A gated platform will not accept an unsolicited + // message here. Reporting the panic to the user is + // best-effort; the panic is already logged and sent + // to analytics above. + log.Warningf(ctx, "not reporting error to user: %v", err) + } else { + log.Errorf(ctx, fmt.Errorf("failed to report error to user: %w", err).Error()) + } } } } diff --git a/botswebhook/router.go b/botswebhook/router.go index 863dc03..6981584 100644 --- a/botswebhook/router.go +++ b/botswebhook/router.go @@ -778,9 +778,14 @@ func logInputDetails(whc botsfw.WebhookContext, isKnownType bool) { } m := whc.NewMessage(fmt.Sprintf("Unknown Type=%d", inputType)) // TODO: Move out of framework to app? - _, err := whc.Responder().SendMessage(c, m, botsfw.BotAPISendMessageOverResponse) - if err != nil { - log.Errorf(c, "Failed to send message: %v", err) + if _, err := botsfw.SendMessageThroughGate(c, whc.Responder(), m, botsfw.BotAPISendMessageOverResponse); err != nil { + if botsfw.IsSendNotPermitted(err) { + // Expected on gated platforms, not a failure: the platform does not + // permit an unsolicited message right now. + log.Warningf(c, "Not reporting unknown input type to the user: %v", err) + } else { + log.Errorf(c, "Failed to send message: %v", err) + } } } @@ -802,7 +807,12 @@ func (whRouter *webhooksRouter) processCommandResponse( if responseChannel == "" { responseChannel = botsfw.BotAPISendMessageOverResponse } - if _, err = responder.SendMessage(c, m, responseChannel); err != nil { + if gateErr := botsfw.CanSend(c, responder, m); gateErr != nil { + // The platform does not permit this send right now — e.g. WhatsApp outside + // the 24h customer-service window. Skip it rather than spending an API call + // to earn a rejection. Not an error: it is the platform working as designed. + log.Warningf(c, "command response not sent: %v", gateErr) + } else if _, err = responder.SendMessage(c, m, responseChannel); err != nil { const failedToSendMessageToMessenger = "failed to send a message to messenger" errText := err.Error() switch {