Improve connection timeout handling for SmtpConnectionPool

org.eclipse.scout.rt.mail.test/src/test/java/org/eclipse/scout/rt/mail/smtp/SmtpConnectionPoolTest.java

@@ -1,24 +1,29 @@
 /*
- * Copyright (c) 2010-2023 BSI Business Systems Integration AG.
- * All rights reserved. This program and the accompanying materials
- * are made available under the terms of the Eclipse Public License v1.0
- * which accompanies this distribution, and is available at
- * https://www.eclipse.org/legal/epl-v10.html
+ * Copyright (c) 2010, 2026 BSI Business Systems Integration AG
  *
- * Contributors:
- *     BSI Business Systems Integration AG - initial API and implementation
+ * This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License 2.0
+ * which is available at https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
  */
 package org.eclipse.scout.rt.mail.smtp;
 
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
 
+import java.net.SocketException;
+
+import jakarta.mail.Address;
+import jakarta.mail.MessagingException;
 import jakarta.mail.NoSuchProviderException;
 import jakarta.mail.Session;
 import jakarta.mail.Transport;
+import jakarta.mail.internet.MimeMessage;
 
 import org.eclipse.scout.rt.platform.BEANS;
 import org.eclipse.scout.rt.platform.exception.ProcessingException;
+import org.eclipse.scout.rt.platform.holders.BooleanHolder;
 import org.eclipse.scout.rt.platform.job.IBlockingCondition;
 import org.eclipse.scout.rt.platform.job.IFuture;
 import org.eclipse.scout.rt.platform.job.Jobs;
@@ -30,9 +35,12 @@
 import org.junit.Test;
 import org.mockito.MockMakers;
 
+import com.sun.mail.smtp.SMTPSendFailedException;
+
 public class SmtpConnectionPoolTest {
 
   protected SmtpHelper m_mockSmtpHelper;
+  private Transport m_mockTransport;
 
   @Rule
   public final RegisterBeanTestRule<SmtpHelper> m_smtpHelperBeanTestRule = new RegisterBeanTestRule<SmtpHelper>(SmtpHelper.class, () -> m_mockSmtpHelper = createMockSmtpHelper());
@@ -41,7 +49,9 @@ protected SmtpHelper createMockSmtpHelper() {
     SmtpHelper mock = mock(SmtpHelper.class);
     Session session = mock(Session.class, withSettings().mockMaker(MockMakers.INLINE));
     try {
-      when(session.getTransport()).thenReturn(mock(Transport.class));
+      Transport transport = mock(Transport.class);
+      when(session.getTransport()).thenReturn(transport);
+      m_mockTransport = transport;
     }
     catch (NoSuchProviderException e) {
       throw new ProcessingException("Mocking error", e);
@@ -50,6 +60,53 @@ protected SmtpHelper createMockSmtpHelper() {
     return mock;
   }
 
+  @Test
+  public void testSendMessage() throws MessagingException {
+    SmtpServerConfig config = createDefaultServerConfig();
+    SmtpConnectionPool pool = createDefaultSmtpConnectionPool();
+
+    // successful message
+    pool.sendMessage(config, mock(MimeMessage.class), new Address[]{mock(Address.class)});
+
+    // failed message
+    doThrow(MessagingException.class).when(m_mockTransport).sendMessage(any(), any());
+    assertThrows(MessagingException.class, () -> pool.sendMessage(config, mock(MimeMessage.class), new Address[]{mock(Address.class)}));
+
+    // failed message / connection error
+    BooleanHolder firstFailed = new BooleanHolder(false);
+    BooleanHolder messageSent = new BooleanHolder(false);
+    doAnswer(invocation -> {
+      if (firstFailed.getValue()) {
+        messageSent.setValue(true);
+        return null;
+      }
+      else {
+        firstFailed.setValue(true);
+        throw new MessagingException("Socket error", new SocketException());
+      }
+    }).when(m_mockTransport).sendMessage(any(), any());
+    pool.sendMessage(config, mock(MimeMessage.class), new Address[]{mock(Address.class)});
+    assertTrue(messageSent.getValue());
+  }
+
+  @Test
+  public void testIsConnectionFailure() {
+    SmtpConnectionPool pool = createDefaultSmtpConnectionPool();
+
+    assertFalse(pool.isConnectionFailure(null));
+
+    assertFalse(pool.isConnectionFailure(new MessagingException()));
+    assertFalse(pool.isConnectionFailure(new MessagingException("Foo")));
+    assertFalse(pool.isConnectionFailure(new MessagingException("", new NullPointerException())));
+    assertTrue(pool.isConnectionFailure(new MessagingException("", new SocketException())));
+
+    assertFalse(pool.isConnectionFailure(new SMTPSendFailedException(null, -1, "Foo", null, null, null, null)));
+    assertTrue(pool.isConnectionFailure(new SMTPSendFailedException(null, -1, "[EOF]", null, null, null, null)));
+    assertFalse(pool.isConnectionFailure(new SMTPSendFailedException(null, 400, "Foo", null, null, null, null)));
+    assertTrue(pool.isConnectionFailure(new SMTPSendFailedException(null, 421, "Foo", null, null, null, null)));
+    assertTrue(pool.isConnectionFailure(new SMTPSendFailedException(null, 451, "Foo", null, null, null, null)));
+  }
+
   @Test(timeout = 15000)
   public void testLeaseAndReleaseConnection() {
     SmtpServerConfig config = createDefaultServerConfig();

org.eclipse.scout.rt.mail/src/main/java/org/eclipse/scout/rt/mail/smtp/SmtpConnectionPool.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2023 BSI Business Systems Integration AG
+ * Copyright (c) 2010, 2026 BSI Business Systems Integration AG
  *
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -365,12 +365,13 @@ protected Predicate<SmtpConnectionPoolEntry> getPoolFilter(SmtpServerConfig smtp
   }
 
   protected void startCloseIdleConnectionsJob() {
+    long closeIdleConnectionsJobScheduleInSeconds = CONFIG.getPropertyValue(CloseIdleConnectionsJobScheduleProperty.class);
     Jobs.schedule(this::closeIdleConnections, Jobs.newInput()
         .withName(JOB_NAME_CLOSE_IDLE_CONNECTIONS)
         .withExecutionHint(m_jobExecutionHint)
         .withExecutionTrigger(Jobs.newExecutionTrigger()
-            .withStartIn(1, TimeUnit.MINUTES)
-            .withSchedule(FixedDelayScheduleBuilder.repeatForever(1, TimeUnit.MINUTES))));
+            .withStartIn(closeIdleConnectionsJobScheduleInSeconds, TimeUnit.SECONDS)
+            .withSchedule(FixedDelayScheduleBuilder.repeatForever(closeIdleConnectionsJobScheduleInSeconds, TimeUnit.SECONDS))));
   }
 
   /**
@@ -454,11 +455,39 @@ protected String getNextPoolEntryName() {
   }
 
   protected boolean isConnectionFailure(MessagingException e) {
-    // when trying to send an e-mail using a broken exception there seem to be two variants of exceptions being thrown:
+    if (e == null) {
+      return false;
+    }
+
+    // when trying to send an e-mail using a broken exception there seem to be the following variants of exceptions being thrown:
     // 1. MessagingException with a next SocketException as next exception
-    // 2. SMTPSendFailedException with "[EOF]" as message
-    return e != null && (e.getNextException() instanceof SocketException ||
-                             (e instanceof SMTPSendFailedException && "[EOF]".equals(e.getMessage())));
+    if (e.getNextException() instanceof SocketException) {
+      return true;
+    }
+
+    if (e instanceof SMTPSendFailedException) {
+      SMTPSendFailedException smtpSendFailedException = (SMTPSendFailedException) e;
+
+      // 2. SMTPSendFailedException with "[EOF]" as message
+      if ("[EOF]".equals(e.getMessage())) {
+        return true;
+      }
+
+      // 3. A connection explicitly sending an error code
+      // - 421 indicating a server shutdown (might lead to an exchanged connection which just fails again)
+      // - 451 arbitrary error code which according to RFC should be treated the same way a connection failure would be treated
+      //
+      // See also:
+      // * https://datatracker.ietf.org/doc/html/rfc5321#section-3.8
+      // * https://datatracker.ietf.org/doc/html/rfc5321#section-4.2.3
+      // * https://github.com/eclipse-ee4j/angus-mail/blob/eca8e08baa56a83ab68ae3faaca25d8ca3c0e5ee/providers/smtp/src/main/java/org/eclipse/angus/mail/smtp/SMTPTransport.java#L1453-L1457
+      int returnCode = smtpSendFailedException.getReturnCode();
+      if (returnCode == 421 || returnCode == 451) {
+        return true;
+      }
+    }
+
+    return false;
   }
 
   protected void closeIdleConnections() {
@@ -577,7 +606,7 @@ protected <T> T callWithPoolLock(Callable<T> callable) {
   public static class SmtpPoolMaxIdleTimeProperty extends AbstractPositiveIntegerConfigProperty {
     @Override
     public Integer getDefaultValue() {
-      return 60; // 1 minute
+      return 20; // 20 seconds
     }
 
     @Override
@@ -625,6 +654,23 @@ public String description() {
     }
   }
 
+  public static class CloseIdleConnectionsJobScheduleProperty extends AbstractPositiveIntegerConfigProperty {
+    @Override
+    public Integer getDefaultValue() {
+      return 20; // 20 seconds
+    }
+
+    @Override
+    public String getKey() {
+      return "scout.smtp.pool.closeIdleConnectionsJobSchedule";
+    }
+
+    @Override
+    public String description() {
+      return "Repetition schedule for the close idle connections job in seconds (run if at least one connection is currently open). Must be a positive number. If set to a greater number than the maximum idle timeout, then connections may exceed their maximum idle timeout. Default value: " + getDefaultValue() + ".";
+    }
+  }
+
   public static class PlatformListener implements IPlatformListener {
     @Override
     public void stateChanged(final PlatformEvent event) {