From 6809084e80e59735a5c6d9787cc755a721695f8c Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 13 Nov 2024 16:11:28 +0100 Subject: [PATCH 01/14] Add enhanced status code and error code to SendError Enhance error handling by adding error code and enhanced status code to the SendError struct. This allows for better troubleshooting and debugging by providing more detailed SMTP server responses. --- client.go | 24 ++++++++++++----- client_test.go | 53 +++++++++++++++++++++++++++++++++++++ senderror.go | 71 ++++++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 136 insertions(+), 12 deletions(-) diff --git a/client.go b/client.go index fbb28f5..d64c172 100644 --- a/client.go +++ b/client.go @@ -1190,6 +1190,7 @@ func (c *Client) auth() error { func (c *Client) sendSingleMsg(message *Msg) error { c.mutex.Lock() defer c.mutex.Unlock() + escSupport, _ := c.smtpClient.Extension("ENHANCEDSTATUSCODES") if message.encoding == NoEncoding { if ok, _ := c.smtpClient.Extension("8BITMIME"); !ok { @@ -1200,14 +1201,16 @@ func (c *Client) sendSingleMsg(message *Msg) error { if err != nil { return &SendError{ Reason: ErrGetSender, errlist: []error{err}, isTemp: isTempError(err), - affectedMsg: message, + affectedMsg: message, errcode: getErrorCode(err), + enhancedStatusCode: getEnhancedStatusCode(err, escSupport), } } rcpts, err := message.GetRecipients() if err != nil { return &SendError{ Reason: ErrGetRcpts, errlist: []error{err}, isTemp: isTempError(err), - affectedMsg: message, + affectedMsg: message, errcode: getErrorCode(err), + enhancedStatusCode: getEnhancedStatusCode(err, escSupport), } } @@ -1219,7 +1222,8 @@ func (c *Client) sendSingleMsg(message *Msg) error { if err = c.smtpClient.Mail(from); err != nil { retError := &SendError{ Reason: ErrSMTPMailFrom, errlist: []error{err}, isTemp: isTempError(err), - affectedMsg: message, + affectedMsg: message, errcode: getErrorCode(err), + enhancedStatusCode: getEnhancedStatusCode(err, escSupport), } if resetSendErr := c.smtpClient.Reset(); resetSendErr != nil { retError.errlist = append(retError.errlist, resetSendErr) @@ -1238,6 +1242,8 @@ func (c *Client) sendSingleMsg(message *Msg) error { rcptSendErr.errlist = append(rcptSendErr.errlist, err) rcptSendErr.rcpt = append(rcptSendErr.rcpt, rcpt) rcptSendErr.isTemp = isTempError(err) + rcptSendErr.errcode = getErrorCode(err) + rcptSendErr.enhancedStatusCode = getEnhancedStatusCode(err, escSupport) hasError = true } } @@ -1251,20 +1257,23 @@ func (c *Client) sendSingleMsg(message *Msg) error { if err != nil { return &SendError{ Reason: ErrSMTPData, errlist: []error{err}, isTemp: isTempError(err), - affectedMsg: message, + affectedMsg: message, errcode: getErrorCode(err), + enhancedStatusCode: getEnhancedStatusCode(err, escSupport), } } _, err = message.WriteTo(writer) if err != nil { return &SendError{ Reason: ErrWriteContent, errlist: []error{err}, isTemp: isTempError(err), - affectedMsg: message, + affectedMsg: message, errcode: getErrorCode(err), + enhancedStatusCode: getEnhancedStatusCode(err, escSupport), } } if err = writer.Close(); err != nil { return &SendError{ Reason: ErrSMTPDataClose, errlist: []error{err}, isTemp: isTempError(err), - affectedMsg: message, + affectedMsg: message, errcode: getErrorCode(err), + enhancedStatusCode: getEnhancedStatusCode(err, escSupport), } } message.isDelivered = true @@ -1272,7 +1281,8 @@ func (c *Client) sendSingleMsg(message *Msg) error { if err = c.Reset(); err != nil { return &SendError{ Reason: ErrSMTPReset, errlist: []error{err}, isTemp: isTempError(err), - affectedMsg: message, + affectedMsg: message, errcode: getErrorCode(err), + enhancedStatusCode: getEnhancedStatusCode(err, escSupport), } } return nil diff --git a/client_test.go b/client_test.go index 39bd7e0..2687611 100644 --- a/client_test.go +++ b/client_test.go @@ -3148,6 +3148,59 @@ func TestClient_sendSingleMsg(t *testing.T) { t.Errorf("expected ErrSMTPDataClose, got %s", sendErr.Reason) } }) + t.Run("error code and enhanced status code support", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + PortAdder.Add(1) + serverPort := int(TestServerPortBase + PortAdder.Load()) + featureSet := "250-ENHANCEDSTATUSCODES\r\n250-8BITMIME\r\n250-DSN\r\n250 SMTPUTF8" + go func() { + if err := simpleSMTPServer(ctx, t, &serverProps{ + FailOnMailFrom: true, + FeatureSet: featureSet, + ListenPort: serverPort, + }); err != nil { + t.Errorf("failed to start test server: %s", err) + return + } + }() + time.Sleep(time.Millisecond * 30) + + message := testMessage(t) + + ctxDial, cancelDial := context.WithTimeout(ctx, time.Millisecond*500) + t.Cleanup(cancelDial) + + client, err := NewClient(DefaultHost, WithPort(serverPort), WithTLSPolicy(NoTLS)) + if err != nil { + t.Fatalf("failed to create new client: %s", err) + } + if err = client.DialWithContext(ctxDial); err != nil { + var netErr net.Error + if errors.As(err, &netErr) && netErr.Timeout() { + t.Skip("failed to connect to the test server due to timeout") + } + t.Fatalf("failed to connect to test server: %s", err) + } + t.Cleanup(func() { + if err := client.Close(); err != nil { + t.Errorf("failed to close client: %s", err) + } + }) + if err = client.sendSingleMsg(message); err == nil { + t.Error("expected mail delivery to fail") + } + var sendErr *SendError + if !errors.As(err, &sendErr) { + t.Fatalf("expected SendError, got %s", err) + } + if sendErr.errcode != 500 { + t.Errorf("expected error code 500, got %d", sendErr.errcode) + } + if !strings.EqualFold(sendErr.enhancedStatusCode, "5.5.2") { + t.Errorf("expected enhanced status code 5.5.2, got %s", sendErr.enhancedStatusCode) + } + }) } func TestClient_checkConn(t *testing.T) { diff --git a/senderror.go b/senderror.go index 4471208..a371c6e 100644 --- a/senderror.go +++ b/senderror.go @@ -6,6 +6,8 @@ package mail import ( "errors" + "regexp" + "strconv" "strings" ) @@ -60,11 +62,13 @@ const ( // details about the affected message, a list of errors, the recipient list, and whether // the error is temporary or permanent. It also includes a reason code for the error. type SendError struct { - affectedMsg *Msg - errlist []error - isTemp bool - rcpt []string - Reason SendErrReason + affectedMsg *Msg + errcode int + enhancedStatusCode string + errlist []error + isTemp bool + rcpt []string + Reason SendErrReason } // SendErrReason represents a comparable reason on why the delivery failed @@ -175,6 +179,27 @@ func (e *SendError) Msg() *Msg { return e.affectedMsg } +// EnhancedStatusCode returns the enhanced status code of the server response if the +// server supports it, as described in RFC 2034. +// +// This function retrieves the enhanced status code of an error returned by the server. This +// requires that the receiving server supports this SMTP extension as described in RFC 2034. +// Since this is the SendError interface, we only collect status codes for error responses, +// meaning 4xx or 5xx. If the server does not support the ENHANCEDSTATUSCODES extension or +// the error did not include an enhanced status code, it will return an empty string. +// +// Returns: +// - The enhanced status code as returned by the server, or an empty string is not supported. +// +// References: +// - https://datatracker.ietf.org/doc/html/rfc2034 +func (e *SendError) EnhancedStatusCode() string { + if e == nil { + return "" + } + return e.enhancedStatusCode +} + // String satisfies the fmt.Stringer interface for the SendErrReason type. // // This function converts the SendErrReason into a human-readable string representation based @@ -224,3 +249,39 @@ func (r SendErrReason) String() string { func isTempError(err error) bool { return err.Error()[0] == '4' } + +func getErrorCode(err error) int { + rootErr := errors.Unwrap(err) + if rootErr != nil { + err = rootErr + } + firstrune := err.Error()[0] + if firstrune < 52 || firstrune > 53 { + return 0 + } + code := err.Error()[0:3] + errcode, cerr := strconv.Atoi(code) + if cerr != nil { + return 0 + } + return errcode +} + +func getEnhancedStatusCode(err error, supported bool) string { + if err == nil || !supported { + return "" + } + rootErr := errors.Unwrap(err) + if rootErr != nil { + err = rootErr + } + firstrune := err.Error()[0] + if firstrune != 50 && firstrune != 52 && firstrune != 53 { + return "" + } + re, rerr := regexp.Compile(`\b([245])\.\d{1,3}\.\d{1,3}\b`) + if rerr != nil { + return "" + } + return re.FindString(err.Error()) +} From b9d94492521beb8f740542e723f19b80e71cf30c Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 13 Nov 2024 21:25:39 +0100 Subject: [PATCH 02/14] Change test server port base for SMTP client tests Updated the TestServerPortBase from 12025 to 30025 to avoid port conflicts with other services running on the common 12025 port. This adjustment aims to ensure that the tests run reliably in diverse environments. --- client_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client_test.go b/client_test.go index 2687611..d1afc86 100644 --- a/client_test.go +++ b/client_test.go @@ -35,7 +35,7 @@ const ( // TestServerAddr is the address the simple SMTP test server listens on TestServerAddr = "127.0.0.1" // TestServerPortBase is the base port for the simple SMTP test server - TestServerPortBase = 12025 + TestServerPortBase = 30025 // TestSenderValid is a test sender email address considered valid for sending test emails. TestSenderValid = "valid-from@domain.tld" // TestRcptValid is a test recipient email address considered valid for sending test emails. From ad265cac57670289aedacd5acc27331d7a98d1df Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 13 Nov 2024 21:26:11 +0100 Subject: [PATCH 03/14] Add ErrorCode method to SendError Implemented ErrorCode method to retrieve the error code from the server response in SendError. This method distinguishes between server-generated errors and client-generated errors, returning 0 for errors generated by the client. --- senderror.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/senderror.go b/senderror.go index a371c6e..93ab6f2 100644 --- a/senderror.go +++ b/senderror.go @@ -200,6 +200,21 @@ func (e *SendError) EnhancedStatusCode() string { return e.enhancedStatusCode } +// ErrorCode returns the error code of the server response. +// +// This function retrieves the error code the error returned by the server. The error code will +// start with 5 on permanent errors and with 4 on a temporary error. If the error is not returned +// by the server, but is generated by go-mail, the code will be 0. +// +// Returns: +// - The error code as returned by the server, or 0 if not a server error. +func (e *SendError) ErrorCode() int { + if e == nil { + return 0 + } + return e.errcode +} + // String satisfies the fmt.Stringer interface for the SendErrReason type. // // This function converts the SendErrReason into a human-readable string representation based From 615155bfc2f34c1d085bc7fe2fbae5bd6bccf36d Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 13 Nov 2024 21:27:27 +0100 Subject: [PATCH 04/14] Add tests for SendError's enhanced status and error codes Implemented new unit tests for SendError to validate the enhanced status code and error codes in various scenarios, including nil SendError cases, errors with no enhanced status code, and errors with both permanent and temporary error codes. This ensures the correctness of the error handling behavior across different conditions. --- senderror_test.go | 72 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/senderror_test.go b/senderror_test.go index 8584e32..5797520 100644 --- a/senderror_test.go +++ b/senderror_test.go @@ -218,6 +218,78 @@ func TestSendError_Msg(t *testing.T) { }) } +func TestSendError_EnhancedStatusCode(t *testing.T) { + t.Run("SendError with no enhanced status code", func(t *testing.T) { + err := &SendError{ + errlist: []error{ErrNoRcptAddresses}, + rcpt: []string{"", ""}, + Reason: ErrAmbiguous, + } + if err.EnhancedStatusCode() != "" { + t.Errorf("expected empty enhanced status code, got: %s", err.EnhancedStatusCode()) + } + }) + t.Run("SendError with enhanced status code", func(t *testing.T) { + err := &SendError{ + errlist: []error{ErrNoRcptAddresses}, + rcpt: []string{"", ""}, + Reason: ErrAmbiguous, + enhancedStatusCode: "5.7.1", + } + if err.EnhancedStatusCode() != "5.7.1" { + t.Errorf("expected enhanced status code: %s, got: %s", "5.7.1", err.EnhancedStatusCode()) + } + }) + t.Run("enhanced status code on nil error should return empty string", func(t *testing.T) { + var err *SendError + if err.EnhancedStatusCode() != "" { + t.Error("expected empty enhanced status code on nil-senderror") + } + }) +} + +func TestSendError_ErrorCode(t *testing.T) { + t.Run("ErrorCode with a go-mail error should return 0", func(t *testing.T) { + err := &SendError{ + errlist: []error{ErrNoRcptAddresses}, + rcpt: []string{"", ""}, + Reason: ErrAmbiguous, + errcode: getErrorCode(ErrNoRcptAddresses), + } + if err.ErrorCode() != 0 { + t.Errorf("expected error code: %d, got: %d", 0, err.ErrorCode()) + } + }) + t.Run("SendError with permanent error", func(t *testing.T) { + err := &SendError{ + errlist: []error{ErrNoRcptAddresses}, + rcpt: []string{"", ""}, + Reason: ErrAmbiguous, + errcode: getErrorCode(errors.New("535 5.7.8 Error: authentication failed")), + } + if err.ErrorCode() != 535 { + t.Errorf("expected error code: %d, got: %d", 535, err.ErrorCode()) + } + }) + t.Run("SendError with temporary error", func(t *testing.T) { + err := &SendError{ + errlist: []error{ErrNoRcptAddresses}, + rcpt: []string{"", ""}, + Reason: ErrAmbiguous, + errcode: getErrorCode(errors.New("441 4.1.0 Server currently unavailable")), + } + if err.ErrorCode() != 441 { + t.Errorf("expected error code: %d, got: %d", 441, err.ErrorCode()) + } + }) + t.Run("error code on nil error should return 0", func(t *testing.T) { + var err *SendError + if err.ErrorCode() != 0 { + t.Error("expected 0 error code on nil-senderror") + } + }) +} + // returnSendError is a helper method to retunr a SendError with a specific reason func returnSendError(r SendErrReason, t bool) error { message := NewMsg() From e8fb977afeb81436bff654c5f992f7adc0fc1f13 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 13 Nov 2024 21:48:24 +0100 Subject: [PATCH 05/14] Add tests for getErrorCode function Introduce a suite of unit tests for the getErrorCode function to validate its behavior with various error types, including go-mail errors, permanent and temporary errors, wrapper errors, non-4xx/5xx errors, and non-3-digit codes. --- senderror_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/senderror_test.go b/senderror_test.go index 5797520..7e538c6 100644 --- a/senderror_test.go +++ b/senderror_test.go @@ -6,6 +6,7 @@ package mail import ( "errors" + "fmt" "strings" "testing" ) @@ -290,6 +291,45 @@ func TestSendError_ErrorCode(t *testing.T) { }) } +func TestSendError_getErrorCode(t *testing.T) { + t.Run("getErrorCode with a go-mail error should return 0", func(t *testing.T) { + code := getErrorCode(ErrNoRcptAddresses) + if code != 0 { + t.Errorf("expected error code: %d, got: %d", 0, code) + } + }) + t.Run("getErrorCode with permanent error", func(t *testing.T) { + code := getErrorCode(errors.New("535 5.7.8 Error: authentication failed")) + if code != 535 { + t.Errorf("expected error code: %d, got: %d", 535, code) + } + }) + t.Run("getErrorCode with temporary error", func(t *testing.T) { + code := getErrorCode(errors.New("443 4.1.0 Server currently unavailable")) + if code != 443 { + t.Errorf("expected error code: %d, got: %d", 443, code) + } + }) + t.Run("getErrorCode with wrapper error", func(t *testing.T) { + code := getErrorCode(fmt.Errorf("an error occured: %w", errors.New("443 4.1.0 Server currently unavailable"))) + if code != 443 { + t.Errorf("expected error code: %d, got: %d", 443, code) + } + }) + t.Run("getErrorCode with non-4xx and non-5xx error", func(t *testing.T) { + code := getErrorCode(errors.New("220 2.1.0 This is not an error")) + if code != 0 { + t.Errorf("expected error code: %d, got: %d", 0, code) + } + }) + t.Run("getErrorCode with non 3-digit code", func(t *testing.T) { + code := getErrorCode(errors.New("4xx 4.1.0 The status code is invalid")) + if code != 0 { + t.Errorf("expected error code: %d, got: %d", 0, code) + } + }) +} + // returnSendError is a helper method to retunr a SendError with a specific reason func returnSendError(r SendErrReason, t bool) error { message := NewMsg() From 6268acac445150cfab834b293e7e5575ec9f2ab5 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 13 Nov 2024 22:47:53 +0100 Subject: [PATCH 06/14] Refactor error handling by renaming functions. Renamed `getErrorCode` to `errorCode` and `getEnhancedStatusCode` to `enhancedStatusCode` for consistency. Updated all references in `client.go` and `senderror.go` accordingly, improving readability and maintaining uniformity across the codebase. --- client.go | 32 ++++++++++++++++---------------- senderror.go | 4 ++-- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/client.go b/client.go index d64c172..9b3251e 100644 --- a/client.go +++ b/client.go @@ -1201,16 +1201,16 @@ func (c *Client) sendSingleMsg(message *Msg) error { if err != nil { return &SendError{ Reason: ErrGetSender, errlist: []error{err}, isTemp: isTempError(err), - affectedMsg: message, errcode: getErrorCode(err), - enhancedStatusCode: getEnhancedStatusCode(err, escSupport), + affectedMsg: message, errcode: errorCode(err), + enhancedStatusCode: enhancedStatusCode(err, escSupport), } } rcpts, err := message.GetRecipients() if err != nil { return &SendError{ Reason: ErrGetRcpts, errlist: []error{err}, isTemp: isTempError(err), - affectedMsg: message, errcode: getErrorCode(err), - enhancedStatusCode: getEnhancedStatusCode(err, escSupport), + affectedMsg: message, errcode: errorCode(err), + enhancedStatusCode: enhancedStatusCode(err, escSupport), } } @@ -1222,8 +1222,8 @@ func (c *Client) sendSingleMsg(message *Msg) error { if err = c.smtpClient.Mail(from); err != nil { retError := &SendError{ Reason: ErrSMTPMailFrom, errlist: []error{err}, isTemp: isTempError(err), - affectedMsg: message, errcode: getErrorCode(err), - enhancedStatusCode: getEnhancedStatusCode(err, escSupport), + affectedMsg: message, errcode: errorCode(err), + enhancedStatusCode: enhancedStatusCode(err, escSupport), } if resetSendErr := c.smtpClient.Reset(); resetSendErr != nil { retError.errlist = append(retError.errlist, resetSendErr) @@ -1242,8 +1242,8 @@ func (c *Client) sendSingleMsg(message *Msg) error { rcptSendErr.errlist = append(rcptSendErr.errlist, err) rcptSendErr.rcpt = append(rcptSendErr.rcpt, rcpt) rcptSendErr.isTemp = isTempError(err) - rcptSendErr.errcode = getErrorCode(err) - rcptSendErr.enhancedStatusCode = getEnhancedStatusCode(err, escSupport) + rcptSendErr.errcode = errorCode(err) + rcptSendErr.enhancedStatusCode = enhancedStatusCode(err, escSupport) hasError = true } } @@ -1257,23 +1257,23 @@ func (c *Client) sendSingleMsg(message *Msg) error { if err != nil { return &SendError{ Reason: ErrSMTPData, errlist: []error{err}, isTemp: isTempError(err), - affectedMsg: message, errcode: getErrorCode(err), - enhancedStatusCode: getEnhancedStatusCode(err, escSupport), + affectedMsg: message, errcode: errorCode(err), + enhancedStatusCode: enhancedStatusCode(err, escSupport), } } _, err = message.WriteTo(writer) if err != nil { return &SendError{ Reason: ErrWriteContent, errlist: []error{err}, isTemp: isTempError(err), - affectedMsg: message, errcode: getErrorCode(err), - enhancedStatusCode: getEnhancedStatusCode(err, escSupport), + affectedMsg: message, errcode: errorCode(err), + enhancedStatusCode: enhancedStatusCode(err, escSupport), } } if err = writer.Close(); err != nil { return &SendError{ Reason: ErrSMTPDataClose, errlist: []error{err}, isTemp: isTempError(err), - affectedMsg: message, errcode: getErrorCode(err), - enhancedStatusCode: getEnhancedStatusCode(err, escSupport), + affectedMsg: message, errcode: errorCode(err), + enhancedStatusCode: enhancedStatusCode(err, escSupport), } } message.isDelivered = true @@ -1281,8 +1281,8 @@ func (c *Client) sendSingleMsg(message *Msg) error { if err = c.Reset(); err != nil { return &SendError{ Reason: ErrSMTPReset, errlist: []error{err}, isTemp: isTempError(err), - affectedMsg: message, errcode: getErrorCode(err), - enhancedStatusCode: getEnhancedStatusCode(err, escSupport), + affectedMsg: message, errcode: errorCode(err), + enhancedStatusCode: enhancedStatusCode(err, escSupport), } } return nil diff --git a/senderror.go b/senderror.go index 93ab6f2..e32d74b 100644 --- a/senderror.go +++ b/senderror.go @@ -265,7 +265,7 @@ func isTempError(err error) bool { return err.Error()[0] == '4' } -func getErrorCode(err error) int { +func errorCode(err error) int { rootErr := errors.Unwrap(err) if rootErr != nil { err = rootErr @@ -282,7 +282,7 @@ func getErrorCode(err error) int { return errcode } -func getEnhancedStatusCode(err error, supported bool) string { +func enhancedStatusCode(err error, supported bool) string { if err == nil || !supported { return "" } From f367db0278db541daa802d4250dbde024def1098 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 13 Nov 2024 22:53:18 +0100 Subject: [PATCH 07/14] Refactor error code functions and add enhanced status code tests Renamed `getErrorCode` function to `errorCode` for consistency. Added new tests for the `enhancedStatusCode` function to validate its behavior with various error scenarios. --- senderror_test.go | 65 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 49 insertions(+), 16 deletions(-) diff --git a/senderror_test.go b/senderror_test.go index 7e538c6..63d4c73 100644 --- a/senderror_test.go +++ b/senderror_test.go @@ -255,7 +255,7 @@ func TestSendError_ErrorCode(t *testing.T) { errlist: []error{ErrNoRcptAddresses}, rcpt: []string{"", ""}, Reason: ErrAmbiguous, - errcode: getErrorCode(ErrNoRcptAddresses), + errcode: errorCode(ErrNoRcptAddresses), } if err.ErrorCode() != 0 { t.Errorf("expected error code: %d, got: %d", 0, err.ErrorCode()) @@ -266,7 +266,7 @@ func TestSendError_ErrorCode(t *testing.T) { errlist: []error{ErrNoRcptAddresses}, rcpt: []string{"", ""}, Reason: ErrAmbiguous, - errcode: getErrorCode(errors.New("535 5.7.8 Error: authentication failed")), + errcode: errorCode(errors.New("535 5.7.8 Error: authentication failed")), } if err.ErrorCode() != 535 { t.Errorf("expected error code: %d, got: %d", 535, err.ErrorCode()) @@ -277,7 +277,7 @@ func TestSendError_ErrorCode(t *testing.T) { errlist: []error{ErrNoRcptAddresses}, rcpt: []string{"", ""}, Reason: ErrAmbiguous, - errcode: getErrorCode(errors.New("441 4.1.0 Server currently unavailable")), + errcode: errorCode(errors.New("441 4.1.0 Server currently unavailable")), } if err.ErrorCode() != 441 { t.Errorf("expected error code: %d, got: %d", 441, err.ErrorCode()) @@ -291,45 +291,78 @@ func TestSendError_ErrorCode(t *testing.T) { }) } -func TestSendError_getErrorCode(t *testing.T) { - t.Run("getErrorCode with a go-mail error should return 0", func(t *testing.T) { - code := getErrorCode(ErrNoRcptAddresses) +func TestSendError_errorCode(t *testing.T) { + t.Run("errorCode with a go-mail error should return 0", func(t *testing.T) { + code := errorCode(ErrNoRcptAddresses) if code != 0 { t.Errorf("expected error code: %d, got: %d", 0, code) } }) - t.Run("getErrorCode with permanent error", func(t *testing.T) { - code := getErrorCode(errors.New("535 5.7.8 Error: authentication failed")) + t.Run("errorCode with permanent error", func(t *testing.T) { + code := errorCode(errors.New("535 5.7.8 Error: authentication failed")) if code != 535 { t.Errorf("expected error code: %d, got: %d", 535, code) } }) - t.Run("getErrorCode with temporary error", func(t *testing.T) { - code := getErrorCode(errors.New("443 4.1.0 Server currently unavailable")) + t.Run("errorCode with temporary error", func(t *testing.T) { + code := errorCode(errors.New("443 4.1.0 Server currently unavailable")) if code != 443 { t.Errorf("expected error code: %d, got: %d", 443, code) } }) - t.Run("getErrorCode with wrapper error", func(t *testing.T) { - code := getErrorCode(fmt.Errorf("an error occured: %w", errors.New("443 4.1.0 Server currently unavailable"))) + t.Run("errorCode with wrapper error", func(t *testing.T) { + code := errorCode(fmt.Errorf("an error occured: %w", errors.New("443 4.1.0 Server currently unavailable"))) if code != 443 { t.Errorf("expected error code: %d, got: %d", 443, code) } }) - t.Run("getErrorCode with non-4xx and non-5xx error", func(t *testing.T) { - code := getErrorCode(errors.New("220 2.1.0 This is not an error")) + t.Run("errorCode with non-4xx and non-5xx error", func(t *testing.T) { + code := errorCode(errors.New("220 2.1.0 This is not an error")) if code != 0 { t.Errorf("expected error code: %d, got: %d", 0, code) } }) - t.Run("getErrorCode with non 3-digit code", func(t *testing.T) { - code := getErrorCode(errors.New("4xx 4.1.0 The status code is invalid")) + t.Run("errorCode with non 3-digit code", func(t *testing.T) { + code := errorCode(errors.New("4xx 4.1.0 The status code is invalid")) if code != 0 { t.Errorf("expected error code: %d, got: %d", 0, code) } }) } +func TestSendError_enhancedStatusCode(t *testing.T) { + t.Run("enhancedStatusCode with nil error should return empty string", func(t *testing.T) { + code := enhancedStatusCode(nil, true) + if code != "" { + t.Errorf("expected empty enhanced status code, got: %s", code) + } + }) + t.Run("enhancedStatusCode with error but no support should return empty string", func(t *testing.T) { + code := enhancedStatusCode(errors.New("553 5.5.3 something went wrong"), false) + if code != "" { + t.Errorf("expected empty enhanced status code, got: %s", code) + } + }) + t.Run("enhancedStatusCode with error and support", func(t *testing.T) { + code := enhancedStatusCode(errors.New("553 5.5.3 something went wrong"), true) + if code != "5.5.3" { + t.Errorf("expected enhanced status code: %s, got: %s", "5.5.3", code) + } + }) + t.Run("enhancedStatusCode with wrapped error and support", func(t *testing.T) { + code := enhancedStatusCode(fmt.Errorf("this error is wrapped: %w", errors.New("553 5.5.3 something went wrong")), true) + if code != "5.5.3" { + t.Errorf("expected enhanced status code: %s, got: %s", "5.5.3", code) + } + }) + t.Run("enhancedStatusCode with 3xx error", func(t *testing.T) { + code := enhancedStatusCode(errors.New("300 3.0.0 i don't know what i'm doing"), true) + if code != "" { + t.Errorf("expected enhanced status code to be empty, got: %s", code) + } + }) +} + // returnSendError is a helper method to retunr a SendError with a specific reason func returnSendError(r SendErrReason, t bool) error { message := NewMsg() From 719e5b217cbef2ef056c901f769f01eae0483a8f Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 13 Nov 2024 23:02:30 +0100 Subject: [PATCH 08/14] Enhance error handling with ENHANCEDSTATUSCODES check Added a check for the ENHANCEDSTATUSCODES extension and included error code and enhanced status code information in SendError. This helps in providing more detailed error reporting and troubleshooting. --- client_119.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/client_119.go b/client_119.go index 093967e..b9931b8 100644 --- a/client_119.go +++ b/client_119.go @@ -27,8 +27,13 @@ import "errors" // - An error that represents the sending result, which may include multiple SendErrors if // any occurred; otherwise, returns nil. func (c *Client) Send(messages ...*Msg) error { + escSupport := false + if c.smtpClient != nil { + escSupport, _ = c.smtpClient.Extension("ENHANCEDSTATUSCODES") + } if err := c.checkConn(); err != nil { - return &SendError{Reason: ErrConnCheck, errlist: []error{err}, isTemp: isTempError(err)} + return &SendError{Reason: ErrConnCheck, errlist: []error{err}, isTemp: isTempError(err), + errcode: errorCode(err), enhancedStatusCode: enhancedStatusCode(err, escSupport)} } var errs []*SendError for id, message := range messages { From a5ac7c33708413f39ad99001ef01570e65603280 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 13 Nov 2024 23:04:39 +0100 Subject: [PATCH 09/14] Update error handling to include error code and status Previously, only the isTemp flag was considered when aggregating errors. Now, the error code and enhanced status code from the last error are also included. This ensures more comprehensive error reporting and handling. --- client_119.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/client_119.go b/client_119.go index b9931b8..5837337 100644 --- a/client_119.go +++ b/client_119.go @@ -55,9 +55,11 @@ func (c *Client) Send(messages ...*Msg) error { returnErr.rcpt = append(returnErr.rcpt, errs[i].rcpt...) } - // We assume that the isTemp flag from the last error we received should be the + // We assume that the error codes and flags from the last error we received should be the // indicator for the returned isTemp flag as well returnErr.isTemp = errs[len(errs)-1].isTemp + returnErr.errcode = errs[len(errs)-1].errcode + returnErr.enhancedStatusCode = errs[len(errs)-1].enhancedStatusCode return returnErr } From c8d7cf86e14642aee524bcfd703bbaca45583208 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Thu, 14 Nov 2024 10:17:18 +0100 Subject: [PATCH 10/14] Enhance error handling in Client's Send method Added support for Enhanced Status Codes (ESC) when checking the SMTP client's extensions. The SendError struct now includes the error code and enhanced status code for improved diagnostics. --- client_120.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/client_120.go b/client_120.go index 012a4f7..38eb76e 100644 --- a/client_120.go +++ b/client_120.go @@ -27,8 +27,13 @@ import ( // Returns: // - An error that aggregates any SendErrors encountered during the sending process; otherwise, returns nil. func (c *Client) Send(messages ...*Msg) (returnErr error) { + escSupport := false + if c.smtpClient != nil { + escSupport, _ = c.smtpClient.Extension("ENHANCEDSTATUSCODES") + } if err := c.checkConn(); err != nil { - returnErr = &SendError{Reason: ErrConnCheck, errlist: []error{err}, isTemp: isTempError(err)} + returnErr = &SendError{Reason: ErrConnCheck, errlist: []error{err}, isTemp: isTempError(err), + errcode: errorCode(err), enhancedStatusCode: enhancedStatusCode(err, escSupport)} return } From bd655b768b9c44a62f6fc7b61d6e97ac766de86d Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Thu, 14 Nov 2024 10:20:52 +0100 Subject: [PATCH 11/14] Refactor SendError initialization for better readability Structured the initialization of SendError on connection errors to improve code readability and maintainability. This change affects the error handling in both client_120.go and client_119.go by spreading the error details across multiple lines. --- client_119.go | 6 ++++-- client_120.go | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/client_119.go b/client_119.go index 5837337..a28f747 100644 --- a/client_119.go +++ b/client_119.go @@ -32,8 +32,10 @@ func (c *Client) Send(messages ...*Msg) error { escSupport, _ = c.smtpClient.Extension("ENHANCEDSTATUSCODES") } if err := c.checkConn(); err != nil { - return &SendError{Reason: ErrConnCheck, errlist: []error{err}, isTemp: isTempError(err), - errcode: errorCode(err), enhancedStatusCode: enhancedStatusCode(err, escSupport)} + return &SendError{ + Reason: ErrConnCheck, errlist: []error{err}, isTemp: isTempError(err), + errcode: errorCode(err), enhancedStatusCode: enhancedStatusCode(err, escSupport), + } } var errs []*SendError for id, message := range messages { diff --git a/client_120.go b/client_120.go index 38eb76e..67c5b5e 100644 --- a/client_120.go +++ b/client_120.go @@ -32,8 +32,10 @@ func (c *Client) Send(messages ...*Msg) (returnErr error) { escSupport, _ = c.smtpClient.Extension("ENHANCEDSTATUSCODES") } if err := c.checkConn(); err != nil { - returnErr = &SendError{Reason: ErrConnCheck, errlist: []error{err}, isTemp: isTempError(err), - errcode: errorCode(err), enhancedStatusCode: enhancedStatusCode(err, escSupport)} + returnErr = &SendError{ + Reason: ErrConnCheck, errlist: []error{err}, isTemp: isTempError(err), + errcode: errorCode(err), enhancedStatusCode: enhancedStatusCode(err, escSupport), + } return } From ca3f50552e283eb2e7b040d2038b7157b7ba11b0 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Thu, 14 Nov 2024 10:36:24 +0100 Subject: [PATCH 12/14] Allow configuration of test server port via environment variable Moved TestServerPortBase initialization to use an environment variable `TEST_BASEPORT` if provided. This adjustment helps in specifying custom base ports for running tests, ensuring better flexibility in different testing environments. --- client_test.go | 18 ++++++++++++++++-- smtp/smtp_test.go | 18 ++++++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/client_test.go b/client_test.go index d1afc86..2ac8811 100644 --- a/client_test.go +++ b/client_test.go @@ -17,6 +17,7 @@ import ( "net/mail" "os" "reflect" + "strconv" "strings" "sync" "sync/atomic" @@ -34,14 +35,15 @@ const ( TestServerProto = "tcp" // TestServerAddr is the address the simple SMTP test server listens on TestServerAddr = "127.0.0.1" - // TestServerPortBase is the base port for the simple SMTP test server - TestServerPortBase = 30025 // TestSenderValid is a test sender email address considered valid for sending test emails. TestSenderValid = "valid-from@domain.tld" // TestRcptValid is a test recipient email address considered valid for sending test emails. TestRcptValid = "valid-to@domain.tld" ) +// TestServerPortBase is the base port for the simple SMTP test server +var TestServerPortBase int32 = 30025 + // PortAdder is an atomic counter used to increment port numbers for the test SMTP server instances. var PortAdder atomic.Int32 @@ -98,6 +100,18 @@ type logData struct { Lines []logLine `json:"lines"` } +func init() { + testPort := os.Getenv("TEST_BASEPORT") + if testPort == "" { + return + } + if port, err := strconv.Atoi(testPort); err == nil { + if port <= 65000 && port > 1023 { + TestServerPortBase = int32(port) + } + } +} + func TestNewClient(t *testing.T) { t.Run("create new Client", func(t *testing.T) { client, err := NewClient(DefaultHost) diff --git a/smtp/smtp_test.go b/smtp/smtp_test.go index 1b616fc..764d65b 100644 --- a/smtp/smtp_test.go +++ b/smtp/smtp_test.go @@ -30,6 +30,7 @@ import ( "io" "net" "os" + "strconv" "strings" "sync" "sync/atomic" @@ -46,13 +47,14 @@ const ( TestServerProto = "tcp" // TestServerAddr is the address the simple SMTP test server listens on TestServerAddr = "127.0.0.1" - // TestServerPortBase is the base port for the simple SMTP test server - TestServerPortBase = 30025 ) // PortAdder is an atomic counter used to increment port numbers for the test SMTP server instances. var PortAdder atomic.Int32 +// TestServerPortBase is the base port for the simple SMTP test server +var TestServerPortBase int32 = 30025 + // localhostCert is a PEM-encoded TLS cert generated from src/crypto/tls: // // go run generate_cert.go --rsa-bits 1024 --host 127.0.0.1,::1,example.com \ @@ -231,6 +233,18 @@ var authTests = []authTest{ }, } +func init() { + testPort := os.Getenv("TEST_BASEPORT") + if testPort == "" { + return + } + if port, err := strconv.Atoi(testPort); err == nil { + if port <= 65000 && port > 1023 { + TestServerPortBase = int32(port) + } + } +} + func TestAuth(t *testing.T) { t.Run("Auth for all supported auth methods", func(t *testing.T) { for i, tt := range authTests { From a70dde5a4d976808d881e4a62a12620cca6262e2 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Thu, 14 Nov 2024 10:41:10 +0100 Subject: [PATCH 13/14] Add TEST_BASEPORT environment variable to CI workflow In the CI configuration file, the TEST_BASEPORT environment variable was added to various job scopes. This ensures consistency and allows the test base port to be set properly across different OS versions and Go versions. --- .github/workflows/ci.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2a842e9..7de4077 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,6 +33,7 @@ jobs: PERFORM_ONLINE_TEST: ${{ vars.PERFORM_ONLINE_TEST }} PERFORM_UNIX_OPEN_WRITE_TESTS: ${{ vars.PERFORM_UNIX_OPEN_WRITE_TESTS }} PERFORM_SENDMAIL_TESTS: ${{ vars.PERFORM_SENDMAIL_TESTS }} + TEST_BASEPORT: ${{ vars.TEST_BASEPORT }} TEST_HOST: ${{ secrets.TEST_HOST }} TEST_USER: ${{ secrets.TEST_USER }} TEST_PASS: ${{ secrets.TEST_PASS }} @@ -126,6 +127,8 @@ jobs: matrix: os: [ubuntu-latest, macos-latest, windows-latest] go: ['1.19', '1.20', '1.21', '1.22', '1.23'] + env: + TEST_BASEPORT: ${{ vars.TEST_BASEPORT }} steps: - name: Harden Runner uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1 @@ -149,6 +152,8 @@ jobs: strategy: matrix: osver: ['14.1', '14.0', 13.4'] + env: + TEST_BASEPORT: ${{ vars.TEST_BASEPORT }} steps: - name: Checkout Code uses: actions/checkout@61b9e3751b92087fd0b06925ba6dd6314e06f089 # master @@ -189,6 +194,7 @@ jobs: go: ['1.23'] env: PERFORM_ONLINE_TEST: ${{ vars.PERFORM_ONLINE_TEST }} + TEST_BASEPORT: ${{ vars.TEST_BASEPORT }} TEST_HOST: ${{ secrets.TEST_HOST }} TEST_USER: ${{ secrets.TEST_USER }} TEST_PASS: ${{ secrets.TEST_PASS }} From 2bde3404286584c8caf628297609869a85634203 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Thu, 14 Nov 2024 10:45:35 +0100 Subject: [PATCH 14/14] Update SMTP test port variable and CI configuration Changed the SMTP test server base port and updated the corresponding environment variable name to `TEST_BASEPORT_SMTP`. This ensures consistency across the test setup and CI workflow configuration. --- .github/workflows/ci.yml | 4 ++++ smtp/smtp_test.go | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7de4077..8004942 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,6 +34,7 @@ jobs: PERFORM_UNIX_OPEN_WRITE_TESTS: ${{ vars.PERFORM_UNIX_OPEN_WRITE_TESTS }} PERFORM_SENDMAIL_TESTS: ${{ vars.PERFORM_SENDMAIL_TESTS }} TEST_BASEPORT: ${{ vars.TEST_BASEPORT }} + TEST_BASEPORT_SMTP: ${{ vars.TEST_BASEPORT_SMTP }} TEST_HOST: ${{ secrets.TEST_HOST }} TEST_USER: ${{ secrets.TEST_USER }} TEST_PASS: ${{ secrets.TEST_PASS }} @@ -129,6 +130,7 @@ jobs: go: ['1.19', '1.20', '1.21', '1.22', '1.23'] env: TEST_BASEPORT: ${{ vars.TEST_BASEPORT }} + TEST_BASEPORT_SMTP: ${{ vars.TEST_BASEPORT_SMTP }} steps: - name: Harden Runner uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1 @@ -154,6 +156,7 @@ jobs: osver: ['14.1', '14.0', 13.4'] env: TEST_BASEPORT: ${{ vars.TEST_BASEPORT }} + TEST_BASEPORT_SMTP: ${{ vars.TEST_BASEPORT_SMTP }} steps: - name: Checkout Code uses: actions/checkout@61b9e3751b92087fd0b06925ba6dd6314e06f089 # master @@ -195,6 +198,7 @@ jobs: env: PERFORM_ONLINE_TEST: ${{ vars.PERFORM_ONLINE_TEST }} TEST_BASEPORT: ${{ vars.TEST_BASEPORT }} + TEST_BASEPORT_SMTP: ${{ vars.TEST_BASEPORT_SMTP }} TEST_HOST: ${{ secrets.TEST_HOST }} TEST_USER: ${{ secrets.TEST_USER }} TEST_PASS: ${{ secrets.TEST_PASS }} diff --git a/smtp/smtp_test.go b/smtp/smtp_test.go index 764d65b..471b1e1 100644 --- a/smtp/smtp_test.go +++ b/smtp/smtp_test.go @@ -53,7 +53,7 @@ const ( var PortAdder atomic.Int32 // TestServerPortBase is the base port for the simple SMTP test server -var TestServerPortBase int32 = 30025 +var TestServerPortBase int32 = 20025 // localhostCert is a PEM-encoded TLS cert generated from src/crypto/tls: // @@ -234,7 +234,7 @@ var authTests = []authTest{ } func init() { - testPort := os.Getenv("TEST_BASEPORT") + testPort := os.Getenv("TEST_BASEPORT_SMTP") if testPort == "" { return }