diff options
author | Jon Marius Venstad <jonmv@users.noreply.github.com> | 2021-06-08 16:50:58 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-06-08 16:50:58 +0200 |
commit | ce8501ed3ac9199d85cc4090c794940cb75e0b40 (patch) | |
tree | fa731945378b5a33bc087f0c7f3c39a8332332f3 | |
parent | 26b6320ab8fd8100028da9c1de6cf66c41f77a41 (diff) | |
parent | 1e6e0ec006aef053688d1217cc6f1d88453ae748 (diff) |
Merge pull request #18168 from vespa-engine/jonmv/vespa-feed-client
Document API errors are BAD_GATEWAY
2 files changed, 23 insertions, 8 deletions
diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java index 58857d1d8e6..789a2ab537f 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java @@ -743,11 +743,18 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { private static void serverError(HttpRequest request, Throwable t, ResponseHandler handler) { loggingException(() -> { - log.log(WARNING, "Uncaught exception handling request " + request.getMethod() + " " + request.getUri().getRawPath() + ":", t); + log.log(WARNING, "Uncaught exception handling request " + request.getMethod() + " " + request.getUri().getRawPath(), t); JsonResponse.create(request, Exceptions.toMessageString(t), handler).respond(Response.Status.INTERNAL_SERVER_ERROR); }); } + private static void badGateway(HttpRequest request, Throwable t, ResponseHandler handler) { + loggingException(() -> { + log.log(FINE, t, () -> "Document access error handling request " + request.getMethod() + " " + request.getUri().getRawPath()); + JsonResponse.create(request, Exceptions.toMessageString(t), handler).respond(Response.Status.BAD_GATEWAY); + }); + } + private static void timeout(HttpRequest request, String message, ResponseHandler handler) { loggingException(() -> { log.log(FINE, () -> "Timeout handling request " + request.getMethod() + " " + request.getUri().getRawPath() + ": " + message); @@ -803,6 +810,9 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { catch (IllegalArgumentException e) { badRequest(request, e, handler); } + catch (DispatchException e) { + badGateway(request, e, handler); + } catch (RuntimeException e) { serverError(request, e, handler); } @@ -821,12 +831,16 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { return false; if (result.type() == Result.ResultType.FATAL_ERROR) - throw new RuntimeException(result.getError()); + throw new DispatchException(result.getError()); outstanding.incrementAndGet(); return true; } + private static class DispatchException extends RuntimeException { + private DispatchException(Throwable cause) { super(cause); } + } + /** Readable content channel which forwards data to a reader when closed. */ static class ForwardingContentChannel implements ContentChannel { @@ -923,7 +937,7 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { log.log(WARNING, "Unexpected document API operation outcome '" + response.outcome() + "'"); case ERROR: log.log(FINE, () -> "Exception performing document operation: " + response.getTextMessage()); - jsonResponse.commit(Response.Status.INTERNAL_SERVER_ERROR); + jsonResponse.commit(Response.Status.BAD_GATEWAY); } } } @@ -1118,7 +1132,7 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { if (getVisitorStatistics() != null) response.writeDocumentCount(getVisitorStatistics().getDocumentsReturned()); - response.respond(Response.Status.INTERNAL_SERVER_ERROR); + response.respond(Response.Status.BAD_GATEWAY); } }); visitDispatcher.execute(() -> { diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java index 6f1b0466350..a0f6fd45dd8 100644 --- a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java +++ b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java @@ -330,6 +330,7 @@ public class DocumentV1ApiTest { assertEquals(400, response.getStatus()); // DELETE with namespace and document type is a restricted visit which deletes visited documents. + // When visiting fails fatally, a 502 BAD GATEWAY is returned. access.expect(tokens.subList(0, 1)); access.expect(parameters -> { assertEquals("(false) and (music) and (id.namespace=='space')", parameters.getDocumentSelection()); @@ -351,7 +352,7 @@ public class DocumentV1ApiTest { " \"pathId\": \"/document/v1/space/music/docid\"," + " \"message\": \"boom\"" + "}", response.readAll()); - assertEquals(500, response.getStatus()); + assertEquals(502, response.getStatus()); // DELETE at the root is also a deletion visit. These also require a selection. access.expect(parameters -> { @@ -386,7 +387,7 @@ public class DocumentV1ApiTest { " \"documents\": []," + " \"message\": \"error\"" + "}", response.readAll()); - assertEquals(500, response.getStatus()); + assertEquals(502, response.getStatus()); // GET with namespace, document type and number is a restricted visit. access.expect(parameters -> { @@ -649,12 +650,12 @@ public class DocumentV1ApiTest { " \"pathId\": \"/document/v1/space/music/number/1/two\"," + " \"message\": \"error\"" + "}", response1.readAll()); - assertEquals(500, response1.getStatus()); + assertEquals(502, response1.getStatus()); assertSameJson("{" + " \"pathId\": \"/document/v1/space/music/number/1/two\"," + " \"message\": \"error\"" + "}", response2.readAll()); - assertEquals(500, response2.getStatus()); + assertEquals(502, response2.getStatus()); // Request response does not arrive before timeout has passed. AtomicReference<ResponseHandler> handler = new AtomicReference<>(); |