SmtpEmailSender: Refactor open method and improve logging

- Log the reply code and reply string when the connection is
  rejected by the SMTP server.

- Reuse the reply string in subsequent error handling clauses.

- Combine handling of IOException and EmailException into a single
  catch block to reduce duplicated code for disconnecting the client.

Change-Id: Icf36e8bf10e2273ff6678734f9f0759a52505f24

1 files changed, 15 insertions, 19 deletions

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java
index 2f8f75d2f5..ee21316bda 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java
@@ -248,16 +248,19 @@ public class SmtpEmailSender implements EmailSender {
       client.enableSSL(sslVerify);
     }
 
+    client.setConnectTimeout(connectTimeout);
     try {
-      client.setConnectTimeout(connectTimeout);
       client.connect(smtpHost, smtpPort);
-      if (!SMTPReply.isPositiveCompletion(client.getReplyCode())) {
-        throw new EmailException("SMTP server rejected connection");
+      int replyCode = client.getReplyCode();
+      String replyString = client.getReplyString();
+      if (!SMTPReply.isPositiveCompletion(replyCode)) {
+        throw new EmailException(
+            String.format("SMTP server rejected connection: %d: %s",
+                replyCode, replyString));
       }
       if (!client.login()) {
-        String e = client.getReplyString();
         throw new EmailException(
-            "SMTP server rejected HELO/EHLO greeting: " + e);
+            "SMTP server rejected HELO/EHLO greeting: " + replyString);
       }
 
       if (smtpEncryption == Encryption.TLS) {
@@ -265,32 +268,25 @@ public class SmtpEmailSender implements EmailSender {
           throw new EmailException("SMTP server does not support TLS");
         }
         if (!client.login()) {
-          String e = client.getReplyString();
-          throw new EmailException("SMTP server rejected login: " + e);
+          throw new EmailException("SMTP server rejected login: " + replyString);
         }
       }
 
       if (smtpUser != null && !client.auth(smtpUser, smtpPass)) {
-        String e = client.getReplyString();
-        throw new EmailException("SMTP server rejected auth: " + e);
+        throw new EmailException("SMTP server rejected auth: " + replyString);
       }
-    } catch (IOException e) {
+      return client;
+    } catch (IOException | EmailException e) {
       if (client.isConnected()) {
         try {
           client.disconnect();
         } catch (IOException e2) {
         }
       }
-      throw new EmailException(e.getMessage(), e);
-    } catch (EmailException e) {
-      if (client.isConnected()) {
-        try {
-          client.disconnect();
-        } catch (IOException e2) {
-        }
+      if (e instanceof EmailException) {
+        throw (EmailException) e;
       }
-      throw e;
+      throw new EmailException(e.getMessage(), e);
     }
-    return client;
   }
 }