From: Julien Lancelot <julien.lancelot@sonarsource.com>
Date: Wed, 7 Sep 2016 16:35:18 +0000 (+0200)
Subject: SONAR-7973 Do not return error 500 when email sending is failing
X-Git-Tag: 6.1-RC1~188
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=refs%2Fpull%2F1220%2Fhead;p=sonarqube.git

SONAR-7973 Do not return error 500 when email sending is failing
---

diff --git a/server/sonar-server/src/main/java/org/sonar/server/email/ws/SendAction.java b/server/sonar-server/src/main/java/org/sonar/server/email/ws/SendAction.java
index a7b866acfe1..a406760f9e2 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/email/ws/SendAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/email/ws/SendAction.java
@@ -20,10 +20,17 @@
 
 package org.sonar.server.email.ws;
 
+import com.google.common.base.Throwables;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.commons.mail.EmailException;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.core.permission.GlobalPermissions;
+import org.sonar.server.exceptions.BadRequestException;
+import org.sonar.server.exceptions.Message;
 import org.sonar.server.notification.email.EmailNotificationChannel;
 import org.sonar.server.user.UserSession;
 
@@ -68,8 +75,21 @@ public class SendAction implements EmailsWsAction {
   @Override
   public void handle(Request request, Response response) throws Exception {
     userSession.checkPermission(GlobalPermissions.SYSTEM_ADMIN);
-    emailNotificationChannel.sendTestEmail(request.mandatoryParam(PARAM_TO), request.param(PARAM_SUBJECT), request.mandatoryParam(PARAM_MESSAGE));
+    try {
+      emailNotificationChannel.sendTestEmail(request.mandatoryParam(PARAM_TO), request.param(PARAM_SUBJECT), request.mandatoryParam(PARAM_MESSAGE));
+    } catch (EmailException emailException) {
+      throw createBadRequestException(emailException);
+    }
     response.noContent();
   }
 
+  private static BadRequestException createBadRequestException(EmailException emailException) {
+    List<Message> messages = Throwables.getCausalChain(emailException)
+      .stream()
+      .map(e -> Message.of(e.getMessage()))
+      .collect(Collectors.toList());
+    Collections.reverse(messages);
+    return new BadRequestException(messages);
+  }
+
 }
diff --git a/server/sonar-server/src/main/java/org/sonar/server/notification/email/EmailNotificationChannel.java b/server/sonar-server/src/main/java/org/sonar/server/notification/email/EmailNotificationChannel.java
index d3f40feea2d..1a55f2ac133 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/notification/email/EmailNotificationChannel.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/notification/email/EmailNotificationChannel.java
@@ -217,7 +217,7 @@ public class EmailNotificationChannel extends NotificationChannel {
       emailMessage.setMessage(message);
       send(emailMessage);
     } catch (EmailException e) {
-      LOG.error("Fail to send test email to: " + toAddress, e);
+      LOG.debug("Fail to send test email to: " + toAddress, e);
       throw e;
     }
   }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/email/ws/SendActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/email/ws/SendActionTest.java
index e90cda1057b..4d7b5220bf8 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/email/ws/SendActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/email/ws/SendActionTest.java
@@ -21,18 +21,24 @@
 package org.sonar.server.email.ws;
 
 import javax.annotation.Nullable;
+import org.apache.commons.mail.EmailException;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.core.permission.GlobalPermissions;
+import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.exceptions.ForbiddenException;
+import org.sonar.server.exceptions.Message;
 import org.sonar.server.notification.email.EmailNotificationChannel;
 import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.ws.TestRequest;
 import org.sonar.server.ws.WsActionTester;
 
 import static org.assertj.core.api.Java6Assertions.assertThat;
+import static org.junit.Assert.fail;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.sonar.core.permission.GlobalPermissions.SYSTEM_ADMIN;
@@ -91,6 +97,24 @@ public class SendActionTest {
     ws.newRequest().execute();
   }
 
+  @Test
+  public void fail_with_BadRequestException_when_EmailException_is_generated() throws Exception {
+    setUserAsSystemAdmin();
+    IllegalArgumentException exception1 = new IllegalArgumentException("root cause");
+    IllegalArgumentException exception2 = new IllegalArgumentException("parent cause", exception1);
+    IllegalArgumentException exception3 = new IllegalArgumentException("child cause", exception2);
+    EmailException emailException = new EmailException("last message", exception3);
+    doThrow(emailException).when(emailNotificationChannel).sendTestEmail(anyString(), anyString(), anyString());
+
+    try {
+      executeRequest("john@doo.com", "Test Message from SonarQube", "This is a test message from SonarQube at http://localhost:9000");
+      fail();
+    } catch (BadRequestException e) {
+      assertThat(e.errors().messages()).extracting(Message::getKey).containsExactly(
+        "root cause", "parent cause", "child cause", "last message");
+    }
+  }
+
   @Test
   public void test_ws_definition() {
     WebService.Action action = ws.getDef();