fix(channels): address second round of review feedback on QQ channel
- Fix SendMedia: bypass media store for direct http(s) URLs in part.Ref; only fall back to store.Resolve for media:// refs; log clear warning for local-only paths instead of silently skipping - Fix chatType routing: default unknown chatIDs to "group" (safer for QQ since outbound-only destinations like reasoning_channel_id are groups); pre-register reasoning_channel_id as group at Start() time; add debug log for untracked chatIDs - Add dedup hard cap (10000 entries): evict oldest entry when map exceeds capacity to prevent unbounded memory growth under high traffic
This commit is contained in:
parent
aa1bc73536
commit
0ac7af02b0
1 changed files with 62 additions and 26 deletions
|
|
@ -26,6 +26,7 @@ import (
|
||||||
const (
|
const (
|
||||||
dedupTTL = 5 * time.Minute
|
dedupTTL = 5 * time.Minute
|
||||||
dedupInterval = 60 * time.Second
|
dedupInterval = 60 * time.Second
|
||||||
|
dedupMaxSize = 10000 // hard cap on dedup map entries
|
||||||
typingResend = 8 * time.Second
|
typingResend = 8 * time.Second
|
||||||
typingSeconds = 10
|
typingSeconds = 10
|
||||||
)
|
)
|
||||||
|
|
@ -133,6 +134,12 @@ func (c *QQChannel) Start(ctx context.Context) error {
|
||||||
// start dedup janitor goroutine
|
// start dedup janitor goroutine
|
||||||
go c.dedupJanitor()
|
go c.dedupJanitor()
|
||||||
|
|
||||||
|
// Pre-register reasoning_channel_id as group chat if configured,
|
||||||
|
// so outbound-only destinations are routed correctly.
|
||||||
|
if c.config.ReasoningChannelID != "" {
|
||||||
|
c.chatType.Store(c.config.ReasoningChannelID, "group")
|
||||||
|
}
|
||||||
|
|
||||||
c.SetRunning(true)
|
c.SetRunning(true)
|
||||||
logger.InfoC("qq", "QQ bot started successfully")
|
logger.InfoC("qq", "QQ bot started successfully")
|
||||||
|
|
||||||
|
|
@ -154,13 +161,18 @@ func (c *QQChannel) Stop(ctx context.Context) error {
|
||||||
}
|
}
|
||||||
|
|
||||||
// getChatKind returns the chat type for a given chatID ("group" or "direct").
|
// getChatKind returns the chat type for a given chatID ("group" or "direct").
|
||||||
|
// Unknown chatIDs default to "group" and log a warning, since QQ group IDs are
|
||||||
|
// more common as outbound-only destinations (e.g. reasoning_channel_id).
|
||||||
func (c *QQChannel) getChatKind(chatID string) string {
|
func (c *QQChannel) getChatKind(chatID string) string {
|
||||||
if v, ok := c.chatType.Load(chatID); ok {
|
if v, ok := c.chatType.Load(chatID); ok {
|
||||||
if k, ok := v.(string); ok {
|
if k, ok := v.(string); ok {
|
||||||
return k
|
return k
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return "direct"
|
logger.DebugCF("qq", "Unknown chat type for chatID, defaulting to group", map[string]any{
|
||||||
|
"chat_id": chatID,
|
||||||
|
})
|
||||||
|
return "group"
|
||||||
}
|
}
|
||||||
|
|
||||||
func (c *QQChannel) Send(ctx context.Context, msg bus.OutboundMessage) error {
|
func (c *QQChannel) Send(ctx context.Context, msg bus.OutboundMessage) error {
|
||||||
|
|
@ -292,9 +304,9 @@ func (c *QQChannel) StartTyping(ctx context.Context, chatID string) (func(), err
|
||||||
}
|
}
|
||||||
|
|
||||||
// SendMedia implements the channels.MediaSender interface.
|
// SendMedia implements the channels.MediaSender interface.
|
||||||
// It sends rich media (images, videos, audio, files) via QQ RichMediaMessage.
|
// QQ RichMediaMessage requires an HTTP/HTTPS URL — local file paths are not supported.
|
||||||
// Note: RichMediaMessage requires an HTTP/HTTPS URL. Local-only files are skipped
|
// If part.Ref is already an http(s) URL it is used directly; otherwise we try
|
||||||
// with a warning since QQ API does not accept local file paths.
|
// the media store, and skip with a warning if the resolved path is not an HTTP URL.
|
||||||
func (c *QQChannel) SendMedia(ctx context.Context, msg bus.OutboundMediaMessage) error {
|
func (c *QQChannel) SendMedia(ctx context.Context, msg bus.OutboundMediaMessage) error {
|
||||||
if !c.IsRunning() {
|
if !c.IsRunning() {
|
||||||
return channels.ErrNotRunning
|
return channels.ErrNotRunning
|
||||||
|
|
@ -302,29 +314,37 @@ func (c *QQChannel) SendMedia(ctx context.Context, msg bus.OutboundMediaMessage)
|
||||||
|
|
||||||
chatKind := c.getChatKind(msg.ChatID)
|
chatKind := c.getChatKind(msg.ChatID)
|
||||||
|
|
||||||
store := c.GetMediaStore()
|
|
||||||
if store == nil {
|
|
||||||
return fmt.Errorf("no media store available: %w", channels.ErrSendFailed)
|
|
||||||
}
|
|
||||||
|
|
||||||
for _, part := range msg.Parts {
|
for _, part := range msg.Parts {
|
||||||
localPath, err := store.Resolve(part.Ref)
|
// If the ref is already an HTTP(S) URL, use it directly.
|
||||||
if err != nil {
|
mediaURL := part.Ref
|
||||||
logger.ErrorCF("qq", "Failed to resolve media ref", map[string]any{
|
if !isHTTPURL(mediaURL) {
|
||||||
"ref": part.Ref,
|
// Try resolving through media store.
|
||||||
"error": err.Error(),
|
store := c.GetMediaStore()
|
||||||
})
|
if store == nil {
|
||||||
continue
|
logger.WarnCF("qq", "QQ media requires HTTP/HTTPS URL, no media store available", map[string]any{
|
||||||
}
|
"ref": part.Ref,
|
||||||
|
})
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
// QQ RichMediaMessage requires an HTTP/HTTPS URL.
|
resolved, err := store.Resolve(part.Ref)
|
||||||
// If the resolved path is a local file, skip with a warning.
|
if err != nil {
|
||||||
if !isHTTPURL(localPath) {
|
logger.ErrorCF("qq", "Failed to resolve media ref", map[string]any{
|
||||||
logger.WarnCF("qq", "QQ media requires HTTP/HTTPS URL, skipping local file", map[string]any{
|
"ref": part.Ref,
|
||||||
"ref": part.Ref,
|
"error": err.Error(),
|
||||||
"path": localPath,
|
})
|
||||||
})
|
continue
|
||||||
continue
|
}
|
||||||
|
|
||||||
|
if !isHTTPURL(resolved) {
|
||||||
|
logger.WarnCF("qq", "QQ media requires HTTP/HTTPS URL, local files not supported", map[string]any{
|
||||||
|
"ref": part.Ref,
|
||||||
|
"resolved": resolved,
|
||||||
|
})
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
mediaURL = resolved
|
||||||
}
|
}
|
||||||
|
|
||||||
// Map part type to QQ file type: 1=image, 2=video, 3=audio, 4=file.
|
// Map part type to QQ file type: 1=image, 2=video, 3=audio, 4=file.
|
||||||
|
|
@ -342,7 +362,7 @@ func (c *QQChannel) SendMedia(ctx context.Context, msg bus.OutboundMediaMessage)
|
||||||
|
|
||||||
richMedia := &dto.RichMediaMessage{
|
richMedia := &dto.RichMediaMessage{
|
||||||
FileType: fileType,
|
FileType: fileType,
|
||||||
URL: localPath,
|
URL: mediaURL,
|
||||||
SrvSendMsg: true,
|
SrvSendMsg: true,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -503,6 +523,7 @@ func (c *QQChannel) handleGroupATMessage() event.GroupATMessageEventHandler {
|
||||||
}
|
}
|
||||||
|
|
||||||
// isDuplicate checks whether a message has been seen within the TTL window.
|
// isDuplicate checks whether a message has been seen within the TTL window.
|
||||||
|
// It also enforces a hard cap on map size by evicting oldest entries.
|
||||||
func (c *QQChannel) isDuplicate(messageID string) bool {
|
func (c *QQChannel) isDuplicate(messageID string) bool {
|
||||||
c.muDedup.Lock()
|
c.muDedup.Lock()
|
||||||
defer c.muDedup.Unlock()
|
defer c.muDedup.Unlock()
|
||||||
|
|
@ -511,6 +532,21 @@ func (c *QQChannel) isDuplicate(messageID string) bool {
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Enforce hard cap: evict oldest entries when at capacity.
|
||||||
|
if len(c.dedup) >= dedupMaxSize {
|
||||||
|
var oldestID string
|
||||||
|
var oldestTS time.Time
|
||||||
|
for id, ts := range c.dedup {
|
||||||
|
if oldestID == "" || ts.Before(oldestTS) {
|
||||||
|
oldestID = id
|
||||||
|
oldestTS = ts
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if oldestID != "" {
|
||||||
|
delete(c.dedup, oldestID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
c.dedup[messageID] = time.Now()
|
c.dedup[messageID] = time.Now()
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue