Merge pull request #217 from beeper/highest/plat-37946

fix: reaction, image, join with letter sealing off groups/dms

Author: highesttt

pkg/connector/handlers/audio.go

@@ -45,11 +45,12 @@ func (h *Handler) ConvertAudio(ctx context.Context, portal *bridgev2.Portal, int
 		sid = "m"
 	}
 	downloadOptions := lineOBSDownloadOptions(data.ContentMetadata, isPlainMedia)
-	audioData, err := client.DownloadOBSWithSIDOptions(ctx, oid, data.ID, sid, downloadOptions)
+	talkMetaMessageID := obsTalkMetaMessageID(data.ID, isPlainMedia)
+	audioData, err := client.DownloadOBSWithSIDOptions(ctx, oid, talkMetaMessageID, sid, downloadOptions)
 
 	if newClient, ok := h.tryRecoverClient(ctx, err); ok {
 		client = newClient
-		audioData, err = client.DownloadOBSWithSIDOptions(ctx, oid, data.ID, sid, downloadOptions)
+		audioData, err = client.DownloadOBSWithSIDOptions(ctx, oid, talkMetaMessageID, sid, downloadOptions)
 	}
 
 	if err != nil {
@@ -58,19 +59,8 @@ func (h *Handler) ConvertAudio(ctx context.Context, portal *bridgev2.Portal, int
 			Str("oid", oid).
 			Str("msg_id", data.ID).
 			Bool("plain_media", isPlainMedia).
-			Msg("Failed to download audio from OBS, sending placeholder")
-		return &bridgev2.ConvertedMessage{
-			Parts: []*bridgev2.ConvertedMessagePart{
-				{
-					Type: event.EventMessage,
-					Content: &event.MessageEventContent{
-						MsgType:   event.MsgNotice,
-						Body:      "[Audio unavailable — LINE media expired before it could be bridged]",
-						RelatesTo: relatesTo,
-					},
-				},
-			},
-		}, nil
+			Msg("Failed to download audio from OBS")
+		return mediaDownloadFailure("Audio", err, relatesTo)
 	}
 
 	// Decrypt audio if it has keyMaterial (E2EE)

pkg/connector/handlers/file.go

@@ -36,31 +36,21 @@ func (h *Handler) ConvertFile(ctx context.Context, portal *bridgev2.Portal, inte
 		sid = "m"
 	}
 	downloadOptions := lineOBSDownloadOptions(data.ContentMetadata, isPlainMedia)
-	fileData, err := client.DownloadOBSWithSIDOptions(ctx, oid, data.ID, sid, downloadOptions)
+	talkMetaMessageID := obsTalkMetaMessageID(data.ID, isPlainMedia)
+	fileData, err := client.DownloadOBSWithSIDOptions(ctx, oid, talkMetaMessageID, sid, downloadOptions)
 
 	if newClient, ok := h.tryRecoverClient(ctx, err); ok {
 		client = newClient
-		fileData, err = client.DownloadOBSWithSIDOptions(ctx, oid, data.ID, sid, downloadOptions)
+		fileData, err = client.DownloadOBSWithSIDOptions(ctx, oid, talkMetaMessageID, sid, downloadOptions)
 	}
 
 	if err != nil {
 		h.Log.Warn().
 			Err(err).
 			Str("oid", oid).
 			Bool("plain_media", isPlainMedia).
-			Msg("Failed to download file from OBS, sending placeholder")
-		return &bridgev2.ConvertedMessage{
-			Parts: []*bridgev2.ConvertedMessagePart{
-				{
-					Type: event.EventMessage,
-					Content: &event.MessageEventContent{
-						MsgType:   event.MsgNotice,
-						Body:      "[File unavailable — LINE media expired before it could be bridged]",
-						RelatesTo: relatesTo,
-					},
-				},
-			},
-		}, nil
+			Msg("Failed to download file from OBS")
+		return mediaDownloadFailure("File", err, relatesTo)
 	}
 
 	// Try to decrypt using keyMaterial from encrypted payload

pkg/connector/handlers/handler.go

@@ -2,9 +2,14 @@ package handlers
 
 import (
 	"context"
+	"errors"
+	"fmt"
 	"net/http"
+	"strings"
 
 	"github.com/rs/zerolog"
+	"maunium.net/go/mautrix/bridgev2"
+	"maunium.net/go/mautrix/event"
 
 	"github.com/highesttt/matrix-line-messenger/pkg/line"
 )
@@ -28,6 +33,35 @@ type Handler struct {
 	DecryptMedia func(data []byte, keyMaterial string) ([]byte, error)
 }
 
+func obsTalkMetaMessageID(messageID string, isPlainMedia bool) string {
+	if isPlainMedia {
+		return ""
+	}
+	return messageID
+}
+
+func mediaDownloadFailure(kind string, err error, relatesTo *event.RelatesTo) (*bridgev2.ConvertedMessage, error) {
+	if !errors.Is(err, line.ErrOBSObjectNotFound) {
+		// Keep ambiguous OBS failures retryable. Returning ErrIgnoringRemoteEvent
+		// prevents bridgev2 from posting a generic error notice, while omitting a
+		// converted message means the remote event isn't stored as successfully
+		// bridged and can be retried by a later backfill.
+		return nil, fmt.Errorf("%w: failed to download %s from LINE OBS: %w", bridgev2.ErrIgnoringRemoteEvent, strings.ToLower(kind), err)
+	}
+	return &bridgev2.ConvertedMessage{
+		Parts: []*bridgev2.ConvertedMessagePart{
+			{
+				Type: event.EventMessage,
+				Content: &event.MessageEventContent{
+					MsgType:   event.MsgNotice,
+					Body:      fmt.Sprintf("[%s unavailable — LINE media expired before it could be bridged]", kind),
+					RelatesTo: relatesTo,
+				},
+			},
+		},
+	}, nil
+}
+
 // tryRecoverClient attempts token recovery on auth errors and returns a fresh client.
 // Returns (newClient, true) on success, (nil, false) if recovery was not needed or failed.
 func (h *Handler) tryRecoverClient(ctx context.Context, err error) (*line.Client, bool) {

pkg/connector/handlers/handler_test.go

@@ -4,6 +4,11 @@ import (
 	"context"
 	"errors"
 	"testing"
+
+	"maunium.net/go/mautrix/bridgev2"
+	"maunium.net/go/mautrix/event"
+
+	"github.com/highesttt/matrix-line-messenger/pkg/line"
 )
 
 func TestTryRecoverClientUsesShouldRecover(t *testing.T) {
@@ -34,3 +39,64 @@ func TestTryRecoverClientUsesShouldRecover(t *testing.T) {
 		t.Fatal("RecoverToken was called despite ShouldRecover returning false")
 	}
 }
+
+func TestTryRecoverClientRecoversOBSObjectInfoUnauthorized(t *testing.T) {
+	recoveredClient := line.NewClient("refreshed-token")
+	var recoverCalled bool
+	h := &Handler{
+		ShouldRecover: func(_ context.Context, err error) bool {
+			return line.IsUnauthorizedStatus(err)
+		},
+		IsLoggedOut: func(error) bool {
+			return false
+		},
+		RecoverToken: func(context.Context) error {
+			recoverCalled = true
+			return nil
+		},
+		NewClient: func() *line.Client {
+			return recoveredClient
+		},
+	}
+
+	client, ok := h.tryRecoverClient(context.Background(), errors.New("OBS object info failed (401): unauthorized"))
+	if !ok || client != recoveredClient {
+		t.Fatalf("tryRecoverClient returned client=%v ok=%v, want refreshed client", client, ok)
+	}
+	if !recoverCalled {
+		t.Fatal("RecoverToken was not called for OBS object-info 401")
+	}
+}
+
+func TestMediaDownloadFailureOnlyMaterializesKnownExpiry(t *testing.T) {
+	converted, err := mediaDownloadFailure("Image", line.ErrOBSObjectNotFound, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if converted == nil || len(converted.Parts) != 1 {
+		t.Fatalf("converted = %#v, want one placeholder part", converted)
+	}
+	if converted.Parts[0].Content.MsgType != event.MsgNotice || converted.Parts[0].Content.Body != "[Image unavailable — LINE media expired before it could be bridged]" {
+		t.Fatalf("placeholder content = %#v", converted.Parts[0].Content)
+	}
+
+	converted, err = mediaDownloadFailure("Image", line.ErrOBSEncodingIncomplete, nil)
+	if converted != nil {
+		t.Fatalf("converted transient failure = %#v, want nil", converted)
+	}
+	if !errors.Is(err, line.ErrOBSEncodingIncomplete) {
+		t.Fatalf("err = %v, want ErrOBSEncodingIncomplete", err)
+	}
+	if !errors.Is(err, bridgev2.ErrIgnoringRemoteEvent) {
+		t.Fatalf("err = %v, want ErrIgnoringRemoteEvent", err)
+	}
+}
+
+func TestOBSTalkMetaMessageID(t *testing.T) {
+	if got := obsTalkMetaMessageID("message-id", true); got != "" {
+		t.Fatalf("plain media talk-meta ID = %q, want empty", got)
+	}
+	if got := obsTalkMetaMessageID("message-id", false); got != "message-id" {
+		t.Fatalf("encrypted media talk-meta ID = %q", got)
+	}
+}

pkg/connector/handlers/image.go

@@ -30,6 +30,7 @@ func (h *Handler) ConvertImage(ctx context.Context, portal *bridgev2.Portal, int
 
 	mediaCategory := lineMediaCategory(data.ContentMetadata)
 	downloadOptions := lineOBSDownloadOptions(data.ContentMetadata, isPlainMedia)
+	talkMetaMessageID := obsTalkMetaMessageID(data.ID, isPlainMedia)
 
 	var imgData []byte
 	var err error
@@ -43,18 +44,18 @@ func (h *Handler) ConvertImage(ctx context.Context, portal *bridgev2.Portal, int
 		Bool("plain_media", isPlainMedia).
 		Msg("Downloading image from LINE OBS")
 	if isPlainMedia {
-		imgData, err = client.DownloadOBSWithSIDOptions(ctx, oid, data.ID, "m", downloadOptions)
+		imgData, err = client.DownloadOBSWithSIDOptions(ctx, oid, talkMetaMessageID, "m", downloadOptions)
 	} else {
-		imgData, err = client.DownloadOBSWithOptions(ctx, oid, data.ID, downloadOptions)
+		imgData, err = client.DownloadOBSWithOptions(ctx, oid, talkMetaMessageID, downloadOptions)
 	}
 
 	// Refresh token if we get a 401
 	if newClient, ok := h.tryRecoverClient(ctx, err); ok {
 		client = newClient
 		if isPlainMedia {
-			imgData, err = client.DownloadOBSWithSIDOptions(ctx, oid, data.ID, "m", downloadOptions)
+			imgData, err = client.DownloadOBSWithSIDOptions(ctx, oid, talkMetaMessageID, "m", downloadOptions)
 		} else {
-			imgData, err = client.DownloadOBSWithOptions(ctx, oid, data.ID, downloadOptions)
+			imgData, err = client.DownloadOBSWithOptions(ctx, oid, talkMetaMessageID, downloadOptions)
 		}
 	}
 	downloadDuration := time.Since(dlStart)
@@ -66,19 +67,8 @@ func (h *Handler) ConvertImage(ctx context.Context, portal *bridgev2.Portal, int
 			Str("msg_id", data.ID).
 			Bool("plain_media", isPlainMedia).
 			Dur("download_duration", downloadDuration).
-			Msg("Failed to download image from OBS, sending placeholder")
-		return &bridgev2.ConvertedMessage{
-			Parts: []*bridgev2.ConvertedMessagePart{
-				{
-					Type: event.EventMessage,
-					Content: &event.MessageEventContent{
-						MsgType:   event.MsgNotice,
-						Body:      "[Image unavailable — LINE media expired before it could be bridged]",
-						RelatesTo: relatesTo,
-					},
-				},
-			},
-		}, nil
+			Msg("Failed to download image from OBS")
+		return mediaDownloadFailure("Image", err, relatesTo)
 	}
 
 	// Decrypt image if it has keyMaterial (E2EE)

pkg/connector/handlers/video.go

@@ -46,12 +46,13 @@ func (h *Handler) ConvertVideo(ctx context.Context, portal *bridgev2.Portal, int
 		sid = "m"
 	}
 	downloadOptions := lineOBSDownloadOptions(data.ContentMetadata, isPlainMedia)
+	talkMetaMessageID := obsTalkMetaMessageID(data.ID, isPlainMedia)
 	dlStart := time.Now()
-	videoData, err := client.DownloadOBSWithSIDOptions(ctx, oid, data.ID, sid, downloadOptions)
+	videoData, err := client.DownloadOBSWithSIDOptions(ctx, oid, talkMetaMessageID, sid, downloadOptions)
 
 	if newClient, ok := h.tryRecoverClient(ctx, err); ok {
 		client = newClient
-		videoData, err = client.DownloadOBSWithSIDOptions(ctx, oid, data.ID, sid, downloadOptions)
+		videoData, err = client.DownloadOBSWithSIDOptions(ctx, oid, talkMetaMessageID, sid, downloadOptions)
 	}
 
 	if err != nil {
@@ -61,19 +62,8 @@ func (h *Handler) ConvertVideo(ctx context.Context, portal *bridgev2.Portal, int
 			Str("msg_id", data.ID).
 			Bool("plain_media", isPlainMedia).
 			Dur("download_duration", time.Since(dlStart)).
-			Msg("Failed to download video from OBS, sending placeholder")
-		return &bridgev2.ConvertedMessage{
-			Parts: []*bridgev2.ConvertedMessagePart{
-				{
-					Type: event.EventMessage,
-					Content: &event.MessageEventContent{
-						MsgType:   event.MsgNotice,
-						Body:      "[Video unavailable — LINE media expired before it could be bridged]",
-						RelatesTo: relatesTo,
-					},
-				},
-			},
-		}, nil
+			Msg("Failed to download video from OBS")
+		return mediaDownloadFailure("Video", err, relatesTo)
 	}
 
 	decrypted := false