improve dsn handling

have the full smtp reply in the Diagnostic-Code field, not something that
resembles it but isn't quite the same.

include any additional error message in the Status field as comment.

before, we ended up having an Diagnostic-Code that didn't include the original
smtp code. it only had the enhanced error code.

dsn/dsn.go

@@ -11,7 +11,6 @@ import (
 	"io"
 	"mime/multipart"
 	"net/textproto"
-	"strconv"
 	"strings"
 	"time"
 
@@ -100,9 +99,10 @@ type Recipient struct {
 	Action         Action
 
 	// Enhanced status code. First digit indicates permanent or temporary
-	// error. If the string contains more than just a status, that
+	// error.
-	// additional text is added as comment when composing a DSN.
 	Status string
+	// For additional details, included in comment.
+	StatusComment string
 
 	// Optional fields.
 	// Original intended recipient of message. Used with the DSN extensions ORCPT
@@ -114,10 +114,10 @@ type Recipient struct {
 	// deliveries.
 	RemoteMTA NameIP
 
-	// If RemoteMTA is present, DiagnosticCode is from remote. When
+	// DiagnosticCode should either be empty, or start with "smtp; " followed by the
-	// creating a DSN, additional text in the string will be added to the
+	// literal full SMTP response lines, space separated.
-	// DSN as comment.
 	DiagnosticCode string
+
 	LastAttemptDate time.Time
 	FinalLogID      string
 
@@ -272,11 +272,9 @@ func (m *Message) Compose(log mlog.Log, smtputf8 bool) ([]byte, error) {
 				st = "2.0.0"
 			}
 		}
-		var rest string
-		st, rest = codeLine(st)
 		statusLine := st
-		if rest != "" {
+		if r.StatusComment != "" {
-			statusLine += " (" + rest + ")"
+			statusLine += " (" + r.StatusComment + ")"
 		}
 		status("Status", statusLine) // ../rfc/3464:975
 		if !r.RemoteMTA.IsZero() {
@@ -289,13 +287,8 @@ func (m *Message) Compose(log mlog.Log, smtputf8 bool) ([]byte, error) {
 		}
 		// Presence of Diagnostic-Code indicates the code is from Remote-MTA. ../rfc/3464:1053
 		if r.DiagnosticCode != "" {
-			diagCode, rest := codeLine(r.DiagnosticCode)
+			// ../rfc/3461:1342 ../rfc/6533:589
-			diagLine := diagCode
+			status("Diagnostic-Code", r.DiagnosticCode)
-			if rest != "" {
-				diagLine += " (" + rest + ")"
-			}
-			// ../rfc/6533:589
-			status("Diagnostic-Code", "smtp; "+diagLine)
 		}
 		if !r.LastAttemptDate.IsZero() {
 			status("Last-Attempt-Date", r.LastAttemptDate.Format(message.RFC5322Z)) // ../rfc/3464:1076
@@ -388,34 +381,3 @@ func (w *errWriter) Write(buf []byte) (int, error) {
 	w.err = err
 	return n, err
 }
-
-// split a line into enhanced status code and rest.
-func codeLine(s string) (string, string) {
-	t := strings.SplitN(s, " ", 2)
-	l := strings.Split(t[0], ".")
-	if len(l) != 3 {
-		return "", s
-	}
-	for i, e := range l {
-		_, err := strconv.ParseInt(e, 10, 32)
-		if err != nil {
-			return "", s
-		}
-		if i == 0 && len(e) != 1 {
-			return "", s
-		}
-	}
-
-	var rest string
-	if len(t) == 2 {
-		rest = t[1]
-	}
-	return t[0], rest
-}
-
-// HasCode returns whether line starts with an enhanced SMTP status code.
-func HasCode(line string) bool {
-	// ../rfc/3464:986
-	ecode, _ := codeLine(line)
-	return ecode != ""
-}

dsn/dsn_test.go

@@ -192,34 +192,3 @@ func TestDSN(t *testing.T) {
 	tcheckType(t, &part.Parts[1], "message", "global-delivery-status", "8bit")
 	tcompare(t, pmsg.Recipients[0].FinalRecipient, m.Recipients[0].FinalRecipient)
 }
-
-func TestCode(t *testing.T) {
-	testCodeLine := func(line, ecode, rest string) {
-		t.Helper()
-		e, r := codeLine(line)
-		if e != ecode || r != rest {
-			t.Fatalf("codeLine %q: got %q %q, expected %q %q", line, e, r, ecode, rest)
-		}
-	}
-	testCodeLine("4.0.0", "4.0.0", "")
-	testCodeLine("4.0.0 more", "4.0.0", "more")
-	testCodeLine("other", "", "other")
-	testCodeLine("other more", "", "other more")
-
-	testHasCode := func(line string, exp bool) {
-		t.Helper()
-		got := HasCode(line)
-		if got != exp {
-			t.Fatalf("HasCode %q: got %v, expected %v", line, got, exp)
-		}
-	}
-	testHasCode("4.0.0", true)
-	testHasCode("5.7.28", true)
-	testHasCode("10.0.0", false) // first number must be single digit.
-	testHasCode("4.1.1 more", true)
-	testHasCode("other ", false)
-	testHasCode("4.2.", false)
-	testHasCode("4.2. ", false)
-	testHasCode(" 4.2.4", false)
-	testHasCode(" 4.2.4 ", false)
-}

dsn/parse.go

@@ -226,6 +226,13 @@ func parseRecipientHeader(mr *textproto.Reader, utf8 bool) (Recipient, error) {
 		case "Status":
 			// todo: parse the enhanced status code?
 			r.Status = v
+			t := strings.SplitN(v, "(", 2)
+			v = strings.TrimSpace(v)
+			if len(t) == 2 && strings.HasSuffix(v, ")") {
+				r.Status = strings.TrimSpace(t[0])
+				r.StatusComment = strings.TrimSpace(strings.TrimSuffix(t[1], ")"))
+			}
+
 		case "Remote-Mta":
 			r.RemoteMTA = NameIP{Name: v}
 		case "Diagnostic-Code":

queue/direct.go

@@ -89,14 +89,19 @@ var (
 )
 
 // todo: rename function, perhaps put some of the params in a delivery struct so we don't pass all the params all the time?
-func fail(ctx context.Context, qlog mlog.Log, m Msg, backoff time.Duration, permanent bool, remoteMTA dsn.NameIP, secodeOpt, errmsg string, moreLines []string) {
+func fail(ctx context.Context, qlog mlog.Log, m Msg, backoff time.Duration, permanent bool, remoteMTA dsn.NameIP, secodeOpt, errmsg, firstLine string, moreLines []string) {
 	// todo future: when we implement relaying, we should be able to send DSNs to non-local users. and possibly specify a null mailfrom. ../rfc/5321:1503
 	// todo future: when we implement relaying, and a dsn cannot be delivered, and requiretls was active, we cannot drop the message. instead deliver to local postmaster? though ../rfc/8689:383 may intend to say the dsn should be delivered without requiretls?
 	// todo future: when we implement smtp dsn extension, parameter RET=FULL must be disregarded for messages with REQUIRETLS. ../rfc/8689:379
 
+	var smtpLines []string
+	if firstLine != "" {
+		smtpLines = append([]string{firstLine}, moreLines...)
+	}
+
 	if permanent || m.MaxAttempts == 0 && m.Attempts >= 8 || m.MaxAttempts > 0 && m.Attempts >= m.MaxAttempts {
 		qlog.Errorx("permanent failure delivering from queue", errors.New(errmsg))
-		deliverDSNFailure(ctx, qlog, m, remoteMTA, secodeOpt, errmsg, moreLines)
+		deliverDSNFailure(ctx, qlog, m, remoteMTA, secodeOpt, errmsg, smtpLines)
 
 		if err := queueDelete(context.Background(), m.ID); err != nil {
 			qlog.Errorx("deleting message from queue after permanent failure", err)
@@ -116,7 +121,7 @@ func fail(ctx context.Context, qlog mlog.Log, m Msg, backoff time.Duration, perm
 		qlog.Errorx("temporary failure delivering from queue, sending delayed dsn", errors.New(errmsg), slog.Duration("backoff", backoff))
 
 		retryUntil := m.LastAttempt.Add((4 + 8 + 16) * time.Hour)
-		deliverDSNDelay(ctx, qlog, m, remoteMTA, secodeOpt, errmsg, moreLines, retryUntil)
+		deliverDSNDelay(ctx, qlog, m, remoteMTA, secodeOpt, errmsg, smtpLines, retryUntil)
 	} else {
 		qlog.Errorx("temporary failure delivering from queue", errors.New(errmsg), slog.Duration("backoff", backoff), slog.Time("nextattempt", m.NextAttempt))
 	}
@@ -168,7 +173,7 @@ func deliverDirect(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale
 			recipientDomainResult.Summary.TotalFailureSessionCount++
 		}
 
-		fail(ctx, qlog, m, backoff, permanent, dsn.NameIP{}, "", err.Error(), nil)
+		fail(ctx, qlog, m, backoff, permanent, dsn.NameIP{}, "", err.Error(), "", nil)
 		return
 	}
 
@@ -188,7 +193,7 @@ func deliverDirect(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale
 			} else {
 				qlog.Infox("mtasts lookup temporary error, aborting delivery attempt", err, slog.Any("domain", origNextHop))
 				recipientDomainResult.Summary.TotalFailureSessionCount++
-				fail(ctx, qlog, m, backoff, false, dsn.NameIP{}, "", err.Error(), nil)
+				fail(ctx, qlog, m, backoff, false, dsn.NameIP{}, "", err.Error(), "", nil)
 				return
 			}
 		}
@@ -205,7 +210,7 @@ func deliverDirect(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale
 	// RFC 5321 does not specify a clear algorithm, but common practice is probably
 	// ../rfc/3974:268.
 	var remoteMTA dsn.NameIP
-	var secodeOpt, errmsg string
+	var firstLine, secodeOpt, errmsg string
 	var moreLines []string // For additional SMTP response lines, included in DSN.
 	permanent = false
 	nmissingRequireTLS := 0
@@ -231,6 +236,7 @@ func deliverDirect(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale
 					metricTLSRequiredNoIgnored.WithLabelValues("mtastsmx").Inc()
 				} else {
 					errmsg = fmt.Sprintf("mx host %s does not match enforced mta-sts policy with hosts %s", h.Domain, strings.Join(policyHosts, ","))
+					firstLine = ""
 					moreLines = nil
 					qlog.Error("mx host does not match mta-sts policy in mode enforce, skipping", slog.Any("host", h.Domain), slog.Any("policyhosts", policyHosts))
 					recipientDomainResult.Summary.TotalFailureSessionCount++
@@ -271,7 +277,7 @@ func deliverDirect(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale
 
 		var badTLS, ok bool
 		var hostResult tlsrpt.Result
-		permanent, tlsDANE, badTLS, secodeOpt, remoteIP, errmsg, moreLines, hostResult, ok = deliverHost(nqlog, resolver, dialer, ourHostname, transportName, h, enforceMTASTS, haveMX, origNextHopAuthentic, origNextHop, expandedNextHopAuthentic, expandedNextHop, &m, tlsMode, tlsPKIX, &recipientDomainResult)
+		permanent, tlsDANE, badTLS, secodeOpt, remoteIP, errmsg, firstLine, moreLines, hostResult, ok = deliverHost(nqlog, resolver, dialer, ourHostname, transportName, h, enforceMTASTS, haveMX, origNextHopAuthentic, origNextHop, expandedNextHopAuthentic, expandedNextHop, &m, tlsMode, tlsPKIX, &recipientDomainResult)
 
 		var zerotype tlsrpt.PolicyType
 		if hostResult.Policy.Type != zerotype {
@@ -298,7 +304,7 @@ func deliverDirect(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale
 				slog.Bool("enforcemtasts", enforceMTASTS),
 				slog.Bool("tlsdane", tlsDANE),
 				slog.Any("requiretls", m.RequireTLS))
-			permanent, _, _, secodeOpt, remoteIP, errmsg, moreLines, _, ok = deliverHost(nqlog, resolver, dialer, ourHostname, transportName, h, enforceMTASTS, haveMX, origNextHopAuthentic, origNextHop, expandedNextHopAuthentic, expandedNextHop, &m, smtpclient.TLSSkip, false, &tlsrpt.Result{})
+			permanent, _, _, secodeOpt, remoteIP, errmsg, firstLine, moreLines, _, ok = deliverHost(nqlog, resolver, dialer, ourHostname, transportName, h, enforceMTASTS, haveMX, origNextHopAuthentic, origNextHop, expandedNextHopAuthentic, expandedNextHop, &m, smtpclient.TLSSkip, false, &tlsrpt.Result{})
 		}
 
 		if ok {
@@ -334,7 +340,7 @@ func deliverDirect(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale
 		permanent = true
 	}
 
-	fail(ctx, qlog, m, backoff, permanent, remoteMTA, secodeOpt, errmsg, moreLines)
+	fail(ctx, qlog, m, backoff, permanent, remoteMTA, secodeOpt, errmsg, firstLine, moreLines)
 	return
 }
 
@@ -354,7 +360,7 @@ func deliverDirect(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale
 // The returned hostResult holds TLSRPT reporting results for the connection
 // attempt. Its policy type can be the zero value, indicating there was no finding
 // (e.g. internal error).
-func deliverHost(log mlog.Log, resolver dns.Resolver, dialer smtpclient.Dialer, ourHostname dns.Domain, transportName string, host dns.IPDomain, enforceMTASTS, haveMX, origNextHopAuthentic bool, origNextHop dns.Domain, expandedNextHopAuthentic bool, expandedNextHop dns.Domain, m *Msg, tlsMode smtpclient.TLSMode, tlsPKIX bool, recipientDomainResult *tlsrpt.Result) (permanent, tlsDANE, badTLS bool, secodeOpt string, remoteIP net.IP, errmsg string, moreLines []string, hostResult tlsrpt.Result, ok bool) {
+func deliverHost(log mlog.Log, resolver dns.Resolver, dialer smtpclient.Dialer, ourHostname dns.Domain, transportName string, host dns.IPDomain, enforceMTASTS, haveMX, origNextHopAuthentic bool, origNextHop dns.Domain, expandedNextHopAuthentic bool, expandedNextHop dns.Domain, m *Msg, tlsMode smtpclient.TLSMode, tlsPKIX bool, recipientDomainResult *tlsrpt.Result) (permanent, tlsDANE, badTLS bool, secodeOpt string, remoteIP net.IP, errmsg, firstLine string, moreLines []string, hostResult tlsrpt.Result, ok bool) {
 	// About attempting delivery to multiple addresses of a host: ../rfc/5321:3898
 
 	tlsRequiredNo := m.RequireTLS != nil && !*m.RequireTLS
@@ -388,7 +394,7 @@ func deliverHost(log mlog.Log, resolver dns.Resolver, dialer smtpclient.Dialer,
 	// Open message to deliver.
 	f, err := os.Open(m.MessagePath())
 	if err != nil {
-		return false, false, false, "", nil, fmt.Sprintf("open message file: %s", err), nil, hostResult, false
+		return false, false, false, "", nil, fmt.Sprintf("open message file: %s", err), "", nil, hostResult, false
 	}
 	msgr := store.FileMsgReader(m.MsgPrefix, f)
 	defer func() {
@@ -519,7 +525,7 @@ func deliverHost(log mlog.Log, resolver dns.Resolver, dialer smtpclient.Dialer,
 		log.Info("verified tls is required, but destination has no usable dane records and no mta-sts policy, canceling delivery attempt to host")
 		metricRequireTLSUnsupported.WithLabelValues("nopolicy").Inc()
 		// Resond with proper enhanced status code. ../rfc/8689:301
-		return false, tlsDANE, false, smtp.SePol7MissingReqTLS, remoteIP, "missing required tls verification mechanism", nil, hostResult, false
+		return false, tlsDANE, false, smtp.SePol7MissingReqTLS, remoteIP, "missing required tls verification mechanism", "", nil, hostResult, false
 	}
 
 	// Dial the remote host given the IPs if no error yet.
@@ -547,7 +553,7 @@ func deliverHost(log mlog.Log, resolver dns.Resolver, dialer smtpclient.Dialer,
 	metricConnection.WithLabelValues(result).Inc()
 	if err != nil {
 		log.Debugx("connecting to remote smtp", err, slog.Any("host", host))
-		return false, tlsDANE, false, "", remoteIP, fmt.Sprintf("dialing smtp server: %v", err), nil, hostResult, false
+		return false, tlsDANE, false, "", remoteIP, fmt.Sprintf("dialing smtp server: %v", err), "", nil, hostResult, false
 	}
 
 	var mailFrom string
@@ -638,7 +644,7 @@ func deliverHost(log mlog.Log, resolver dns.Resolver, dialer smtpclient.Dialer,
 		deliveryResult = "error"
 	}
 	if err == nil {
-		return false, tlsDANE, false, "", remoteIP, "", nil, hostResult, true
+		return false, tlsDANE, false, "", remoteIP, "", "", nil, hostResult, true
 	} else if cerr, ok := err.(smtpclient.Error); ok {
 		// If we are being rejected due to policy reasons on the first
 		// attempt and remote has both IPv4 and IPv6, we'll give it
@@ -654,9 +660,9 @@ func deliverHost(log mlog.Log, resolver dns.Resolver, dialer smtpclient.Dialer,
 			secode = smtp.SePol7MissingReqTLS
 			metricRequireTLSUnsupported.WithLabelValues("norequiretls").Inc()
 		}
-		return permanent, tlsDANE, errors.Is(cerr, smtpclient.ErrTLS), secode, remoteIP, cerr.Error(), cerr.MoreLines, hostResult, false
+		return permanent, tlsDANE, errors.Is(cerr, smtpclient.ErrTLS), secode, remoteIP, cerr.Error(), cerr.Line, cerr.MoreLines, hostResult, false
 	} else {
-		return false, tlsDANE, errors.Is(cerr, smtpclient.ErrTLS), "", remoteIP, err.Error(), nil, hostResult, false
+		return false, tlsDANE, errors.Is(cerr, smtpclient.ErrTLS), "", remoteIP, err.Error(), "", nil, hostResult, false
 	}
 }
 

queue/dsn.go

@@ -30,7 +30,7 @@ var (
 	)
 )
 
-func deliverDSNFailure(ctx context.Context, log mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg string, moreLines []string) {
+func deliverDSNFailure(ctx context.Context, log mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg string, smtpLines []string) {
 	const subject = "mail delivery failed"
 	message := fmt.Sprintf(`
 Delivery has failed permanently for your email to:
@@ -42,12 +42,15 @@ No further deliveries will be attempted.
 Error during the last delivery attempt:
 
 	%s
-`, m.Recipient().XString(m.SMTPUTF8), strings.Join(append([]string{errmsg}, moreLines...), "\n\t"))
+`, m.Recipient().XString(m.SMTPUTF8), errmsg)
+	if len(smtpLines) > 0 {
+		message += "\nFull SMTP response:\n\n\t" + strings.Join(smtpLines, "\n\t") + "\n"
+	}
 
-	deliverDSN(ctx, log, m, remoteMTA, secodeOpt, errmsg, true, nil, subject, message)
+	deliverDSN(ctx, log, m, remoteMTA, secodeOpt, errmsg, smtpLines, true, nil, subject, message)
 }
 
-func deliverDSNDelay(ctx context.Context, log mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg string, moreLines []string, retryUntil time.Time) {
+func deliverDSNDelay(ctx context.Context, log mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg string, smtpLines []string, retryUntil time.Time) {
 	// Should not happen, but doesn't hurt to prevent sending delayed delivery
 	// notifications for DMARC reports. We don't want to waste postmaster attention.
 	if m.IsDMARCReport {
@@ -66,16 +69,19 @@ If these attempts all fail, you will receive a notice.
 Error during the last delivery attempt:
 
 	%s
-`, m.Recipient().XString(false), strings.Join(append([]string{errmsg}, moreLines...), "\n\t"))
+`, m.Recipient().XString(false), errmsg)
+	if len(smtpLines) > 0 {
+		message += "\nFull SMTP response:\n\n\t" + strings.Join(smtpLines, "\n\t") + "\n"
+	}
 
-	deliverDSN(ctx, log, m, remoteMTA, secodeOpt, errmsg, false, &retryUntil, subject, message)
+	deliverDSN(ctx, log, m, remoteMTA, secodeOpt, errmsg, smtpLines, false, &retryUntil, subject, message)
 }
 
 // We only queue DSNs for delivery failures for emails submitted by authenticated
 // users. So we are delivering to local users. ../rfc/5321:1466
 // ../rfc/5321:1494
 // ../rfc/7208:490
-func deliverDSN(ctx context.Context, log mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg string, permanent bool, retryUntil *time.Time, subject, textBody string) {
+func deliverDSN(ctx context.Context, log mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg string, smtpLines []string, permanent bool, retryUntil *time.Time, subject, textBody string) {
 	kind := "delayed delivery"
 	if permanent {
 		kind = "failure"
@@ -115,9 +121,11 @@ func deliverDSN(ctx context.Context, log mlog.Log, m Msg, remoteMTA dsn.NameIP,
 	} else {
 		status += "0.0"
 	}
-	diagCode := errmsg
+
-	if !dsn.HasCode(diagCode) {
+	// ../rfc/3461:1329
-		diagCode = status + " " + errmsg
+	var smtpDiag string
+	if len(smtpLines) > 0 {
+		smtpDiag = "smtp; " + strings.Join(smtpLines, " ")
 	}
 
 	dsnMsg := &dsn.Message{
@@ -138,8 +146,9 @@ func deliverDSN(ctx context.Context, log mlog.Log, m Msg, remoteMTA dsn.NameIP,
 				FinalRecipient:  m.Recipient(),
 				Action:          action,
 				Status:          status,
+				StatusComment:   errmsg,
 				RemoteMTA:       remoteMTA,
-				DiagnosticCode:  diagCode,
+				DiagnosticCode:  smtpDiag,
 				LastAttemptDate: *m.LastAttempt,
 				WillRetryUntil:  retryUntil,
 			},

queue/queue.go

@@ -581,7 +581,7 @@ func deliver(log mlog.Log, resolver dns.Resolver, m Msg) {
 		transport, ok = mox.Conf.Static.Transports[m.Transport]
 		if !ok {
 			var remoteMTA dsn.NameIP // Zero value, will not be included in DSN. ../rfc/3464:1027
-			fail(ctx, qlog, m, backoff, false, remoteMTA, "", fmt.Sprintf("cannot find transport %q", m.Transport), nil)
+			fail(ctx, qlog, m, backoff, false, remoteMTA, "", fmt.Sprintf("cannot find transport %q", m.Transport), "", nil)
 			return
 		}
 		transportName = m.Transport
@@ -692,10 +692,10 @@ func deliver(log mlog.Log, resolver dns.Resolver, m Msg) {
 		if transport.Socks != nil {
 			socksdialer, err := proxy.SOCKS5("tcp", transport.Socks.Address, nil, &net.Dialer{})
 			if err != nil {
-				fail(ctx, qlog, m, backoff, false, dsn.NameIP{}, "", fmt.Sprintf("socks dialer: %v", err), nil)
+				fail(ctx, qlog, m, backoff, false, dsn.NameIP{}, "", fmt.Sprintf("socks dialer: %v", err), "", nil)
 				return
 			} else if d, ok := socksdialer.(smtpclient.Dialer); !ok {
-				fail(ctx, qlog, m, backoff, false, dsn.NameIP{}, "", "socks dialer is not a contextdialer", nil)
+				fail(ctx, qlog, m, backoff, false, dsn.NameIP{}, "", "socks dialer is not a contextdialer", "", nil)
 				return
 			} else {
 				dialer = d

queue/submit.go

@@ -78,7 +78,7 @@ func deliverSubmit(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale
 	requireTLS := m.RequireTLS != nil && *m.RequireTLS
 	if requireTLS && (tlsMode != smtpclient.TLSRequiredStartTLS && tlsMode != smtpclient.TLSImmediate || !tlsPKIX) {
 		errmsg = fmt.Sprintf("transport %s: message requires verified tls but transport does not verify tls", transportName)
-		fail(ctx, qlog, m, backoff, true, dsn.NameIP{}, smtp.SePol7MissingReqTLS, errmsg, nil)
+		fail(ctx, qlog, m, backoff, true, dsn.NameIP{}, smtp.SePol7MissingReqTLS, errmsg, "", nil)
 		return
 	}
 
@@ -115,7 +115,7 @@ func deliverSubmit(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale
 		}
 		qlog.Errorx("dialing for submission", err, slog.String("remote", addr))
 		errmsg = fmt.Sprintf("transport %s: dialing %s for submission: %v", transportName, addr, err)
-		fail(ctx, qlog, m, backoff, false, dsn.NameIP{}, "", errmsg, nil)
+		fail(ctx, qlog, m, backoff, false, dsn.NameIP{}, "", errmsg, "", nil)
 		return
 	}
 	dialcancel()
@@ -171,7 +171,7 @@ func deliverSubmit(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale
 		qlog.Errorx("establishing smtp session for submission", err, slog.String("remote", addr))
 		errmsg = fmt.Sprintf("transport %s: establishing smtp session with %s for submission: %v", transportName, addr, err)
 		secodeOpt = smtperr.Secode
-		fail(ctx, qlog, m, backoff, false, remoteMTA, secodeOpt, errmsg, smtperr.MoreLines)
+		fail(ctx, qlog, m, backoff, false, remoteMTA, secodeOpt, errmsg, smtperr.Line, smtperr.MoreLines)
 		return
 	}
 	defer func() {
@@ -196,7 +196,7 @@ func deliverSubmit(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale
 		if err != nil {
 			qlog.Errorx("opening message for delivery", err, slog.String("remote", addr), slog.String("path", p))
 			errmsg = fmt.Sprintf("transport %s: opening message file for submission: %v", transportName, err)
-			fail(ctx, qlog, m, backoff, false, dsn.NameIP{}, "", errmsg, nil)
+			fail(ctx, qlog, m, backoff, false, dsn.NameIP{}, "", errmsg, "", nil)
 			return
 		}
 		msgr = store.FileMsgReader(m.MsgPrefix, f)
@@ -239,7 +239,7 @@ func deliverSubmit(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale
 		permanent = smtperr.Permanent
 		secodeOpt = smtperr.Secode
 		errmsg = fmt.Sprintf("transport %s: submitting email to %s: %v", transportName, addr, err)
-		fail(ctx, qlog, m, backoff, permanent, remoteMTA, secodeOpt, errmsg, smtperr.MoreLines)
+		fail(ctx, qlog, m, backoff, permanent, remoteMTA, secodeOpt, errmsg, smtperr.Line, smtperr.MoreLines)
 		return
 	}
 	qlog.Info("delivered from queue with transport")

smtpclient/client.go

@@ -161,7 +161,7 @@ type Error struct {
 	// SMTP command causing failure.
 	Command string
 	// For errors due to SMTP responses, the full SMTP line excluding CRLF that caused
-	// the error. Typically the last line read.
+	// the error. First line of a multi-line response.
 	Line string
 	// Optional additional lines in case of multi-line SMTP response.  Most SMTP
 	// responses are single-line, leaving this field empty.