[JENKINS-68518] Email-ext treats transient SMTP errors as fatal errors

src/main/java/hudson/plugins/emailext/ExtendedEmailPublisher.java

@@ -61,6 +61,8 @@
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.io.UnsupportedEncodingException;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.net.ConnectException;
 import java.net.MalformedURLException;
 import java.net.SocketException;
@@ -75,6 +77,8 @@
 import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import jenkins.model.Jenkins;
 import org.apache.commons.lang3.StringUtils;
 import org.codehaus.groovy.control.CompilerConfiguration;
@@ -527,6 +531,90 @@ private boolean _perform(
         return true;
     }
 
+    /**
+     * Checks if an exception represents a transient SMTP error (4xx code) that should be retried.
+     *
+     * @param e the exception to check
+     * @return true if the exception represents a transient SMTP error (4xx code), false otherwise
+     */
+    boolean isTransientSmtpError(Exception e) {
+        if (e == null) {
+            return false;
+        }
+
+        Integer returnCode = getSmtpReturnCode(e);
+        if (returnCode != null && returnCode >= 400 && returnCode < 500) {
+            return true;
+        }
+
+        Integer parsedCode = parseSmtpErrorCode(e);
+        if (parsedCode != null && parsedCode >= 400 && parsedCode < 500) {
+            return true;
+        }
+
+        if (e instanceof MessagingException) {
+            Exception nextException = ((MessagingException) e).getNextException();
+            if (nextException != null && isTransientSmtpError(nextException)) {
+                return true;
+            }
+        }
+
+        Throwable cause = e.getCause();
+        if (cause instanceof Exception) {
+            return isTransientSmtpError((Exception) cause);
+        }
+
+        return false;
+    }
+
+    /**
+     * Tries to get SMTP return code from an exception using reflection.
+     * This works with SMTPAddressFailedException and SMTPSendFailedException
+     * without requiring a compile-time dependency.
+     *
+     * @param e the exception
+     * @return the SMTP return code, or null if not available
+     */
+    private Integer getSmtpReturnCode(Exception e) {
+        if (e == null) {
+            return null;
+        }
+        try {
+            Method method = e.getClass().getMethod("getReturnCode");
+            Object result = method.invoke(e);
+            if (result instanceof Integer) {
+                return (Integer) result;
+            }
+        } catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException ignored) {
+            // Method doesn't exist or couldn't be invoked
+        }
+        return null;
+    }
+
+    /**
+     * Parses SMTP error code from exception message.
+     * Looks for patterns like "421 Service not available" in the message.
+     *
+     * @param e the exception
+     * @return the SMTP error code, or null if not found
+     */
+    private Integer parseSmtpErrorCode(Exception e) {
+        if (e == null || e.getMessage() == null) {
+            return null;
+        }
+        String message = e.getMessage();
+        Pattern pattern = Pattern.compile("^(\\d{3})\\s");
+        Matcher matcher = pattern.matcher(message);
+        if (matcher.find()) {
+            try {
+                return Integer.parseInt(matcher.group(1));
+            } catch (NumberFormatException ignored) {
+                // Not a valid number
+            }
+        }
+        return null;
+    }
+
     @SuppressFBWarnings("REC_CATCH_EXCEPTION")
     boolean sendMail(ExtendedEmailPublisherContext context) {
         try {
@@ -612,12 +700,15 @@ boolean sendMail(ExtendedEmailPublisherContext context) {
                             }
                             break;
                         } catch (SendFailedException e) {
-                            if (e.getNextException() != null
-                                    && (e.getNextException() instanceof SocketException
-                                            || e.getNextException() instanceof ConnectException)) {
+                            boolean isTransient = isTransientSmtpError(e);
+                            if ((e.getNextException() != null
+                                            && (e.getNextException() instanceof SocketException
+                                                    || e.getNextException() instanceof ConnectException))
+                                    || isTransient) {
+                                String reason = isTransient ? "Transient SMTP error" : "Socket error";
                                 context.getListener()
                                         .getLogger()
-                                        .println("Socket error sending email, retrying once more in 10 seconds...");
+                                        .println(reason + " sending email, retrying once more in 10 seconds...");
                                 transport.close();
                                 Thread.sleep(10000);
                             } else {
@@ -664,11 +755,13 @@ boolean sendMail(ExtendedEmailPublisherContext context) {
                                 break;
                             }
                         } catch (MessagingException e) {
-                            if (e.getNextException() != null && e.getNextException() instanceof ConnectException) {
+                            boolean isTransient = isTransientSmtpError(e);
+                            if ((e.getNextException() != null && e.getNextException() instanceof ConnectException)
+                                    || isTransient) {
+                                String reason = isTransient ? "Transient SMTP error" : "Connection error";
                                 context.getListener()
                                         .getLogger()
-                                        .println(
-                                                "SMTP connection error while sending email. Retrying once more in 10 seconds.");
+                                        .println(reason + " sending email, retrying once more in 10 seconds...");
                                 transport.close();
                                 Thread.sleep(10000);
                             } else {

src/test/java/hudson/plugins/emailext/SmtpRetryTest.java

@@ -0,0 +1,126 @@
+package hudson.plugins.emailext;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import jakarta.mail.MessagingException;
+import jakarta.mail.SendFailedException;
+import java.io.IOException;
+import java.lang.reflect.Method;
+import java.net.SocketException;
+import org.junit.jupiter.api.Test;
+import org.jvnet.hudson.test.Issue;
+
+/**
+ * Unit tests for SMTP retry logic, particularly for transient SMTP errors (4xx codes).
+ * Since SMTP-specific exception classes are not available at compile time, these tests
+ * focus on the message parsing and exception chain traversal logic.
+ *
+ * @author Akash Manna
+ */
+class SmtpRetryTest {
+
+    /**
+     * Calls the package-private isTransientSmtpError() method using reflection.
+     */
+    private boolean callIsTransientSmtpError(Exception e) throws Exception {
+        ExtendedEmailPublisher publisher = new ExtendedEmailPublisher();
+        Method method = ExtendedEmailPublisher.class.getDeclaredMethod("isTransientSmtpError", Exception.class);
+        method.setAccessible(true);
+        return (Boolean) method.invoke(publisher, e);
+    }
+
+    @Test
+    @Issue("JENKINS-68518")
+    void testIsTransientSmtpErrorWithMessageParsing() throws Exception {
+        MessagingException ex421 = new MessagingException("421 Service not available, try again later");
+        MessagingException ex450 = new MessagingException("450 Requested mail action not taken: mailbox unavailable");
+        MessagingException ex451 = new MessagingException("451 Requested action aborted: error in processing");
+        MessagingException ex452 = new MessagingException("452 Insufficient system storage");
+
+        assertTrue(
+                callIsTransientSmtpError(ex421),
+                "MessagingException with '421' in message should be identified as transient");
+        assertTrue(
+                callIsTransientSmtpError(ex450),
+                "MessagingException with '450' in message should be identified as transient");
+        assertTrue(
+                callIsTransientSmtpError(ex451),
+                "MessagingException with '451' in message should be identified as transient");
+        assertTrue(
+                callIsTransientSmtpError(ex452),
+                "MessagingException with '452' in message should be identified as transient");
+
+        MessagingException ex500 = new MessagingException("500 Syntax error");
+        MessagingException ex550 = new MessagingException("550 Requested action not taken");
+
+        assertFalse(
+                callIsTransientSmtpError(ex500),
+                "MessagingException with '500' in message should NOT be identified as transient");
+        assertFalse(
+                callIsTransientSmtpError(ex550),
+                "MessagingException with '550' in message should NOT be identified as transient");
+    }
+
+    @Test
+    @Issue("JENKINS-68518")
+    void testIsTransientSmtpErrorWithNonSmtpExceptions() throws Exception {
+        assertFalse(
+                callIsTransientSmtpError(new IOException("Connection error")),
+                "IOException should NOT be identified as transient SMTP error");
+        assertFalse(
+                callIsTransientSmtpError(new MessagingException("Generic messaging error")),
+                "MessagingException without SMTP code should NOT be identified as transient");
+        assertFalse(
+                callIsTransientSmtpError(new SocketException("Connection reset")),
+                "SocketException should NOT be identified as transient SMTP error");
+    }
+
+    @Test
+    @Issue("JENKINS-68518")
+    void testIsTransientSmtpErrorInCauseChain() throws Exception {
+        MessagingException smtpError = new MessagingException("421 Service not available");
+        SendFailedException wrapped = new SendFailedException("Send failed", smtpError);
+        MessagingException doubleWrapped = new MessagingException("Multiple errors", wrapped);
+
+        assertTrue(
+                callIsTransientSmtpError(wrapped),
+                "SMTP 421 error wrapped in SendFailedException should be identified as transient");
+        assertTrue(
+                callIsTransientSmtpError(doubleWrapped),
+                "SMTP 421 error deep in exception chain should be identified as transient");
+    }
+
+    @Test
+    @Issue("JENKINS-68518")
+    void testIsTransientSmtpErrorBoundaryCases() throws Exception {
+        MessagingException ex399 = new MessagingException("399 Below 4xx range");
+        MessagingException ex400 = new MessagingException("400 First 4xx code");
+        MessagingException ex499 = new MessagingException("499 Last 4xx code");
+        MessagingException ex500 = new MessagingException("500 First 5xx code");
+
+        assertFalse(callIsTransientSmtpError(ex399), "SMTP 399 should NOT be identified as transient");
+        assertTrue(callIsTransientSmtpError(ex400), "SMTP 400 should be identified as transient");
+        assertTrue(callIsTransientSmtpError(ex499), "SMTP 499 should be identified as transient");
+        assertFalse(callIsTransientSmtpError(ex500), "SMTP 500 should NOT be identified as transient");
+    }
+
+    @Test
+    @Issue("JENKINS-68518")
+    void testIsTransientSmtpErrorWithNextException() throws Exception {
+        MessagingException inner = new MessagingException("450 Mailbox unavailable");
+        MessagingException outer = new MessagingException("Failed to send");
+        outer.setNextException(inner);
+
+        assertTrue(callIsTransientSmtpError(outer), "Should detect transient error in nextException chain");
+    }
+
+    @Test
+    @Issue("JENKINS-68518")
+    void testIsTransientSmtpErrorWithNullCheck() throws Exception {
+        ExtendedEmailPublisher publisher = new ExtendedEmailPublisher();
+        Method method = ExtendedEmailPublisher.class.getDeclaredMethod("isTransientSmtpError", Exception.class);
+        method.setAccessible(true);
+
+        assertFalse((Boolean) method.invoke(publisher, new Object[] {null}), "Should return false for null exception");
+    }
+}