Throw FeedException on handler errors

3 files changed, 88 insertions, 45 deletions

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<BiFunction<DocumentId, SimpleHttpRequest, CompletableFuture<SimpleHttpResponse>>> 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<SimpleHttpResponse> 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<SimpleHttpResponse> 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<SimpleHttpResponse> 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<SimpleHttpResponse> 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<SimpleHttpResponse> 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());
     }
 
 }