Use 500 for errors in document API, from /doc/v1

2 files changed, 10 insertions, 19 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 ca2b0a10c36..19e0c0dc77d 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
@@ -52,6 +52,7 @@ import com.yahoo.documentapi.metrics.DocumentOperationStatus;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.jdisc.Request;
 import com.yahoo.jdisc.Response;
+import com.yahoo.jdisc.Response.Status;
 import com.yahoo.jdisc.handler.AbstractRequestHandler;
 import com.yahoo.jdisc.handler.BufferedContentChannel;
 import com.yahoo.jdisc.handler.CompletionHandler;
@@ -909,13 +910,6 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler {
         });
     }
 
-    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);
@@ -971,9 +965,6 @@ 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);
             }
@@ -1097,11 +1088,11 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler {
                     case TIMEOUT -> jsonResponse.commit(Response.Status.GATEWAY_TIMEOUT);
                     case ERROR -> {
                         log.log(FINE, () -> "Exception performing document operation: " + response.getTextMessage());
-                        jsonResponse.commit(Response.Status.BAD_GATEWAY);
+                        jsonResponse.commit(Status.INTERNAL_SERVER_ERROR);
                     }
                     default -> {
                         log.log(WARNING, "Unexpected document API operation outcome '" + response.outcome() + "' " + response.getTextMessage());
-                        jsonResponse.commit(Response.Status.BAD_GATEWAY);
+                        jsonResponse.commit(Status.INTERNAL_SERVER_ERROR);
                     }
                 }
             }
@@ -1402,7 +1393,7 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler {
                             if (session.get() != null)
                                 response.writeTrace(session.get().getTrace());
 
-                            int status = Response.Status.BAD_GATEWAY;
+                            int status = Status.INTERNAL_SERVER_ERROR;
                             switch (code) {
                                 case TIMEOUT: // Intentional fallthrough.
                                 case ABORTED:
@@ -1539,7 +1530,7 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler {
                 case 404 -> report(DocumentOperationStatus.NOT_FOUND);
                 case 412 -> report(DocumentOperationStatus.CONDITION_FAILED);
                 case 429 -> report(DocumentOperationStatus.TOO_MANY_REQUESTS);
-                case 500,502,503,504,507 -> report(DocumentOperationStatus.SERVER_ERROR);
+                case 500,503,504,507 -> report(DocumentOperationStatus.SERVER_ERROR);
                 default -> throw new IllegalStateException("Unexpected status code '%s'".formatted(response.getStatus()));
             }
             metrics.reportHttpRequest(clientVersion());
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 43b0db1464c..c8fcb4c4635 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
@@ -460,7 +460,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.
+        // When visiting fails fatally, a 500 INTERAL SERVER ERROR is returned.
         access.expect(tokens.subList(0, 1));
         access.expect(parameters -> {
             assertEquals("(false) and (music) and (id.namespace=='space')", parameters.getDocumentSelection());
@@ -485,7 +485,7 @@ public class DocumentV1ApiTest {
                          "message": "boom"
                        }""",
                        response.readAll());
-        assertEquals(502, response.getStatus());
+        assertEquals(500, response.getStatus());
 
         // DELETE at the root is also a deletion visit. These also require a selection.
         access.expect(parameters -> {
@@ -527,7 +527,7 @@ public class DocumentV1ApiTest {
                          "message": "error"
                        }""",
                        response.readAll());
-        assertEquals(502, response.getStatus());
+        assertEquals(500, response.getStatus());
 
         // GET with namespace, document type and number is a restricted visit.
         access.expect(parameters -> {
@@ -931,12 +931,12 @@ public class DocumentV1ApiTest {
                        "  \"pathId\": \"/document/v1/space/music/number/1/two\"," +
                        "  \"message\": \"[FATAL_ERROR @ localhost]: FATAL_ERROR\"" +
                        "}", response1.readAll());
-        assertEquals(502, response1.getStatus());
+        assertEquals(500, response1.getStatus());
         assertSameJson("{" +
                        "  \"pathId\": \"/document/v1/space/music/number/1/two\"," +
                        "  \"message\": \"[FATAL_ERROR @ localhost]: FATAL_ERROR\"" +
                        "}", response2.readAll());
-        assertEquals(502, response2.getStatus());
+        assertEquals(500, response2.getStatus());
 
         // Request response does not arrive before timeout has passed.
         AtomicReference<ResponseHandler> handler = new AtomicReference<>();