fix(GODT-2582): Do not report errors for duplicate recovered messages

If we fail to append a message that is already in the recovery mailbox,
do not create a new sentry report to avoid spam. Note that this
simplistic approach does not distinguish between different error
sources.

internal/session/errors.go

@@ -37,6 +37,8 @@ func shouldReportIMAPCommandError(err error) bool {
 		return false
 	case errors.Is(err, rfc822.ErrNoSuchPart):
 		return false
+	case errors.Is(err, state.ErrKnownRecoveredMessage):
+		return false
 	}
 
 	return true

internal/state/actions.go

@@ -192,23 +192,23 @@ func (state *State) actionCreateRecoveredMessage(
 	literal []byte,
 	flags imap.FlagSet,
 	date time.Time,
-) error {
+) (bool, error) {
 	internalID := imap.NewInternalMessageID()
 	remoteID := ids.NewRecoveredRemoteMessageID(internalID)
 
 	parsedMessage, err := imap.NewParsedMessage(literal)
 	if err != nil {
-		return err
+		return false, err
 	}
 
 	alreadyKnown, err := state.user.GetRecoveredMessageHashesMap().Insert(internalID, literal)
 	if err == nil && alreadyKnown {
 		// Message is already known to us, so we ignore it.
-		return nil
+		return true, nil
 	}
 
 	if err := state.user.GetStore().SetUnchecked(internalID, bytes.NewReader(literal)); err != nil {
-		return fmt.Errorf("failed to store message literal: %w", err)
+		return false, fmt.Errorf("failed to store message literal: %w", err)
 	}
 
 	req := db.CreateMessageReq{
@@ -228,18 +228,18 @@ func (state *State) actionCreateRecoveredMessage(
 
 	messageUID, flagSet, err := db.CreateAndAddMessageToMailbox(ctx, tx, recoveryMBoxID.InternalID, &req)
 	if err != nil {
-		return err
+		return false, err
 	}
 
 	if err := state.user.QueueOrApplyStateUpdate(ctx, tx, newExistsStateUpdateWithExists(
 		recoveryMBoxID.InternalID,
 		[]*exists{newExists(ids.MessageIDPair{InternalID: internalID, RemoteID: remoteID}, messageUID, flagSet)},
 		nil,
 	)); err != nil {
-		return err
+		return false, err
 	}
 
-	return nil
+	return false, nil
 }
 
 func (state *State) actionAddMessagesToMailbox(

internal/state/mailbox.go

@@ -2,6 +2,8 @@ package state
 
 import (
 	"context"
+	"errors"
+	"fmt"
 	"strings"
 	"time"
 
@@ -231,15 +233,22 @@ func (m *Mailbox) AppendRegular(ctx context.Context, literal []byte, flags imap.
 	})
 }
 
+var ErrKnownRecoveredMessage = errors.New("known recovered message, possible duplication")
+
 func (m *Mailbox) Append(ctx context.Context, literal []byte, flags imap.FlagSet, date time.Time) (imap.UID, error) {
 	uid, err := m.AppendRegular(ctx, literal, flags, date)
 	if err != nil {
 		// Failed to append to mailbox attempt to insert into recovery mailbox.
-		if err := m.state.db().Write(ctx, func(ctx context.Context, tx *ent.Tx) error {
+		knownMessage, recoverErr := db.WriteResult(ctx, m.state.db(), func(ctx context.Context, tx *ent.Tx) (bool, error) {
 			return m.state.actionCreateRecoveredMessage(ctx, tx, literal, flags, date)
-		}); err != nil {
-			logrus.WithError(err).Error("Failed to insert message into recovery mailbox")
-			reporter.ExceptionWithContext(ctx, "Failed to insert message into recovery mailbox", reporter.Context{"error": err})
+		})
+		if recoverErr != nil && !knownMessage {
+			logrus.WithError(recoverErr).Error("Failed to insert message into recovery mailbox")
+			reporter.ExceptionWithContext(ctx, "Failed to insert message into recovery mailbox", reporter.Context{"error": recoverErr})
+		}
+
+		if knownMessage {
+			err = fmt.Errorf("%v: %w", err, ErrKnownRecoveredMessage)
 		}
 	}
 

tests/recovery_mailbox_test.go

@@ -238,6 +238,38 @@ func TestFailedAppendAreDedupedInRecoveryMailbox(t *testing.T) {
 	})
 }
 
+func TestRecoveryMailboxOnlyReportsOnFirstDedupedMessage(t *testing.T) {
+	runOneToOneTestClientWithAuth(t, defaultServerOptions(t, withConnectorBuilder(&failAppendLabelConnectorBuilder{})), func(client *client.Client, s *testSession) {
+		{
+			status, err := client.Status(ids.GluonRecoveryMailboxName, []goimap.StatusItem{goimap.StatusMessages})
+			require.NoError(t, err)
+			require.Equal(t, uint32(0), status.Messages)
+		}
+
+		status, err := client.Select("INBOX", false)
+		require.NoError(t, err)
+		require.Equal(t, uint32(0), status.Messages)
+		require.Error(t, doAppendWithClient(client, "INBOX", "To: [email protected]", time.Now()))
+		require.Error(t, doAppendWithClient(client, "INBOX", "To: [email protected]", time.Now()))
+
+		{
+			status, err := client.Status(ids.GluonRecoveryMailboxName, []goimap.StatusItem{goimap.StatusMessages})
+			require.NoError(t, err)
+			require.Equal(t, uint32(1), status.Messages)
+		}
+		{
+			status, err := client.Status("INBOX", []goimap.StatusItem{goimap.StatusMessages})
+			require.NoError(t, err)
+			require.Equal(t, uint32(0), status.Messages)
+		}
+
+		{
+			reports := s.reporter.getReports()
+			require.Equal(t, 1, len(reports))
+		}
+	})
+}
+
 func TestRecoveryMBoxCanBeCopiedOutOfDedup(t *testing.T) {
 	runOneToOneTestClientWithAuth(t, defaultServerOptions(t, withConnectorBuilder(&recoveryDedupConnectorConnectorBuilder{})), func(client *client.Client, s *testSession) {
 		// Insert first message, fails.