fix: Reduce reporter noise

Introduce the `ErrOperationNotAllowed` and add the `shouldReportIMAPCommandError` to check whether the failure should be reported. The latter will check whether the error should be reported.
ProtonMail · Mar 1, 2023 · 3b1b90f · 3b1b90f
1 parent 247aba9
commit 3b1b90f
Show file tree

Hide file tree

Showing 9 changed files with 66 additions and 29 deletions.
diff --git a/connector/connector.go b/connector/connector.go
@@ -2,11 +2,14 @@ package connector
 
 import (
 	"context"
+	"errors"
 	"time"
 
 	"github.com/ProtonMail/gluon/imap"
 )
 
+var ErrOperationNotAllowed = errors.New("operation not allowed")
+
 // Connector connects the gluon server to a remote mail store.
 type Connector interface {
 	// Authorize returns whether the given username/password combination are valid for this connector.

diff --git a/internal/session/errors.go b/internal/session/errors.go
@@ -1,6 +1,11 @@
 package session
 
-import "errors"
+import (
+	"context"
+	"errors"
+	"github.com/ProtonMail/gluon/connector"
+	"net"
+)
 
 var (
 	ErrCreateInbox = errors.New("cannot create INBOX")
@@ -13,3 +18,18 @@ var (
 
 	ErrNotImplemented = errors.New("not implemented")
 )
+
+func shouldReportIMAPCommandError(err error) bool {
+	var netErr *net.OpError
+
+	switch {
+	case errors.Is(err, connector.ErrOperationNotAllowed):
+		return false
+	case errors.Is(err, context.Canceled):
+		return false
+	case errors.As(err, &netErr):
+		return false
+	}
+
+	return true
+}
diff --git a/internal/session/handle_append.go b/internal/session/handle_append.go
@@ -28,10 +28,12 @@ func (s *Session) handleAppend(ctx context.Context, tag string, cmd *command.App
 	if err := s.state.AppendOnlyMailbox(ctx, nameUTF8, func(mailbox state.AppendOnlyMailbox, isSameMBox bool) error {
 		messageUID, err := mailbox.Append(ctx, cmd.Literal, flags, cmd.DateTime)
 		if err != nil {
-			reporter.MessageWithContext(ctx,
-				"Failed to append message to mailbox from state",
-				reporter.Context{"error": err},
-			)
+			if shouldReportIMAPCommandError(err) {
+				reporter.MessageWithContext(ctx,
+					"Failed to append message to mailbox from state",
+					reporter.Context{"error": err},
+				)
+			}
 
 			return err
 		}

diff --git a/internal/session/handle_copy.go b/internal/session/handle_copy.go
@@ -32,10 +32,12 @@ func (s *Session) handleCopy(ctx context.Context, tag string, cmd *command.Copy,
 	} else if errors.Is(err, state.ErrNoSuchMailbox) {
 		return response.No(tag).WithError(err).WithItems(response.ItemTryCreate()), nil
 	} else if err != nil {
-		reporter.MessageWithContext(ctx,
-			"Failed to copy messages",
-			reporter.Context{"error": err},
-		)
+		if shouldReportIMAPCommandError(err) {
+			reporter.MessageWithContext(ctx,
+				"Failed to copy messages",
+				reporter.Context{"error": err},
+			)
+		}
 
 		return nil, err
 	}

diff --git a/internal/session/handle_create.go b/internal/session/handle_create.go
@@ -25,10 +25,12 @@ func (s *Session) handleCreate(ctx context.Context, tag string, cmd *command.Cre
 	}
 
 	if err := s.state.Create(ctx, nameUTF8); err != nil {
-		reporter.MessageWithContext(ctx,
-			"Failed to create mailbox",
-			reporter.Context{"error": err},
-		)
+		if shouldReportIMAPCommandError(err) {
+			reporter.MessageWithContext(ctx,
+				"Failed to create mailbox",
+				reporter.Context{"error": err},
+			)
+		}
 
 		return err
 	}

diff --git a/internal/session/handle_delete.go b/internal/session/handle_delete.go
@@ -26,10 +26,12 @@ func (s *Session) handleDelete(ctx context.Context, tag string, cmd *command.Del
 
 	selectedDeleted, err := s.state.Delete(ctx, nameUTF8)
 	if err != nil {
-		reporter.MessageWithContext(ctx,
-			"Failed to delete mailbox",
-			reporter.Context{"error": err},
-		)
+		if shouldReportIMAPCommandError(err) {
+			reporter.MessageWithContext(ctx,
+				"Failed to delete mailbox",
+				reporter.Context{"error": err},
+			)
+		}
 
 		return err
 	}

diff --git a/internal/session/handle_fetch.go b/internal/session/handle_fetch.go
@@ -24,10 +24,12 @@ func (s *Session) handleFetch(ctx context.Context, tag string, cmd *command.Fetc
 	if err := mailbox.Fetch(ctx, cmd, ch); errors.Is(err, state.ErrNoSuchMessage) {
 		return response.Bad(tag).WithError(err), nil
 	} else if err != nil {
-		reporter.MessageWithContext(ctx,
-			"Failed to fetch messages",
-			reporter.Context{"error": err},
-		)
+		if shouldReportIMAPCommandError(err) {
+			reporter.MessageWithContext(ctx,
+				"Failed to fetch messages",
+				reporter.Context{"error": err},
+			)
+		}
 
 		return nil, err
 	}

diff --git a/internal/session/handle_move.go b/internal/session/handle_move.go
@@ -32,10 +32,12 @@ func (s *Session) handleMove(ctx context.Context, tag string, cmd *command.Move,
 	} else if errors.Is(err, state.ErrNoSuchMailbox) {
 		return response.No(tag).WithError(err).WithItems(response.ItemTryCreate()), nil
 	} else if err != nil {
-		reporter.MessageWithContext(ctx,
-			"Failed to move messages from mailbox",
-			reporter.Context{"error": err},
-		)
+		if shouldReportIMAPCommandError(err) {
+			reporter.MessageWithContext(ctx,
+				"Failed to move messages from mailbox",
+				reporter.Context{"error": err},
+			)
+		}
 
 		return nil, err
 	}

diff --git a/internal/session/handle_store.go b/internal/session/handle_store.go
@@ -33,10 +33,12 @@ func (s *Session) handleStore(ctx context.Context, tag string, cmd *command.Stor
 	if err := mailbox.Store(ctx, cmd.SeqSet, cmd.Action, flags); errors.Is(err, state.ErrNoSuchMessage) {
 		return response.Bad(tag).WithError(err), nil
 	} else if err != nil {
-		reporter.MessageWithContext(ctx,
-			"Failed to store flags on messages",
-			reporter.Context{"error": err},
-		)
+		if shouldReportIMAPCommandError(err) {
+			reporter.MessageWithContext(ctx,
+				"Failed to store flags on messages",
+				reporter.Context{"error": err},
+			)
+		}
 
 		return nil, err
 	}