diff --git a/src/main/java/hudson/plugins/emailext/ExtendedEmailPublisher.java b/src/main/java/hudson/plugins/emailext/ExtendedEmailPublisher.java index 193971f7..90d84358 100644 --- a/src/main/java/hudson/plugins/emailext/ExtendedEmailPublisher.java +++ b/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 { diff --git a/src/test/java/hudson/plugins/emailext/SmtpRetryTest.java b/src/test/java/hudson/plugins/emailext/SmtpRetryTest.java new file mode 100644 index 00000000..dad9c6f5 --- /dev/null +++ b/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"); + } +}