From 0e4182177969c2a0dc6df53870442d5c54d85752 Mon Sep 17 00:00:00 2001 From: Alberto Bertogli Date: Sun, 2 Oct 2016 01:16:39 +0100 Subject: [PATCH] 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 | 8 ++++---- internal/smtp/smtp.go | 18 ++++++++++++++++++ internal/smtp/smtp_test.go | 19 +++++++++++++++++++ 3 files changed, 41 insertions(+), 4 deletions(-) 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