mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-05-11 04:43:59 +00:00
fix: addresses #299 - email notifications not working
Fixed two critical issues with email notifications: 1. Test email API now returns errors properly instead of always showing success 2. Added timeouts to SMTP connections to prevent hanging (10s dial, 30s overall) The root cause of users not receiving emails was that errors were being silently logged instead of returned to the API, making it appear successful when it wasn't. SMTP connections could also hang indefinitely on unreachable servers. Note: API uses "server" and "port" JSON fields, not "smtpHost"/"smtpPort"
This commit is contained in:
parent
768d60739b
commit
7dbcb5461c
2 changed files with 130 additions and 6 deletions
|
|
@ -176,7 +176,8 @@ func (e *EnhancedEmailManager) sendViaProvider(msg []byte) error {
|
|||
} else if e.config.StartTLS {
|
||||
return e.sendStartTLS(addr, auth, msg)
|
||||
} else {
|
||||
return smtp.SendMail(addr, auth, e.config.From, e.config.To, msg)
|
||||
// Use sendPlain for non-TLS connections with timeout
|
||||
return e.sendPlain(addr, auth, msg)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -187,12 +188,19 @@ func (e *EnhancedEmailManager) sendTLS(addr string, auth smtp.Auth, msg []byte)
|
|||
InsecureSkipVerify: e.config.SkipTLSVerify,
|
||||
}
|
||||
|
||||
conn, err := tls.Dial("tcp", addr, tlsConfig)
|
||||
// Use DialWithDialer with timeout
|
||||
dialer := &net.Dialer{
|
||||
Timeout: 10 * time.Second,
|
||||
}
|
||||
conn, err := tls.DialWithDialer(dialer, "tcp", addr, tlsConfig)
|
||||
if err != nil {
|
||||
return fmt.Errorf("TLS dial failed: %w", err)
|
||||
}
|
||||
defer conn.Close()
|
||||
|
||||
// Set overall connection timeout
|
||||
conn.SetDeadline(time.Now().Add(30 * time.Second))
|
||||
|
||||
client, err := smtp.NewClient(conn, e.config.SMTPHost)
|
||||
if err != nil {
|
||||
return fmt.Errorf("SMTP client creation failed: %w", err)
|
||||
|
|
@ -235,12 +243,16 @@ func (e *EnhancedEmailManager) sendTLS(addr string, auth smtp.Auth, msg []byte)
|
|||
|
||||
// sendStartTLS sends email using STARTTLS
|
||||
func (e *EnhancedEmailManager) sendStartTLS(addr string, auth smtp.Auth, msg []byte) error {
|
||||
conn, err := net.Dial("tcp", addr)
|
||||
// Use DialTimeout to prevent hanging on unreachable servers
|
||||
conn, err := net.DialTimeout("tcp", addr, 10*time.Second)
|
||||
if err != nil {
|
||||
return fmt.Errorf("TCP dial failed: %w", err)
|
||||
}
|
||||
defer conn.Close()
|
||||
|
||||
// Set overall connection timeout
|
||||
conn.SetDeadline(time.Now().Add(30 * time.Second))
|
||||
|
||||
client, err := smtp.NewClient(conn, e.config.SMTPHost)
|
||||
if err != nil {
|
||||
return fmt.Errorf("SMTP client creation failed: %w", err)
|
||||
|
|
@ -339,5 +351,57 @@ func (e *EnhancedEmailManager) TestConnection() error {
|
|||
}
|
||||
}
|
||||
|
||||
return client.Quit()
|
||||
}
|
||||
|
||||
// sendPlain sends email over plain SMTP connection with timeout
|
||||
func (e *EnhancedEmailManager) sendPlain(addr string, auth smtp.Auth, msg []byte) error {
|
||||
// Use DialTimeout to prevent hanging on unreachable servers
|
||||
conn, err := net.DialTimeout("tcp", addr, 10*time.Second)
|
||||
if err != nil {
|
||||
return fmt.Errorf("TCP dial failed: %w", err)
|
||||
}
|
||||
defer conn.Close()
|
||||
|
||||
// Set overall connection timeout
|
||||
conn.SetDeadline(time.Now().Add(30 * time.Second))
|
||||
|
||||
client, err := smtp.NewClient(conn, e.config.SMTPHost)
|
||||
if err != nil {
|
||||
return fmt.Errorf("SMTP client creation failed: %w", err)
|
||||
}
|
||||
defer client.Close()
|
||||
|
||||
if auth != nil {
|
||||
if err = client.Auth(auth); err != nil {
|
||||
return fmt.Errorf("SMTP auth failed: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
if err = client.Mail(e.config.From); err != nil {
|
||||
return fmt.Errorf("MAIL FROM failed: %w", err)
|
||||
}
|
||||
|
||||
for _, to := range e.config.To {
|
||||
if err = client.Rcpt(to); err != nil {
|
||||
return fmt.Errorf("RCPT TO failed for %s: %w", to, err)
|
||||
}
|
||||
}
|
||||
|
||||
w, err := client.Data()
|
||||
if err != nil {
|
||||
return fmt.Errorf("DATA command failed: %w", err)
|
||||
}
|
||||
|
||||
_, err = w.Write(msg)
|
||||
if err != nil {
|
||||
return fmt.Errorf("message write failed: %w", err)
|
||||
}
|
||||
|
||||
err = w.Close()
|
||||
if err != nil {
|
||||
return fmt.Errorf("message close failed: %w", err)
|
||||
}
|
||||
|
||||
return client.Quit()
|
||||
}
|
||||
|
|
@ -279,6 +279,64 @@ func (n *NotificationManager) sendEmail(alert *alerts.Alert) {
|
|||
n.sendHTMLEmail(subject, htmlBody, textBody, config)
|
||||
}
|
||||
|
||||
// sendHTMLEmailWithError sends an HTML email with multipart content and returns any error
|
||||
func (n *NotificationManager) sendHTMLEmailWithError(subject, htmlBody, textBody string, config EmailConfig) error {
|
||||
// Use From address as recipient if To is empty
|
||||
recipients := config.To
|
||||
if len(recipients) == 0 && config.From != "" {
|
||||
recipients = []string{config.From}
|
||||
log.Info().
|
||||
Str("from", config.From).
|
||||
Msg("Using From address as recipient since To is empty")
|
||||
}
|
||||
|
||||
// Create enhanced email configuration with proper STARTTLS support
|
||||
enhancedConfig := EmailProviderConfig{
|
||||
EmailConfig: EmailConfig{
|
||||
From: config.From,
|
||||
To: recipients,
|
||||
SMTPHost: config.SMTPHost,
|
||||
SMTPPort: config.SMTPPort,
|
||||
Username: config.Username,
|
||||
Password: config.Password,
|
||||
},
|
||||
StartTLS: config.StartTLS, // Use the configured StartTLS setting
|
||||
MaxRetries: 2,
|
||||
RetryDelay: 3,
|
||||
RateLimit: 60,
|
||||
SkipTLSVerify: false,
|
||||
AuthRequired: config.Username != "" && config.Password != "",
|
||||
}
|
||||
|
||||
// Use enhanced email manager for better compatibility
|
||||
enhancedManager := NewEnhancedEmailManager(enhancedConfig)
|
||||
|
||||
log.Info().
|
||||
Str("smtp", fmt.Sprintf("%s:%d", config.SMTPHost, config.SMTPPort)).
|
||||
Str("from", config.From).
|
||||
Strs("to", recipients).
|
||||
Bool("hasAuth", config.Username != "" && config.Password != "").
|
||||
Bool("startTLS", enhancedConfig.StartTLS).
|
||||
Msg("Attempting to send email via SMTP with enhanced support")
|
||||
|
||||
err := enhancedManager.SendEmailWithRetry(subject, htmlBody, textBody)
|
||||
|
||||
if err != nil {
|
||||
log.Error().
|
||||
Err(err).
|
||||
Str("smtp", fmt.Sprintf("%s:%d", config.SMTPHost, config.SMTPPort)).
|
||||
Strs("recipients", recipients).
|
||||
Msg("Failed to send email notification")
|
||||
return fmt.Errorf("failed to send email: %w", err)
|
||||
}
|
||||
|
||||
log.Info().
|
||||
Strs("recipients", recipients).
|
||||
Int("recipientCount", len(recipients)).
|
||||
Msg("Email notification sent successfully")
|
||||
return nil
|
||||
}
|
||||
|
||||
// sendHTMLEmail sends an HTML email with multipart content
|
||||
func (n *NotificationManager) sendHTMLEmail(subject, htmlBody, textBody string, config EmailConfig) {
|
||||
// Use From address as recipient if To is empty
|
||||
|
|
@ -809,6 +867,9 @@ func (n *NotificationManager) SendTestNotificationWithConfig(method string, conf
|
|||
Int("port", config.SMTPPort).
|
||||
Str("from", config.From).
|
||||
Int("toCount", len(config.To)).
|
||||
Strs("to", config.To).
|
||||
Bool("smtpEmpty", config.SMTPHost == "").
|
||||
Bool("fromEmpty", config.From == "").
|
||||
Msg("Testing email notification with provided config")
|
||||
|
||||
if !config.Enabled {
|
||||
|
|
@ -822,9 +883,8 @@ func (n *NotificationManager) SendTestNotificationWithConfig(method string, conf
|
|||
// Generate email using template
|
||||
subject, htmlBody, textBody := EmailTemplate([]*alerts.Alert{testAlert}, true)
|
||||
|
||||
// Send using provided config
|
||||
n.sendHTMLEmail(subject, htmlBody, textBody, *config)
|
||||
return nil
|
||||
// Send using provided config and return any error
|
||||
return n.sendHTMLEmailWithError(subject, htmlBody, textBody, *config)
|
||||
|
||||
default:
|
||||
return fmt.Errorf("unsupported method for config-based testing: %s", method)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue