Merge pull request #1530 from marklogic/feature/eval-error-fix

DEVEXP-191 Ensuring eval errors are returned as JSON

Author: rjrudin

marklogic-client-api/src/main/java/com/marklogic/client/impl/OkHttpServices.java

@@ -3851,28 +3851,24 @@ private <U extends OkHttpResultIterator> U postIteratedResourceImpl(
     requestBldr = addTransactionScopedCookies(requestBldr, transaction);
     requestBldr = addTelemetryAgentId(requestBldr);
     requestBldr = addTrailerHeadersIfNecessary(requestBldr, path);
+	requestBldr = setErrorFormatIfNecessary(requestBldr, path);
 
-    Consumer<Boolean> resendableConsumer = new Consumer<Boolean>() {
-      public void accept(Boolean resendable) {
+    Consumer<Boolean> resendableConsumer = resendable -> {
         if (!isResendable) {
           checkFirstRequest();
           throw new ResourceNotResendableException(
             "Cannot retry request for " + path);
         }
-      }
-    };
-    Function<Request.Builder, Response> doPostFunction = new Function<Request.Builder, Response>() {
-      public Response apply(Request.Builder funcBuilder) {
-        return doPost(reqlog, funcBuilder.header(HEADER_ACCEPT, multipartMixedWithBoundary()),
-          inputBase.sendContent());
-      }
     };
-    Response response = sendRequestWithRetry(requestBldr, (transaction == null), doPostFunction, resendableConsumer);
-    int status = response.code();
 
-    checkStatus(response, status, "apply", "resource", path,
-      ResponseStatus.OK_OR_CREATED_OR_NO_CONTENT);
+    Function<Request.Builder, Response> doPostFunction = requestBuilder -> doPost(
+		reqlog,
+		requestBuilder.header(HEADER_ACCEPT, multipartMixedWithBoundary()),
+		inputBase.sendContent()
+	);
 
+	Response response = sendRequestWithRetry(requestBldr, (transaction == null), doPostFunction, resendableConsumer);
+    checkStatus(response, response.code(), "apply", "resource", path, ResponseStatus.OK_OR_CREATED_OR_NO_CONTENT);
     return makeResults(constructor, reqlog, "apply", "resource", response);
   }
 
@@ -4198,6 +4194,18 @@ private Request.Builder addTrailerHeadersIfNecessary(Request.Builder requestBldr
         return requestBldr;
     }
 
+	private Request.Builder setErrorFormatIfNecessary(Request.Builder requestBuilder, String path) {
+		// Slightly dirty hack; per https://docs.marklogic.com/guide/rest-dev/intro#id_34966, the X-Error-Accept header
+		// should be used to specify the error format. A REST API server defaults to 'json', though the App-Services app
+		// server defaults to 'compatible'. If the error format is 'compatible', a block of HTML is sent back which
+		// causes an error that prevents the user from seeing the actual error from the server. So for all eval calls,
+		// X-Error-Accept is used to request any errors back as JSON so that they can be handled correctly.
+		if ("eval".equals(path)) {
+			requestBuilder.addHeader(HEADER_ERROR_FORMAT, "application/json");
+		}
+		return requestBuilder;
+	}
+
   private <W extends AbstractWriteHandle> boolean addParts(
     MultipartBody.Builder multiPart, RequestLogger reqlog, W[] input)
   {

marklogic-client-api/src/test/java/com/marklogic/client/test/EvalTest.java

@@ -19,6 +19,7 @@
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.NullNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.marklogic.client.DatabaseClient;
 import com.marklogic.client.FailedRequestException;
 import com.marklogic.client.Transaction;
@@ -32,6 +33,8 @@
 import com.marklogic.client.io.*;
 import com.marklogic.client.query.DeleteQueryDefinition;
 import com.marklogic.client.query.QueryManager;
+import com.marklogic.mgmt.ManageClient;
+import com.marklogic.mgmt.resource.appservers.ServerManager;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
@@ -61,15 +64,47 @@ public class EvalTest {
   private static DatabaseClient restAdminClient = Common.connectRestAdmin();
 
   @BeforeAll
-  public static void beforeClass() {
+  public static void beforeAll() {
     libMgr = restAdminClient.newServerConfigManager().newExtensionLibrariesManager();
     Common.connectEval();
 
     septFirst.set(2014, Calendar.SEPTEMBER, 1, 0, 0, 0);
     septFirst.set(Calendar.MILLISECOND, 0);
+
+	  // In order to verify that X-Error-Accept is used to request back errors as JSON, the app server used by this test
+	  // must first be modified to default to "compatible" which results in a block of HTML being sent back, which will
+	  // cause an error to be returned when the client tries to process it as JSON.
+	  ObjectNode payload = Common.newServerPayload().put("default-error-format", "compatible");
+	  new ServerManager(Common.newManageClient()).save(payload.toString());
   }
+
   @AfterAll
-  public static void afterClass() {
+  public static void afterAll() {
+	  // Reset the app server back to the default of "json" as the error format
+	  ObjectNode payload = Common.newServerPayload().put("default-error-format", "json");
+	  new ServerManager(Common.newManageClient()).save(payload.toString());
+  }
+
+  @Test
+  void invalidJavascript() {
+	  FailedRequestException ex = assertThrows(FailedRequestException.class, () ->
+		  Common.evalClient.newServerEval().javascript("console.log('This fails").evalAs(String.class));
+
+	  String message = ex.getServerMessage();
+	  assertTrue(message.contains("Invalid or unexpected token"), "The error message from the server is expected " +
+		  "to contain the actual error, which in this case is due to bad syntax. In order for this to happen, the " +
+		  "Java Client should send the X-Error-Accept header per the docs at " +
+		  "https://docs.marklogic.com/guide/rest-dev/intro#id_34966; actual error: " + message);
+  }
+
+  @Test
+  void invalidXQuery() {
+	  FailedRequestException ex = assertThrows(FailedRequestException.class, () ->
+		  Common.evalClient.newServerEval().xquery("let $var := this fails").evalAs(String.class));
+
+	  String message = ex.getServerMessage();
+	  assertTrue(message.contains("Unexpected token syntax error"), "The server error message should contain the " +
+		  "actual error; actual message: " + message);
   }
 
   @Test