git » chasquid » commit 0e41821

smtp: Distinguish permanent errors

author Alberto Bertogli
2016-10-02 00:16:39 UTC
committer Alberto Bertogli
2016-10-09 23:51:04 UTC
parent 7cbe6a5076286e2974b17dbd602e290c29801783

smtp: Distinguish permanent errors

This patch makes the SMTP courier distinguish permanent errors, so the queue
can decide whether to retry or not.

It's based on the SMTP replies: 5xy is considerent permanent, anything else is
transient (including errors other than protocol issues).

internal/courier/smtp.go +4 -4
internal/smtp/smtp.go +18 -0
internal/smtp/smtp_test.go +19 -0

diff --git a/internal/courier/smtp.go b/internal/courier/smtp.go
index 6cfed97..8c5f7f7 100644
--- a/internal/courier/smtp.go
+++ b/internal/courier/smtp.go
@@ -107,21 +107,21 @@ retry:
 		from = ""
 	}
 	if err = c.MailAndRcpt(from, to); err != nil {
-		return tr.Errorf("MAIL+RCPT %v", err), false
+		return tr.Errorf("MAIL+RCPT %v", err), smtp.IsPermanent(err)
 	}
 
 	w, err := c.Data()
 	if err != nil {
-		return tr.Errorf("DATA %v", err), false
+		return tr.Errorf("DATA %v", err), smtp.IsPermanent(err)
 	}
 	_, err = w.Write(data)
 	if err != nil {
-		return tr.Errorf("DATA writing: %v", err), false
+		return tr.Errorf("DATA writing: %v", err), smtp.IsPermanent(err)
 	}
 
 	err = w.Close()
 	if err != nil {
-		return tr.Errorf("DATA closing %v", err), false
+		return tr.Errorf("DATA closing %v", err), smtp.IsPermanent(err)
 	}
 
 	c.Quit()
diff --git a/internal/smtp/smtp.go b/internal/smtp/smtp.go
index f20c78e..6c1637b 100644
--- a/internal/smtp/smtp.go
+++ b/internal/smtp/smtp.go
@@ -2,6 +2,7 @@
 // 5321.  It extends net/smtp as follows:
 //
 //  - Supports SMTPUTF8, via MailAndRcpt.
+//  - Adds IsPermanent.
 //
 package smtp
 
@@ -127,3 +128,20 @@ func isASCII(s string) bool {
 	}
 	return true
 }
+
+// ErrIsPermanent returns true if the error is permanent, and false otherwise.
+// If it can't tell, it returns false.
+func IsPermanent(err error) bool {
+	terr, ok := err.(*textproto.Error)
+	if !ok {
+		return false
+	}
+
+	// Error codes 5yz are permanent.
+	// https://tools.ietf.org/html/rfc5321#section-4.2.1
+	if terr.Code >= 500 && terr.Code < 600 {
+		return true
+	}
+
+	return false
+}
diff --git a/internal/smtp/smtp_test.go b/internal/smtp/smtp_test.go
index e6589bf..5da6d05 100644
--- a/internal/smtp/smtp_test.go
+++ b/internal/smtp/smtp_test.go
@@ -3,6 +3,7 @@ package smtp
 import (
 	"bufio"
 	"bytes"
+	"fmt"
 	"net"
 	"net/smtp"
 	"net/textproto"
@@ -11,6 +12,24 @@ import (
 	"time"
 )
 
+func TestIsPermanent(t *testing.T) {
+	cases := []struct {
+		err       error
+		permanent bool
+	}{
+		{&textproto.Error{499, ""}, false},
+		{&textproto.Error{500, ""}, true},
+		{&textproto.Error{599, ""}, true},
+		{&textproto.Error{600, ""}, false},
+		{fmt.Errorf("something"), false},
+	}
+	for _, c := range cases {
+		if p := IsPermanent(c.err); p != c.permanent {
+			t.Errorf("%v: expected %v, got %v", c.err, c.permanent, p)
+		}
+	}
+}
+
 func TestIsASCII(t *testing.T) {
 	cases := []struct {
 		str   string