From 48b4dc6b6cb6381272512253cea169b3db5de1de Mon Sep 17 00:00:00 2001 From: Winni Neessen 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 +// +// 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 +// +// 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 {