mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
Merge candidate 20260906T210504Z-core-runtime
Change-source: pulse-maintainer
This commit is contained in:
@@ -10670,3 +10670,14 @@ reasoning and real remediation in
|
||||
`docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md`. The repeatable browser
|
||||
proof is `scripts/check-patrol-assistant-journey.mjs`. A passing scripted
|
||||
response does not establish a useful customer outcome or model qualification.
|
||||
|
||||
### Report email shares the canonical SMTP retry verdict
|
||||
|
||||
Attachment delivery through `internal/notifications/email_enhanced.go` uses
|
||||
notifications-owned `sendEmailWithOptions`; reporting must not introduce a
|
||||
separate retry classification. That transport stops on authentication,
|
||||
configuration, or rejection failures and preserves temporary-failure retries.
|
||||
Its returned failure retains the structured cause and actual attempt count;
|
||||
this does not change report API schemas or prove recipient acceptance.
|
||||
`internal/notifications/email_retry_class_test.go` verifies the shared transport
|
||||
failure boundary with in-memory SMTP replies, not end-to-end report delivery.
|
||||
|
||||
@@ -581,6 +581,21 @@ the queue nor any destination type may keep its own list. This generalises to
|
||||
every destination the decision webhook delivery already made for HTTP 4xx in
|
||||
`isRetryableWebhookError`.
|
||||
|
||||
The SMTP transport's inner retry loop must apply that same classifier after
|
||||
an unsuccessful send, before sleeping or attempting another connection.
|
||||
Permanent authentication, configuration, and rejection failures return on that
|
||||
attempt; transient failures retain up to `MaxRetries + 1` transport attempts.
|
||||
The returned error preserves the structured cause and reports the actual
|
||||
attempt count, not the configured maximum. This applies equally to ordinary,
|
||||
threaded, and attachment email through `sendEmailWithOptions`.
|
||||
|
||||
`internal/notifications/email_retry_class_test.go` exercises the real sender
|
||||
with in-memory SMTP handshake failures: permanent 550/535/554/501 replies stop
|
||||
at one attempt, while transient 421 retains three configured attempts even
|
||||
when its prose mentions authentication. It checks classification and the
|
||||
reported attempt count. This is transport-only proof, not queue persistence,
|
||||
post-DATA acceptance, or an installed recipient receipt.
|
||||
|
||||
Dead-lettering early must not lose the notification: `RetryTerminalFailures`
|
||||
remains the operator's recovery path, returning eligible retained terminal
|
||||
failures to the queue with a fresh budget once the credentials or configuration
|
||||
|
||||
@@ -474,10 +474,10 @@ func NewEnhancedEmailManager(config EmailProviderConfig) *EnhancedEmailManager {
|
||||
|
||||
// SendEmailWithRetry sends email with retry logic
|
||||
// Note: When used with the persistent queue, retry behavior is layered:
|
||||
// - Transport retries (this function): up to MaxRetries attempts with RetryDelay between
|
||||
// - Transport retries (this function): up to MaxRetries+1 attempts with RetryDelay between
|
||||
// - Queue retries: up to MaxAttempts (default 3) with exponential backoff
|
||||
// Total attempts = MaxRetries * MaxAttempts (e.g., 3 * 3 = 9 SMTP calls for a single notification)
|
||||
// This ensures delivery even during transient failures at either layer.
|
||||
// Total attempts = (MaxRetries+1) * MaxAttempts for transient failures.
|
||||
// Deterministic failures stop at the first attempt in both layers.
|
||||
func (e *EnhancedEmailManager) SendEmailWithRetry(subject, htmlBody, textBody string) error {
|
||||
return e.sendEmailWithOptions(subject, htmlBody, textBody, nil, "")
|
||||
}
|
||||
@@ -528,6 +528,10 @@ func (e *EnhancedEmailManager) sendEmailWithOptions(subject, htmlBody, textBody
|
||||
Int("attempt", attempt).
|
||||
Str("provider", e.config.Provider).
|
||||
Msg("email send attempt failed")
|
||||
|
||||
if !ClassifyNotificationFailureError(err).Retryable() {
|
||||
return fmt.Errorf("email failed after %d attempts: %w", attempt+1, err)
|
||||
}
|
||||
}
|
||||
|
||||
return fmt.Errorf("email failed after %d attempts: %w", e.config.MaxRetries+1, lastErr)
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
package notifications
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// Exercise the SMTP sender, not just the classifier: its inner retry loop must
|
||||
// respect structured replies before the outer queue ever sees the error.
|
||||
func TestEmailRetryRespectsSMTPFailureClass(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
code int
|
||||
message string
|
||||
wantClass NotificationFailureClass
|
||||
wantAttempts int
|
||||
}{
|
||||
{"rejected despite temporary prose", 550, "temporary timeout", NotificationFailureRejected, 1},
|
||||
{"authentication", 535, "credentials invalid", NotificationFailureAuthentication, 1},
|
||||
{"transaction rejection", 554, "transaction rejected", NotificationFailureRejected, 1},
|
||||
{"configuration", 501, "invalid parameters", NotificationFailureConfiguration, 1},
|
||||
{"transient despite authentication prose", 421, "authentication service unavailable", NotificationFailureServerError, 3},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
var servers sync.WaitGroup
|
||||
originalDial := smtpDialTimeout
|
||||
t.Cleanup(func() { smtpDialTimeout = originalDial; servers.Wait() })
|
||||
attempts := 0
|
||||
smtpDialTimeout = func(string, string, time.Duration) (net.Conn, error) {
|
||||
attempts++
|
||||
client, server := net.Pipe()
|
||||
servers.Add(1)
|
||||
go func() {
|
||||
defer servers.Done()
|
||||
defer server.Close()
|
||||
_ = server.SetDeadline(time.Now().Add(5 * time.Second))
|
||||
_, _ = fmt.Fprintf(server, "%d %s\r\n", tc.code, tc.message)
|
||||
}()
|
||||
return client, nil
|
||||
}
|
||||
manager := NewEnhancedEmailManager(EmailProviderConfig{
|
||||
EmailConfig: EmailConfig{SMTPHost: "smtp.example.test", SMTPPort: 25, From: "pulse@example.test", To: []string{"recipient@example.test"}},
|
||||
MaxRetries: 2,
|
||||
})
|
||||
err := manager.SendEmailWithRetry("test", "<p>test</p>", "test")
|
||||
if err == nil {
|
||||
t.Fatal("expected SMTP failure")
|
||||
}
|
||||
if got := ClassifyNotificationFailureError(err); got != tc.wantClass {
|
||||
t.Errorf("class = %q, want %q", got, tc.wantClass)
|
||||
}
|
||||
if !strings.Contains(err.Error(), fmt.Sprintf("email failed after %d attempts:", tc.wantAttempts)) {
|
||||
t.Errorf("incorrect attempt count in error: %v", err)
|
||||
}
|
||||
if attempts != tc.wantAttempts {
|
||||
t.Errorf("SMTP attempts = %d, want %d", attempts, tc.wantAttempts)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user