diff options
author | Bjørn Christian Seime <bjorncs@oath.com> | 2018-06-08 13:47:08 +0200 |
---|---|---|
committer | Bjørn Christian Seime <bjorncs@oath.com> | 2018-06-08 13:49:51 +0200 |
commit | 3d91147264d908748db400a901e09af773ea7b2c (patch) | |
tree | 5e78f91c70b93e1fe30f220e1c20b8128092867e /vespa-http-client | |
parent | 1428af42e5943fbb75497dac5a7f73260bbf5c58 (diff) |
Try to extract error message from non-2xx responses
Diffstat (limited to 'vespa-http-client')
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; + } } |