Try to extract error message from non-2xx responses

2 files changed, 79 insertions, 3 deletions

diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java
index d48a86eea61..c2e94e14486 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java
@@ -1,6 +1,8 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.http.client.core.communication;
 
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.yahoo.component.Vtag;
 import com.yahoo.vespa.http.client.config.ConnectionParams;
 import com.yahoo.vespa.http.client.config.Endpoint;
@@ -36,6 +38,7 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.UUID;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
@@ -48,6 +51,7 @@ import java.util.zip.GZIPOutputStream;
 class ApacheGatewayConnection implements GatewayConnection {
 
     private static Logger log = Logger.getLogger(ApacheGatewayConnection.class.getName());
+    private static final ObjectMapper mapper = new ObjectMapper();
     private static final String PATH = "/reserved-for-internal-use/feedapi?";
     private final List<Integer> SUPPORTED_VERSIONS = new ArrayList<>();
     private static final byte[] START_OF_FEED_XML = "<vespafeed>\n".getBytes(StandardCharsets.UTF_8);
@@ -253,7 +257,7 @@ class ApacheGatewayConnection implements GatewayConnection {
             throw e;
         }
         try {
-            verifyServerResponseCode(response.getStatusLine());
+            verifyServerResponseCode(response);
             verifyServerVersion(response.getFirstHeader(Headers.VERSION));
             verifySessionHeader(response.getFirstHeader(Headers.SESSION_ID));
         } catch (ServerResponseException e) {
@@ -263,7 +267,8 @@ class ApacheGatewayConnection implements GatewayConnection {
         return response.getEntity().getContent();
     }
 
-    private void verifyServerResponseCode(StatusLine statusLine) throws ServerResponseException {
+    private void verifyServerResponseCode(HttpResponse response) throws ServerResponseException {
+        StatusLine statusLine = response.getStatusLine();
         // We use code 261-299 to report errors related to internal transitive errors that the tenants should not care
         // about to avoid masking more serious errors.
         int statusCode = statusLine.getStatusCode();
@@ -273,7 +278,22 @@ class ApacheGatewayConnection implements GatewayConnection {
         if (statusCode == 299) {
             throw new ServerResponseException(429, "Too  many requests.");
         }
-        throw new ServerResponseException(statusLine.getStatusCode(), statusLine.getReasonPhrase());
+        String message = tryGetDetailedErrorMessage(response)
+                .orElseGet(statusLine::getReasonPhrase);
+        throw new ServerResponseException(statusLine.getStatusCode(), message);
+    }
+
+    private static Optional<String> tryGetDetailedErrorMessage(HttpResponse response) {
+        Header contentType = response.getEntity().getContentType();
+        if (contentType == null || !contentType.getValue().equalsIgnoreCase("application/json")) return Optional.empty();
+        try (InputStream in = response.getEntity().getContent()) {
+            JsonNode jsonNode = mapper.readTree(in);
+            JsonNode message = jsonNode.get("message");
+            if (message == null || message.textValue() == null) return Optional.empty();
+            return Optional.of(response.getStatusLine().getReasonPhrase() + " - " + message.textValue());
+        } catch (IOException e) {
+            return Optional.empty();
+        }
     }
 
     private void verifySessionHeader(Header serverHeader) throws ServerResponseException {
diff --git a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnectionTest.java b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnectionTest.java
index 3136526fd42..5228d2b66f0 100644
--- a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnectionTest.java
+++ b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnectionTest.java
@@ -1,6 +1,9 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.http.client.core.communication;
 
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.yahoo.vespa.http.client.TestUtils;
 import com.yahoo.vespa.http.client.config.ConnectionParams;
 import com.yahoo.vespa.http.client.config.Endpoint;
@@ -17,7 +20,10 @@ import org.apache.http.StatusLine;
 import org.apache.http.client.HttpClient;
 import org.apache.http.client.methods.HttpPost;
 import org.apache.http.entity.InputStreamEntity;
+import org.apache.http.message.BasicHeader;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.mockito.stubbing.Answer;
 
 import java.io.ByteArrayInputStream;
@@ -25,6 +31,8 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
@@ -45,6 +53,9 @@ import static org.mockito.Mockito.when;
 
 public class ApacheGatewayConnectionTest {
 
+    @Rule
+    public ExpectedException expectedException = ExpectedException.none();
+
     @Test
     public void testProtocolV3() throws Exception {
         final Endpoint endpoint = Endpoint.create("hostname", 666, false);
@@ -292,6 +303,35 @@ public class ApacheGatewayConnectionTest {
         verify(headerProvider, times(3)).getHeaderValue(); // 1x connect(), 2x writeOperations()
     }
 
+    @Test
+    public void detailed_error_message_is_extracted_from_error_responses_with_json() throws IOException, ServerResponseException, InterruptedException {
+        String reasonPhrase = "Unauthorized";
+        String errorMessage = "Invalid credentials";
+        expectedException.expect(ServerResponseException.class);
+        expectedException.expectMessage(reasonPhrase + " - " + errorMessage);
+
+        CountDownLatch verifyContentSentLatch = new CountDownLatch(1);
+
+        ApacheGatewayConnection.HttpClientFactory mockFactory = mockHttpClientFactory(post  -> {
+            verifyContentSentLatch.countDown();
+            return createErrorHttpResponse(401, reasonPhrase, errorMessage);
+        });
+
+        ApacheGatewayConnection apacheGatewayConnection =
+            new ApacheGatewayConnection(
+                    Endpoint.create("hostname", 666, false),
+                    new FeedParams.Builder().build(),
+                    "",
+                    new ConnectionParams.Builder().build(),
+                    mockFactory,
+                    "clientId");
+        apacheGatewayConnection.connect();
+        apacheGatewayConnection.handshake();
+
+        apacheGatewayConnection.writeOperations(Collections.singletonList(createDoc("42", "content", true)));
+        assertTrue(verifyContentSentLatch.await(10, TimeUnit.SECONDS));
+    }
+
     private static ApacheGatewayConnection.HttpClientFactory mockHttpClientFactory(HttpExecuteMock httpExecuteMock) throws IOException {
         ApacheGatewayConnection.HttpClientFactory mockFactory =
                 mock(ApacheGatewayConnection.HttpClientFactory.class);
@@ -355,4 +395,20 @@ public class ApacheGatewayConnectionTest {
         when(httpEntityMock.getContent()).thenReturn(inputs);
         return httpResponseMock;
     }
+
+    private static HttpResponse createErrorHttpResponse(int statusCode, String reasonPhrase, String message) throws IOException {
+        HttpResponse response = mock(HttpResponse.class);
+
+        StatusLine statusLine = mock(StatusLine.class);
+        when(statusLine.getStatusCode()).thenReturn(statusCode);
+        when(statusLine.getReasonPhrase()).thenReturn(reasonPhrase);
+        when(response.getStatusLine()).thenReturn(statusLine);
+
+        HttpEntity httpEntity = mock(HttpEntity.class);
+        when(httpEntity.getContentType()).thenReturn(new BasicHeader("Content-Type", "application/json"));
+        String json = String.format("{\"message\": \"%s\"}", message);
+        when(httpEntity.getContent()).thenReturn(new ByteArrayInputStream(json.getBytes()));
+        when(response.getEntity()).thenReturn(httpEntity);
+        return response;
+    }
 }