mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-15 10:14:51 +00:00
fix(channels): suppress agent-error text on public-facing channels
Rework of #911 against dev with correctness fixes. Public-facing channels (Facebook, Telegram, Discord, Feishu, WhatsApp, Zalo OA, Zalo Personal, Pancake, Slack) no longer receive raw internal error text when an agent run fails — an empty outbound is published instead so channels still clean up placeholders / typing indicators. Errors continue to be logged server-side at Error level. Applied on both hot paths: - cmd/gateway_consumer_normal.go (agent run failure) - cmd/gateway_subagent_announce_queue.go (announce lead run failure) Fixes vs #911: - Whitelist uses channels.Type* constants (compile-time safe) instead of string literals. The original PR's literals ("zalo", "line") did not match any real channel type, so Zalo OA/Personal and Pancake leaked errors while "line" was a dead branch. - Added TypePancake and TypeSlack which were missing. - Empty Content in Facebook.Send and Pancake.Send now short-circuits (matching Telegram/Discord/Slack). Previously, suppressed errors flowed to Graph/Pancake APIs as empty payloads, triggering 400s and channel health degradation. - Added table-driven test for isExternalChannel covering all real channel types + empty + unknown + legacy "zalo" short form. Closes #911
This commit is contained in:
1 parent
770af6b1c0
commit
5040fe4cbe
6 files changed
+102
-2
No files matched your search
@@ -415,10 +415,19 @@ func processNormalMessage(
|
||||
return
|
||||
}
|
||||
slog.Error("inbound: agent run failed", "error", outcome.Err, "channel", channel)
|
||||
// Suppress technical error text on public-facing channels (FB, Telegram, etc.)
|
||||
// Empty Content still triggers placeholder/typing cleanup downstream.
|
||||
errContent := formatAgentError(outcome.Err)
|
||||
if deps.ChannelMgr != nil {
|
||||
if ct := deps.ChannelMgr.ChannelTypeForName(channel); isExternalChannel(ct) {
|
||||
slog.Info("inbound: suppressed error for external channel", "channel", channel, "type", ct)
|
||||
errContent = ""
|
||||
}
|
||||
}
|
||||
deps.MsgBus.PublishOutbound(bus.OutboundMessage{
|
||||
Channel: channel,
|
||||
ChatID: chatID,
|
||||
Content: formatAgentError(outcome.Err),
|
||||
Content: errContent,
|
||||
Metadata: meta,
|
||||
})
|
||||
return
|
||||
|
||||
@@ -74,6 +74,27 @@ func isContextOverflowError(lower string) bool {
|
||||
containsAny(lower, "overflow", "too large", "too long", "limit", "exceeded"))
|
||||
}
|
||||
|
||||
// isExternalChannel reports whether a channel type serves end users on a
|
||||
// public-facing platform (Facebook, Telegram, etc.). Internal error details
|
||||
// must not be forwarded to these channels — the caller publishes an empty
|
||||
// outbound instead so placeholders get cleaned up without leaking technical
|
||||
// error text to end users. Internal types ("ws", "") return false.
|
||||
func isExternalChannel(channelType string) bool {
|
||||
switch channelType {
|
||||
case channels.TypeFacebook,
|
||||
channels.TypeTelegram,
|
||||
channels.TypeDiscord,
|
||||
channels.TypeFeishu,
|
||||
channels.TypeWhatsApp,
|
||||
channels.TypeZaloOA,
|
||||
channels.TypeZaloPersonal,
|
||||
channels.TypePancake,
|
||||
channels.TypeSlack:
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// isMessageFormatError checks for tool_use/tool_result mismatch, role ordering,
|
||||
// and other message format errors that indicate corrupted session history.
|
||||
func isMessageFormatError(lower string) bool {
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
package cmd
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/channels"
|
||||
)
|
||||
|
||||
// TestIsExternalChannel ensures the whitelist matches actual channel type
|
||||
// constants — a mismatch here silently leaks internal error strings to end
|
||||
// users (e.g. "zalo" vs "zalo_oa"/"zalo_personal"). Always compare against
|
||||
// channels.Type* constants, never string literals.
|
||||
func TestIsExternalChannel(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
channelType string
|
||||
want bool
|
||||
}{
|
||||
// Public-facing platforms — errors must be suppressed.
|
||||
{"facebook", channels.TypeFacebook, true},
|
||||
{"telegram", channels.TypeTelegram, true},
|
||||
{"discord", channels.TypeDiscord, true},
|
||||
{"feishu", channels.TypeFeishu, true},
|
||||
{"whatsapp", channels.TypeWhatsApp, true},
|
||||
{"zalo_oa", channels.TypeZaloOA, true},
|
||||
{"zalo_personal", channels.TypeZaloPersonal, true},
|
||||
{"pancake", channels.TypePancake, true},
|
||||
{"slack", channels.TypeSlack, true},
|
||||
|
||||
// Internal / unknown channel types — errors must still surface.
|
||||
{"ws", "ws", false},
|
||||
{"empty", "", false},
|
||||
{"unknown", "myplatform", false},
|
||||
// "line" is a Pancake sub-platform, not a top-level channel type.
|
||||
{"line_not_top_level", "line", false},
|
||||
// Legacy short "zalo" string must NOT match — real constants are zalo_oa / zalo_personal.
|
||||
{"zalo_short_form", "zalo", false},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if got := isExternalChannel(tc.channelType); got != tc.want {
|
||||
t.Fatalf("isExternalChannel(%q) = %v, want %v", tc.channelType, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -182,10 +182,16 @@ func processSubagentAnnounceLoop(
|
||||
if outcome.Err != nil {
|
||||
if !errors.Is(outcome.Err, context.Canceled) {
|
||||
slog.Error("subagent announce: lead run failed", "error", outcome.Err, "batch_size", len(entries))
|
||||
errContent := formatAgentError(outcome.Err)
|
||||
if isExternalChannel(r.OrigChannelType) {
|
||||
slog.Info("subagent announce: suppressed error for external channel",
|
||||
"channel", r.OrigChannel, "type", r.OrigChannelType)
|
||||
errContent = ""
|
||||
}
|
||||
msgBus.PublishOutbound(bus.OutboundMessage{
|
||||
Channel: r.OrigChannel,
|
||||
ChatID: r.OrigChatID,
|
||||
Content: formatAgentError(outcome.Err),
|
||||
Content: errContent,
|
||||
Metadata: r.OutMeta,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -198,6 +198,14 @@ func (ch *Channel) isBotEcho(chatID string, eventAt time.Time) bool {
|
||||
|
||||
// Send delivers an outbound message. Dispatches to comment reply or Messenger based on fb_mode metadata.
|
||||
func (ch *Channel) Send(ctx context.Context, msg bus.OutboundMessage) error {
|
||||
// NO_REPLY / suppressed-error path: empty content with no media means the
|
||||
// caller wants downstream cleanup (placeholder, typing) but no user-visible
|
||||
// message. Graph API rejects empty text, so short-circuit here — matches
|
||||
// the pattern used by Telegram, Discord and Slack.
|
||||
if msg.Content == "" && len(msg.Media) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
mode := msg.Metadata["fb_mode"]
|
||||
|
||||
switch mode {
|
||||
|
||||
@@ -184,6 +184,13 @@ func (ch *Channel) Send(ctx context.Context, msg bus.OutboundMessage) error {
|
||||
return fmt.Errorf("pancake: chat_id (conversation_id) is required for outbound message")
|
||||
}
|
||||
|
||||
// NO_REPLY / suppressed-error path: empty content with no media means the
|
||||
// caller wants downstream cleanup only. Pancake API rejects empty payloads,
|
||||
// so short-circuit before dispatch.
|
||||
if msg.Content == "" && len(msg.Media) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
switch msg.Metadata["pancake_mode"] {
|
||||
case "comment":
|
||||
return ch.sendCommentReply(ctx, msg)
|
||||
|
||||
Reference in new issue
Block a user