From e10c6ffc024ab79b1c188378726bb5da60ee2fec Mon Sep 17 00:00:00 2001 From: Teryk Bellahsene Date: Thu, 4 Feb 2016 17:58:07 +0100 Subject: [PATCH] SONAR-7040 Add context when failing to write a web service response --- .../java/org/sonar/server/ws/WsUtils.java | 10 ++++- .../java/org/sonar/server/ws/WsUtilsTest.java | 21 +++++++++ .../org/sonarqube/ws/MessageFormatter.java | 34 +++++++++++++++ .../sonarqube/ws/MessageFormatterTest.java | 43 +++++++++++++++++++ 4 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 sonar-ws/src/main/java/org/sonarqube/ws/MessageFormatter.java create mode 100644 sonar-ws/src/test/java/org/sonarqube/ws/MessageFormatterTest.java diff --git a/server/sonar-server/src/main/java/org/sonar/server/ws/WsUtils.java b/server/sonar-server/src/main/java/org/sonar/server/ws/WsUtils.java index 5b79587d4da..66d172e808a 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ws/WsUtils.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ws/WsUtils.java @@ -20,6 +20,7 @@ package org.sonar.server.ws; import com.google.common.base.Optional; +import com.google.common.base.Throwables; import com.google.protobuf.Message; import java.io.OutputStream; import java.io.OutputStreamWriter; @@ -27,21 +28,25 @@ import javax.annotation.Nullable; import org.apache.commons.io.IOUtils; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; +import org.sonar.api.utils.log.Logger; +import org.sonar.api.utils.log.Loggers; import org.sonar.api.utils.text.JsonWriter; import org.sonar.core.util.ProtobufJsonFormat; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.NotFoundException; import org.sonarqube.ws.MediaTypes; +import org.sonarqube.ws.MessageFormatter; import static java.lang.String.format; public class WsUtils { + private static final Logger LOG = Loggers.get(WsUtils.class); private WsUtils() { // only statics } - public static void writeProtobuf(Message msg, Request request, Response response) throws Exception { + public static void writeProtobuf(Message msg, Request request, Response response) { OutputStream output = response.stream().output(); try { if (request.getMediaType().equals(MediaTypes.PROTOBUF)) { @@ -53,6 +58,9 @@ public class WsUtils { ProtobufJsonFormat.write(msg, JsonWriter.of(writer)); } } + } catch (Exception e) { + LOG.error("Error while writing protobuf message {}", MessageFormatter.print(msg)); + Throwables.propagate(e); } finally { IOUtils.closeQuietly(output); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/ws/WsUtilsTest.java b/server/sonar-server/src/test/java/org/sonar/server/ws/WsUtilsTest.java index 0a0e8be2866..9bc26875551 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ws/WsUtilsTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ws/WsUtilsTest.java @@ -19,12 +19,15 @@ */ package org.sonar.server.ws; +import java.io.IOException; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.sonar.api.utils.log.LogTester; import org.sonar.server.exceptions.BadRequestException; import org.sonarqube.ws.Issues; import org.sonarqube.ws.MediaTypes; +import org.sonarqube.ws.WsPermissions; import static org.assertj.core.api.Assertions.assertThat; @@ -32,6 +35,8 @@ public class WsUtilsTest { @Rule public ExpectedException expectedException = ExpectedException.none(); + @Rule + public LogTester logger = new LogTester(); @Test public void write_json_by_default() throws Exception { @@ -61,6 +66,22 @@ public class WsUtilsTest { assertThat(Issues.Issue.parseFrom(response.getFlushedOutput()).getKey()).isEqualTo("I1"); } + @Test + public void log_message_when_error_writing_message() throws IOException { + TestRequest request = new TestRequest(); + request.setMediaType(MediaTypes.PROTOBUF); + + WsPermissions.Permission message = WsPermissions.Permission.newBuilder().setName("permission-name").build(); + + try { + // provoke NullPointerException + WsUtils.writeProtobuf(message, null, new DumbResponse()); + } catch (Exception e) { + assertThat(e).isInstanceOf(NullPointerException.class); + assertThat(logger.logs()).contains("Error while writing protobuf message org.sonarqube.ws.WsPermissions.Permission[name: \"permission-name\"]"); + } + } + @Test public void checkRequest_ok() { WsUtils.checkRequest(true, "Missing param: %s", "foo"); diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/MessageFormatter.java b/sonar-ws/src/main/java/org/sonarqube/ws/MessageFormatter.java new file mode 100644 index 00000000000..aafc7777115 --- /dev/null +++ b/sonar-ws/src/main/java/org/sonarqube/ws/MessageFormatter.java @@ -0,0 +1,34 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonarqube.ws; + +import com.google.protobuf.MessageOrBuilder; +import com.google.protobuf.TextFormat; + +public class MessageFormatter { + private MessageFormatter() { + // prevent instantiation + } + + public static String print(MessageOrBuilder message) { + return String.format("%s[%s]", message.getClass().getCanonicalName(), TextFormat.shortDebugString(message)); + } +} diff --git a/sonar-ws/src/test/java/org/sonarqube/ws/MessageFormatterTest.java b/sonar-ws/src/test/java/org/sonarqube/ws/MessageFormatterTest.java new file mode 100644 index 00000000000..bad2eaeba4a --- /dev/null +++ b/sonar-ws/src/test/java/org/sonarqube/ws/MessageFormatterTest.java @@ -0,0 +1,43 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonarqube.ws; + +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class MessageFormatterTest { + + @Test + public void print() { + WsPermissions.Permission.Builder message = WsPermissions.Permission.newBuilder() + .setName("permission-name") + .setKey("permission-key") + .setDescription("permission-description") + .setUsersCount(1984) + .setGroupsCount(42); + + String result = MessageFormatter.print(message); + + assertThat(result).isEqualTo("org.sonarqube.ws.WsPermissions.Permission.Builder" + + "[key: \"permission-key\" name: \"permission-name\" description: \"permission-description\" usersCount: 1984 groupsCount: 42]"); + } +} -- 2.39.5