smtpconn: Treat wrapped dial errors as network errors

internal/smtpconn/smtpconn.go

@@ -138,41 +138,54 @@ func (c *C) wrapClientErr(err error, serverName string) error {
 			Err: err,
 		}
 	case *net.OpError:
-		if _, ok := err.Err.(*net.DNSError); ok {
-			reason, misc := exterrors.UnwrapDNSErr(err)
-			misc["remote_server"] = err.Addr
-			misc["io_op"] = err.Op
-
-			code := exterrors.SMTPCode(err, 450, 550)
-			enhancedCode := exterrors.SMTPEnchCode(err, exterrors.EnhancedCode{0, 4, 4})
-			if c.DNSErrorsTemporary {
-				code = 451
-				enhancedCode = exterrors.EnhancedCode{4, 4, 4}
-			}
+		return c.wrapNetErr(err, err)
+	default:
+		// Dial errors are wrapped by attemptConnect, so they need to be
+		// unwrapped to be recognized as (temporary) network errors.
+		var opErr *net.OpError
+		if errors.As(err, &opErr) {
+			return c.wrapNetErr(err, opErr)
+		}
+		return exterrors.WithFields(err, map[string]interface{}{
+			"remote_server": serverName,
+		})
+	}
+}
 
-			return &exterrors.SMTPError{
-				Code:         code,
-				EnhancedCode: enhancedCode,
-				Message:      "DNS error",
-				Err:          err,
-				Reason:       reason,
-				Misc:         misc,
-			}
+// wrapNetErr converts a network error into an SMTP error. err is the error to
+// wrap (it may contain additional context) and opErr is the *net.OpError
+// found in it.
+func (c *C) wrapNetErr(err error, opErr *net.OpError) error {
+	if _, ok := opErr.Err.(*net.DNSError); ok {
+		reason, misc := exterrors.UnwrapDNSErr(opErr)
+		misc["remote_server"] = opErr.Addr
+		misc["io_op"] = opErr.Op
+
+		code := exterrors.SMTPCode(opErr, 450, 550)
+		enhancedCode := exterrors.SMTPEnchCode(opErr, exterrors.EnhancedCode{0, 4, 4})
+		if c.DNSErrorsTemporary {
+			code = 451
+			enhancedCode = exterrors.EnhancedCode{4, 4, 4}
 		}
+
 		return &exterrors.SMTPError{
-			Code:         450,
-			EnhancedCode: exterrors.EnhancedCode{4, 4, 2},
-			Message:      "Network I/O error",
+			Code:         code,
+			EnhancedCode: enhancedCode,
+			Message:      "DNS error",
 			Err:          err,
-			Misc: map[string]interface{}{
-				"remote_addr": err.Addr,
-				"io_op":       err.Op,
-			},
+			Reason:       reason,
+			Misc:         misc,
 		}
-	default:
-		return exterrors.WithFields(err, map[string]interface{}{
-			"remote_server": serverName,
-		})
+	}
+	return &exterrors.SMTPError{
+		Code:         450,
+		EnhancedCode: exterrors.EnhancedCode{4, 4, 2},
+		Message:      "Network I/O error",
+		Err:          err,
+		Misc: map[string]interface{}{
+			"remote_addr": opErr.Addr,
+			"io_op":       opErr.Op,
+		},
 	}
 }
 

internal/smtpconn/smtpconn_test.go

@@ -19,11 +19,18 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
 package smtpconn
 
 import (
+	"context"
+	"errors"
 	"flag"
 	"math/rand"
+	"net"
 	"os"
 	"strconv"
 	"testing"
+
+	"github.com/foxcpp/maddy/framework/config"
+	"github.com/foxcpp/maddy/framework/exterrors"
+	"github.com/foxcpp/maddy/internal/testutils"
 )
 
 var testPort string
@@ -39,3 +46,50 @@ func TestMain(m *testing.M) {
 	testPort = *remoteSmtpPort
 	os.Exit(m.Run())
 }
+
+// TestConnectRefused checks that a failed dial is reported as a temporary
+// "Network I/O error", so the sender retries instead of bouncing the message.
+func TestConnectRefused(t *testing.T) {
+	// Get a local port with nothing listening on it.
+	l, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	port := strconv.Itoa(l.Addr().(*net.TCPAddr).Port)
+	if err := l.Close(); err != nil {
+		t.Fatal(err)
+	}
+	endp := config.Endpoint{Scheme: "tcp", Host: "127.0.0.1", Port: port}
+
+	connect := map[string]func(*C) error{
+		"Connect": func(c *C) error {
+			_, err := c.Connect(context.Background(), endp, false, nil)
+			return err
+		},
+		"ConnectLMTP": func(c *C) error {
+			_, err := c.ConnectLMTP(context.Background(), endp, false, nil)
+			return err
+		},
+	}
+	for name, fn := range connect {
+		t.Run(name, func(t *testing.T) {
+			c := New()
+			c.Log = testutils.Logger(t, "smtpconn")
+
+			err := fn(c)
+			if err == nil {
+				t.Fatal("expected an error")
+			}
+			var smtpErr *exterrors.SMTPError
+			if !errors.As(err, &smtpErr) {
+				t.Fatalf("expected *exterrors.SMTPError, got %T: %v", err, err)
+			}
+			if smtpErr.Code != 450 || smtpErr.EnhancedCode != (exterrors.EnhancedCode{4, 4, 2}) {
+				t.Errorf("expected 450 4.4.2, got %d %v", smtpErr.Code, smtpErr.EnhancedCode)
+			}
+			if !exterrors.IsTemporary(err) {
+				t.Error("expected a temporary error")
+			}
+		})
+	}
+}