From fc86176192b782ab139b420e63764b189d9a3b3a Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Thu, 10 Jun 2021 10:22:07 +0200 Subject: Throw FeedException on handler errors --- .../java/ai/vespa/feed/client/FeedException.java | 7 ++ .../java/ai/vespa/feed/client/HttpFeedClient.java | 21 ++--- .../ai/vespa/feed/client/HttpFeedClientTest.java | 105 ++++++++++++++------- 3 files changed, 88 insertions(+), 45 deletions(-) (limited to 'vespa-feed-client') diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/FeedException.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/FeedException.java index eb31d1aa808..25068818396 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/FeedException.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/FeedException.java @@ -5,4 +5,11 @@ package ai.vespa.feed.client; * @author bjorncs */ public class FeedException extends RuntimeException { + + public FeedException(String message) { super(message); } + + public FeedException(String message, Throwable cause) { super(message, cause); } + + public FeedException(Throwable cause) { super(cause); } + } diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/HttpFeedClient.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/HttpFeedClient.java index 6e7e20ae121..92e598e7307 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/HttpFeedClient.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/HttpFeedClient.java @@ -95,23 +95,18 @@ class HttpFeedClient implements FeedClient { request.setBody(operationJson, ContentType.APPLICATION_JSON); return requestStrategy.enqueue(documentId, request) - .handle((response, thrown) -> { - if (thrown != null) { - // TODO: What to do with exceptions here? Ex on 400, 401, 403, etc, and wrap and throw? - ByteArrayOutputStream buffer = new ByteArrayOutputStream(); - thrown.printStackTrace(new PrintStream(buffer)); - return new Result(Result.Type.failure, documentId, buffer.toString(), null); - } - return toResult(response, documentId); - }); + .thenApply(response -> toResult(request, response, documentId)); } - static Result toResult(SimpleHttpResponse response, DocumentId documentId) { + static Result toResult(SimpleHttpRequest request, SimpleHttpResponse response, DocumentId documentId) { Result.Type type; switch (response.getCode()) { case 200: type = Result.Type.success; break; case 412: type = Result.Type.conditionNotMet; break; - default: type = Result.Type.failure; + case 502: + case 504: + case 507: type = Result.Type.failure; break; + default: type = null; } String message = null; @@ -137,6 +132,10 @@ class HttpFeedClient implements FeedClient { throw new UncheckedIOException(e); } + if (type == null) // Not a Vespa response, but a failure in the HTTP layer. + throw new FeedException("Status " + response.getCode() + " executing '" + request + + "': " + (message == null ? request.getBodyText() : message)); + return new Result(type, documentId, message, trace); } diff --git a/vespa-feed-client/src/test/java/ai/vespa/feed/client/HttpFeedClientTest.java b/vespa-feed-client/src/test/java/ai/vespa/feed/client/HttpFeedClientTest.java index 65fbcb12204..6155d8bf3b6 100644 --- a/vespa-feed-client/src/test/java/ai/vespa/feed/client/HttpFeedClientTest.java +++ b/vespa-feed-client/src/test/java/ai/vespa/feed/client/HttpFeedClientTest.java @@ -12,9 +12,12 @@ import java.net.URI; import java.time.Duration; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiConsumer; +import java.util.function.BiFunction; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; /** * @author jonmv @@ -22,50 +25,84 @@ import static org.junit.jupiter.api.Assertions.assertEquals; class HttpFeedClientTest { @Test - void testRequestGeneration() throws IOException, ExecutionException, InterruptedException { + void testFeeding() throws ExecutionException, InterruptedException { DocumentId id = DocumentId.of("ns", "type", "0"); + AtomicReference>> dispatch = new AtomicReference<>(); class MockRequestStrategy implements RequestStrategy { @Override public OperationStats stats() { throw new UnsupportedOperationException(); } @Override public boolean hasFailed() { return false; } @Override public void destroy() { throw new UnsupportedOperationException(); } @Override public void await() { throw new UnsupportedOperationException(); } - @Override public CompletableFuture enqueue(DocumentId documentId, SimpleHttpRequest request) { - try { - assertEquals(id, documentId); - assertEquals("/document/v1/ns/type/docid/0?create=true&condition=false&timeout=5000ms&route=route", - request.getUri().toString()); - assertEquals("json", request.getBodyText()); + @Override public CompletableFuture enqueue(DocumentId documentId, SimpleHttpRequest request) { return dispatch.get().apply(documentId, request); } + } + FeedClient client = new HttpFeedClient(FeedClientBuilder.create(URI.create("https://dummy:123")), new MockRequestStrategy()); - SimpleHttpResponse response = new SimpleHttpResponse(502); - response.setBody("{\n" + - " \"pathId\": \"/document/v1/ns/type/docid/0\",\n" + - " \"id\": \"id:ns:type::0\",\n" + - " \"message\": \"Ooops! ... I did it again.\",\n" + - " \"trace\": \"I played with your heart. Got lost in the game.\"\n" + - "}", - ContentType.APPLICATION_JSON); - return CompletableFuture.completedFuture(response); - } - catch (Throwable thrown) { - CompletableFuture failed = new CompletableFuture<>(); - failed.completeExceptionally(thrown); - return failed; - } - } + // Vespa error is an error result. + dispatch.set((documentId, request) -> { + try { + assertEquals(id, documentId); + assertEquals("/document/v1/ns/type/docid/0?create=true&condition=false&timeout=5000ms&route=route", + request.getUri().toString()); + assertEquals("json", request.getBodyText()); - } - Result result = new HttpFeedClient(FeedClientBuilder.create(URI.create("https://dummy:123")), - new MockRequestStrategy()) - .put(id, - "json", - OperationParameters.empty() - .createIfNonExistent(true) - .testAndSetCondition("false") - .route("route") - .timeout(Duration.ofSeconds(5))) - .get(); + SimpleHttpResponse response = new SimpleHttpResponse(502); + response.setBody("{\n" + + " \"pathId\": \"/document/v1/ns/type/docid/0\",\n" + + " \"id\": \"id:ns:type::0\",\n" + + " \"message\": \"Ooops! ... I did it again.\",\n" + + " \"trace\": \"I played with your heart. Got lost in the game.\"\n" + + "}", + ContentType.APPLICATION_JSON); + return CompletableFuture.completedFuture(response); + } + catch (Throwable thrown) { + CompletableFuture failed = new CompletableFuture<>(); + failed.completeExceptionally(thrown); + return failed; + } + }); + Result result = client.put(id, + "json", + OperationParameters.empty() + .createIfNonExistent(true) + .testAndSetCondition("false") + .route("route") + .timeout(Duration.ofSeconds(5))) + .get(); assertEquals("Ooops! ... I did it again.", result.resultMessage().get()); assertEquals("I played with your heart. Got lost in the game.", result.traceMessage().get()); + + + // Handler error is a FeedException. + dispatch.set((documentId, request) -> { + try { + assertEquals(id, documentId); + assertEquals("/document/v1/ns/type/docid/0", + request.getUri().toString()); + assertEquals("json", request.getBodyText()); + + SimpleHttpResponse response = new SimpleHttpResponse(500); + response.setBody("{\n" + + " \"pathId\": \"/document/v1/ns/type/docid/0\",\n" + + " \"id\": \"id:ns:type::0\",\n" + + " \"message\": \"Alla ska i jorden.\",\n" + + " \"trace\": \"Din tid den kom, och senn så for den. \"\n" + + "}", + ContentType.APPLICATION_JSON); + return CompletableFuture.completedFuture(response); + } + catch (Throwable thrown) { + CompletableFuture failed = new CompletableFuture<>(); + failed.completeExceptionally(thrown); + return failed; + } + }); + ExecutionException expected = assertThrows(ExecutionException.class, + () -> client.put(id, + "json", + OperationParameters.empty()) + .get()); + assertEquals("Status 500 executing 'POST /document/v1/ns/type/docid/0': Alla ska i jorden.", expected.getCause().getMessage()); } } -- cgit v1.2.3