for messages retired from the delivery queue, set "success" field properly, and include the smtp code/enhanced code on success too (not only on failure)

noticed some time ago when looking at my retired messages queue.
This commit is contained in:
Mechiel Lukkien 2024-10-05 11:06:42 +02:00
parent fb65ec0676
commit fdc0560ac4
No known key found for this signature in database
4 changed files with 11 additions and 5 deletions

View file

@ -303,7 +303,7 @@ func deliverDirect(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale
for i, mr := range result.delivered { for i, mr := range result.delivered {
mqlog := nqlog.With(slog.Int64("msgid", mr.msg.ID), slog.Any("recipient", mr.msg.Recipient())) mqlog := nqlog.With(slog.Int64("msgid", mr.msg.ID), slog.Any("recipient", mr.msg.Recipient()))
mqlog.Info("delivered from queue") mqlog.Info("delivered from queue")
mr.msg.markResult(0, "", "", true) mr.msg.markResult(mr.resp.Code, mr.resp.Secode, "", true)
delMsgs[i] = *mr.msg delMsgs[i] = *mr.msg
} }
if len(delMsgs) > 0 { if len(delMsgs) > 0 {
@ -745,6 +745,7 @@ func deliverHost(log mlog.Log, resolver dns.Resolver, dialer smtpclient.Dialer,
} else if i > 0 && (resps[i].Code == smtp.C452StorageFull || resps[i].Code == smtp.C552MailboxFull) { } else if i > 0 && (resps[i].Code == smtp.C452StorageFull || resps[i].Code == smtp.C552MailboxFull) {
ntodo = append(ntodo, mr) ntodo = append(ntodo, mr)
} else if resps[i].Code == smtp.C250Completed { } else if resps[i].Code == smtp.C250Completed {
mr.resp = resps[i]
delivered = append(delivered, mr) delivered = append(delivered, mr)
} else { } else {
failed = append(failed, mr) failed = append(failed, mr)

View file

@ -206,7 +206,7 @@ func (m *Msg) markResult(code int, secode string, errmsg string, success bool) {
result.Code = code result.Code = code
result.Secode = secode result.Secode = secode
result.Error = errmsg result.Error = errmsg
result.Success = false result.Success = success
} }
// LastResult returns the last result entry, or an empty result. // LastResult returns the last result entry, or an empty result.

View file

@ -1027,6 +1027,10 @@ func TestRetiredHooks(t *testing.T) {
if expResult.Secode != "" { if expResult.Secode != "" {
ecode = fmt.Sprintf("%d.%s", expResult.Code/100, expResult.Secode) ecode = fmt.Sprintf("%d.%s", expResult.Code/100, expResult.Secode)
} }
var code int // Only set for errors.
if expResult.Code != 250 {
code = expResult.Code
}
expOut := webhook.Outgoing{ expOut := webhook.Outgoing{
Event: webhook.OutgoingEvent(expEvent), Event: webhook.OutgoingEvent(expEvent),
Suppressing: expSuppressing, Suppressing: expSuppressing,
@ -1034,7 +1038,7 @@ func TestRetiredHooks(t *testing.T) {
FromID: mr.FromID, FromID: mr.FromID,
MessageID: mr.MessageID, MessageID: mr.MessageID,
Subject: mr.Subject, Subject: mr.Subject,
SMTPCode: expResult.Code, SMTPCode: code,
SMTPEnhancedCode: ecode, SMTPEnhancedCode: ecode,
Extra: mr.Extra, Extra: mr.Extra,
} }
@ -1130,7 +1134,7 @@ func TestRetiredHooks(t *testing.T) {
} }
testAction("mjl", makeLaunchAction(smtpAccept), nil, "", false) testAction("mjl", makeLaunchAction(smtpAccept), nil, "", false)
testAction("retired", makeLaunchAction(smtpAccept), &MsgResult{}, string(webhook.EventDelivered), false) testAction("retired", makeLaunchAction(smtpAccept), &MsgResult{Code: 250, Success: true}, string(webhook.EventDelivered), false)
// 554 is generic, doesn't immediately cause suppression. // 554 is generic, doesn't immediately cause suppression.
testAction("mjl", makeLaunchAction(smtpReject(554)), nil, "", false) testAction("mjl", makeLaunchAction(smtpReject(554)), nil, "", false)
testAction("retired", makeLaunchAction(smtpReject(554)), &MsgResult{Code: 554, Secode: "1.0", Error: "nonempty"}, string(webhook.EventFailed), false) testAction("retired", makeLaunchAction(smtpReject(554)), &MsgResult{Code: 554, Secode: "1.0", Error: "nonempty"}, string(webhook.EventFailed), false)

View file

@ -264,7 +264,8 @@ func processDeliveries(qlog mlog.Log, m0 *Msg, msgs []*Msg, remoteAddr string, r
failMsgsDB(qmlog, []*Msg{m}, m0.DialedIPs, backoff, remoteMTA, err) failMsgsDB(qmlog, []*Msg{m}, m0.DialedIPs, backoff, remoteMTA, err)
failed++ failed++
} else { } else {
m.markResult(0, "", "", true) resp := rcptErrs[i]
m.markResult(resp.Code, resp.Secode, "", true)
delMsgs = append(delMsgs, *m) delMsgs = append(delMsgs, *m)
qmlog.Info("delivered from queue with transport") qmlog.Info("delivered from queue with transport")
delivered++ delivered++