From 1e6e0ec006aef053688d1217cc6f1d88453ae748 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Tue, 8 Jun 2021 15:59:57 +0200 Subject: Document API errors are BAD_GATEWAY --- .../restapi/resource/DocumentV1ApiHandler.java | 22 ++++++++++++++++++---- .../restapi/resource/DocumentV1ApiTest.java | 9 +++++---- 2 files changed, 23 insertions(+), 8 deletions(-) (limited to 'vespaclient-container-plugin') 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 handler = new AtomicReference<>(); -- cgit v1.2.3