From 48b4dc6b6cb6381272512253cea169b3db5de1de Mon Sep 17 00:00:00 2001
From: Winni Neessen <wn@neessen.net>
Date: Sat, 10 Dec 2022 13:41:00 +0100
Subject: [PATCH] Fix #85: Client.Send() failing for all messages if one is
 broken

`Client.Send()` provides the possibility to send multiple `*Msg` in one go. If one of the `*Msg` caused an error with the sending mail server, we were returning completely, while not processing any `*Msg` that came after the failing message.

This PR fixes this behaviour by processing each message first and then return a accumulated error in case any of the `*Msg` processing failed

Additionally, this PR separates the `Client.Send()` method into two different versions. One that makes use of the new `errors.Join()` functionality that is introduced with Go 1.20 and one that handles it the old way for any supported version lower than Go 1.20
---
 client.go      | 52 ---------------------------
 client_119.go  | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++
 client_120.go  | 85 ++++++++++++++++++++++++++++++++++++++++++++
 client_test.go | 44 +++++++++++++++++++++++
 4 files changed, 224 insertions(+), 52 deletions(-)
 create mode 100644 client_119.go
 create mode 100644 client_120.go

diff --git a/client.go b/client.go
index eb608bb..e988d54 100644
--- a/client.go
+++ b/client.go
@@ -463,58 +463,6 @@ func (c *Client) DialWithContext(pc context.Context) error {
 	return nil
 }
 
-// Send sends out the mail message
-func (c *Client) Send(ml ...*Msg) error {
-	if err := c.checkConn(); err != nil {
-		return fmt.Errorf("failed to send mail: %w", err)
-	}
-	for _, m := range ml {
-		if m.encoding == NoEncoding {
-			if ok, _ := c.sc.Extension("8BITMIME"); !ok {
-				return ErrServerNoUnencoded
-			}
-		}
-		f, err := m.GetSender(false)
-		if err != nil {
-			return err
-		}
-		rl, err := m.GetRecipients()
-		if err != nil {
-			return err
-		}
-
-		if err := c.mail(f); err != nil {
-			return fmt.Errorf("sending MAIL FROM command failed: %w", err)
-		}
-		for _, r := range rl {
-			if err := c.rcpt(r); err != nil {
-				return fmt.Errorf("sending RCPT TO command failed: %w", err)
-			}
-		}
-		w, err := c.sc.Data()
-		if err != nil {
-			return fmt.Errorf("sending DATA command failed: %w", err)
-		}
-		_, err = m.WriteTo(w)
-		if err != nil {
-			return fmt.Errorf("sending mail content failed: %w", err)
-		}
-
-		if err := w.Close(); err != nil {
-			return fmt.Errorf("failed to close DATA writer: %w", err)
-		}
-
-		if err := c.Reset(); err != nil {
-			return fmt.Errorf("sending RSET command failed: %w", err)
-		}
-		if err := c.checkConn(); err != nil {
-			return fmt.Errorf("failed to check server connection: %w", err)
-		}
-	}
-
-	return nil
-}
-
 // Close closes the Client connection
 func (c *Client) Close() error {
 	if err := c.checkConn(); err != nil {
diff --git a/client_119.go b/client_119.go
new file mode 100644
index 0000000..f5b942c
--- /dev/null
+++ b/client_119.go
@@ -0,0 +1,95 @@
+// SPDX-FileCopyrightText: 2022 Winni Neessen <winni@neessen.dev>
+//
+// SPDX-License-Identifier: MIT
+
+//go:build !go1.20
+// +build !go1.20
+
+package mail
+
+import (
+	"fmt"
+)
+
+// Send sends out the mail message
+func (c *Client) Send(ml ...*Msg) error {
+	var errs []error
+	if err := c.checkConn(); err != nil {
+		return fmt.Errorf("failed to send mail: %w", err)
+	}
+	for _, m := range ml {
+		if m.encoding == NoEncoding {
+			if ok, _ := c.sc.Extension("8BITMIME"); !ok {
+				errs = append(errs, ErrServerNoUnencoded)
+				continue
+			}
+		}
+		f, err := m.GetSender(false)
+		if err != nil {
+			errs = append(errs, err)
+			continue
+		}
+		rl, err := m.GetRecipients()
+		if err != nil {
+			errs = append(errs, err)
+			continue
+		}
+
+		if err := c.mail(f); err != nil {
+			errs = append(errs, fmt.Errorf("sending MAIL FROM command failed: %w", err))
+			if reserr := c.sc.Reset(); reserr != nil {
+				errs = append(errs, reserr)
+			}
+			continue
+		}
+		failed := false
+		for _, r := range rl {
+			if err := c.rcpt(r); err != nil {
+				errs = append(errs, fmt.Errorf("sending RCPT TO command failed: %w", err))
+				failed = true
+			}
+		}
+		if failed {
+			if reserr := c.sc.Reset(); reserr != nil {
+				errs = append(errs, reserr)
+			}
+			continue
+		}
+		w, err := c.sc.Data()
+		if err != nil {
+			errs = append(errs, fmt.Errorf("sending DATA command failed: %w", err))
+			continue
+		}
+		_, err = m.WriteTo(w)
+		if err != nil {
+			errs = append(errs, fmt.Errorf("sending mail content failed: %w", err))
+			continue
+		}
+
+		if err := w.Close(); err != nil {
+			errs = append(errs, fmt.Errorf("failed to close DATA writer: %w", err))
+			continue
+		}
+
+		if err := c.Reset(); err != nil {
+			errs = append(errs, fmt.Errorf("sending RSET command failed: %w", err))
+			continue
+		}
+		if err := c.checkConn(); err != nil {
+			errs = append(errs, fmt.Errorf("failed to check server connection: %w", err))
+			continue
+		}
+	}
+
+	if len(errs) > 0 {
+		errtxt := ""
+		for i := range errs {
+			errtxt += fmt.Sprintf("%s", errs[i])
+			if i < len(errs) {
+				errtxt += "\n"
+			}
+		}
+		return fmt.Errorf("%s", errtxt)
+	}
+	return nil
+}
diff --git a/client_120.go b/client_120.go
new file mode 100644
index 0000000..8990737
--- /dev/null
+++ b/client_120.go
@@ -0,0 +1,85 @@
+// SPDX-FileCopyrightText: 2022 Winni Neessen <winni@neessen.dev>
+//
+// SPDX-License-Identifier: MIT
+
+//go:build go1.20
+// +build go1.20
+
+package mail
+
+import (
+	"errors"
+	"fmt"
+)
+
+// Send sends out the mail message
+func (c *Client) Send(ml ...*Msg) (rerr error) {
+	if err := c.checkConn(); err != nil {
+		rerr = fmt.Errorf("failed to send mail: %w", err)
+		return
+	}
+	for _, m := range ml {
+		if m.encoding == NoEncoding {
+			if ok, _ := c.sc.Extension("8BITMIME"); !ok {
+				rerr = errors.Join(rerr, ErrServerNoUnencoded)
+				continue
+			}
+		}
+		f, err := m.GetSender(false)
+		if err != nil {
+			rerr = errors.Join(rerr, err)
+			continue
+		}
+		rl, err := m.GetRecipients()
+		if err != nil {
+			rerr = errors.Join(rerr, err)
+			continue
+		}
+
+		if err := c.mail(f); err != nil {
+			rerr = errors.Join(rerr, fmt.Errorf("sending MAIL FROM command failed: %w", err))
+			if reserr := c.sc.Reset(); reserr != nil {
+				rerr = errors.Join(rerr, reserr)
+			}
+			continue
+		}
+		failed := false
+		for _, r := range rl {
+			if err := c.rcpt(r); err != nil {
+				rerr = errors.Join(rerr, fmt.Errorf("sending RCPT TO command failed: %w", err))
+				failed = true
+			}
+		}
+		if failed {
+			if reserr := c.sc.Reset(); reserr != nil {
+				rerr = errors.Join(rerr, reserr)
+			}
+			continue
+		}
+		w, err := c.sc.Data()
+		if err != nil {
+			rerr = errors.Join(rerr, fmt.Errorf("sending DATA command failed: %w", err))
+			continue
+		}
+		_, err = m.WriteTo(w)
+		if err != nil {
+			rerr = errors.Join(rerr, fmt.Errorf("sending mail content failed: %w", err))
+			continue
+		}
+
+		if err := w.Close(); err != nil {
+			rerr = errors.Join(rerr, fmt.Errorf("failed to close DATA writer: %w", err))
+			continue
+		}
+
+		if err := c.Reset(); err != nil {
+			rerr = errors.Join(rerr, fmt.Errorf("sending RSET command failed: %w", err))
+			continue
+		}
+		if err := c.checkConn(); err != nil {
+			rerr = errors.Join(rerr, fmt.Errorf("failed to check server connection: %w", err))
+		}
+	}
+
+	return
+}
diff --git a/client_test.go b/client_test.go
index 645d142..3399762 100644
--- a/client_test.go
+++ b/client_test.go
@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"net/smtp"
 	"os"
+	"strings"
 	"testing"
 	"time"
 
@@ -863,6 +864,49 @@ func TestClient_DialSendCloseBrokenWithDSN(t *testing.T) {
 	}
 }
 
+// TestClient_Send_withBrokenRecipient tests the Send() method of Client with a broken and a working recipient
+func TestClient_Send_withBrokenRecipient(t *testing.T) {
+	if os.Getenv("TEST_ALLOW_SEND") == "" {
+		t.Skipf("TEST_ALLOW_SEND is not set. Skipping mail sending test")
+	}
+	var msgs []*Msg
+	rcpts := []string{"invalid@domain.tld", TestRcpt, "invalid@address.invalid"}
+	for _, rcpt := range rcpts {
+		m := NewMsg()
+		_ = m.FromFormat("go-mail Test Mailer", os.Getenv("TEST_FROM"))
+		_ = m.To(rcpt)
+		m.Subject(fmt.Sprintf("This is a test mail from go-mail/v%s", VERSION))
+		m.SetBulk()
+		m.SetDate()
+		m.SetMessageID()
+		m.SetBodyString(TypeTextPlain, "This is a test mail from the go-mail library")
+		msgs = append(msgs, m)
+	}
+
+	c, err := getTestConnection(true)
+	if err != nil {
+		t.Skipf("failed to create test client: %s. Skipping tests", err)
+	}
+
+	ctx, cfn := context.WithTimeout(context.Background(), DefaultTimeout)
+	defer cfn()
+	if err := c.DialWithContext(ctx); err != nil {
+		t.Errorf("failed to dial to sending server: %s", err)
+	}
+	if err := c.Send(msgs...); err != nil {
+		if !strings.Contains(err.Error(), "invalid@domain.tld") ||
+			!strings.Contains(err.Error(), "invalid@address.invalid") {
+			t.Errorf("sending mails to invalid addresses was supposed to fail but didn't")
+		}
+		if strings.Contains(err.Error(), TestRcpt) {
+			t.Errorf("sending mail to valid addresses failed: %s", err)
+		}
+	}
+	if err := c.Close(); err != nil {
+		t.Errorf("failed to close client connection: %s", err)
+	}
+}
+
 // TestClient_auth tests the Dial(), Send() and Close() method of Client with broken settings
 func TestClient_auth(t *testing.T) {
 	tests := []struct {