MLE-18056 Fixing null pointer bug in JSONErrorParser

Author: rjrudin

Jenkinsfile

@@ -21,6 +21,7 @@ def setupDockerMarkLogic(String image){
     docker compose down -v || true
     docker volume prune -f
     echo "Using image: "'''+image+'''
+    docker pull '''+image+'''
     MARKLOGIC_IMAGE='''+image+''' MARKLOGIC_LOGS_VOLUME=marklogicLogs docker compose up -d --build
 	  echo "mlPassword=admin" > gradle-local.properties
     echo "Waiting for MarkLogic server to initialize."

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

@@ -150,6 +150,11 @@ private static FailedRequest jsonFailedRequest(int httpStatus, InputStream conte
   public FailedRequest() {
   }
 
+  public FailedRequest(int statusCode, String messageString) {
+	  this.statusCode = statusCode;
+	  this.messageString = messageString;
+  }
+
   public String getStackTrace() {
     return stackTrace;
   }

marklogic-client-api/src/main/java/com/marklogic/client/io/JSONErrorParser.java

@@ -3,47 +3,38 @@
  */
 package com.marklogic.client.io;
 
-import java.io.IOException;
-import java.io.InputStream;
-import java.util.Map;
-
-import com.fasterxml.jackson.core.JsonParseException;
-import com.fasterxml.jackson.databind.JsonMappingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.marklogic.client.impl.FailedRequest;
 import com.marklogic.client.impl.FailedRequestParser;
 
-/**
- * This class is provided as a convenience method for parsing MarkLogic errors that
- * are serialized as JSON.  In order to use this class, your project must provide
- * the Jackson data binding library for JSON.
- */
+import java.io.InputStream;
+import java.util.Map;
+
 public class JSONErrorParser implements FailedRequestParser {
 
-  @SuppressWarnings("unchecked")
-  @Override
-  public FailedRequest parseFailedRequest(int httpStatus, InputStream content) {
-    FailedRequest failure = new FailedRequest();
-    ObjectMapper mapper = new ObjectMapper(); // can reuse, share globally
-    Map<String, Map<String, String>> errorData;
-    try {
-      errorData = mapper.readValue(content, Map.class);
-      Map<String, String> errorBody = errorData.get("errorResponse");
-      failure.setStatusCode(httpStatus);
-      failure.setStatusString(errorBody.get("status"));
-      failure.setMessageCode(errorBody.get("messageCode"));
-      failure.setMessageString(errorBody.get("message"));
-      failure.setStackTrace(errorBody.get("stackTrace"));
-    } catch (JsonParseException e1) {
-      failure.setStatusCode(httpStatus);
-      failure.setMessageString("Request failed. Error body not received from server");
-    } catch (JsonMappingException e1) {
-      failure.setStatusCode(httpStatus);
-      failure.setMessageString("Request failed. Error body not received from server");
-    } catch (IOException e1) {
-      failure.setStatusCode(httpStatus);
-      failure.setMessageString("Request failed. Error body not received from server");
-    }
-    return failure;
-  }
+	private final ObjectMapper objectMapper = new ObjectMapper();
+
+	@SuppressWarnings("unchecked")
+	@Override
+	public FailedRequest parseFailedRequest(int httpStatus, InputStream content) {
+		Map<String, Map<String, String>> errorData;
+		try {
+			errorData = objectMapper.readValue(content, Map.class);
+		} catch (Exception ex) {
+			return new FailedRequest(httpStatus, "Request failed; could not parse JSON in response body.");
+		}
+
+		if (!errorData.containsKey("errorResponse")) {
+			return new FailedRequest(httpStatus, "Unexpected JSON in response body; did not find 'errorResponse' key; response: " + errorData);
+		}
+
+		FailedRequest failure = new FailedRequest();
+		Map<String, String> errorBody = errorData.get("errorResponse");
+		failure.setStatusCode(httpStatus);
+		failure.setStatusString(errorBody.get("status"));
+		failure.setMessageCode(errorBody.get("messageCode"));
+		failure.setMessageString(errorBody.get("message"));
+		failure.setStackTrace(errorBody.get("stackTrace"));
+		return failure;
+	}
 }

marklogic-client-api/src/test/java/com/marklogic/client/test/io/DocumentMetadataHandleTest.java

@@ -183,6 +183,9 @@ public void testCapabilityEnum() {
   @Test
   // Requires MarkLogic 11 or higher now that we're using Docker; the INSTALL_CONVERTERS flag does not work for MarkLogic 10.
   @ExtendWith(RequiresML11.class)
+  @Disabled("This is consistently failing in Jenkins with an error of: " +
+	  "Process run error: fork: Cannot allocate memory. It runs fine locally and is ultimately just a test of a " +
+	  "v1/documents feature and not of the Java Client.")
   public void testMetadataPropertiesExtraction() {
 	String docId = "/test.bin";
 	// Make a document manager to work with binary files

marklogic-client-api/src/test/java/com/marklogic/client/test/io/JSONErrorParserTest.java

@@ -0,0 +1,58 @@
+/*
+ * Copyright © 2024 MarkLogic Corporation. All Rights Reserved.
+ */
+package com.marklogic.client.test.io;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.marklogic.client.impl.FailedRequest;
+import com.marklogic.client.io.JSONErrorParser;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.io.ByteArrayInputStream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class JSONErrorParserTest {
+
+	private final JSONErrorParser parser = new JSONErrorParser();
+	private final ObjectMapper mapper = new ObjectMapper();
+	private ObjectNode response;
+
+	@BeforeEach
+	void setup() {
+		response = mapper.createObjectNode();
+	}
+
+	@Test
+	void happyPath() {
+		response.putObject("errorResponse")
+			.put("status", "My status")
+			.put("messageCode", "MY_CODE")
+			.put("message", "My message")
+			.put("stackTrace", "My stacktrace");
+
+		FailedRequest failure = parser.parseFailedRequest(400, new ByteArrayInputStream(response.toPrettyString().getBytes()));
+		assertEquals(400, failure.getStatusCode());
+		assertEquals("My status", failure.getStatus());
+		assertEquals("MY_CODE", failure.getMessageCode());
+		assertEquals("My message", failure.getMessage());
+		assertEquals("My stacktrace", failure.getStackTrace());
+	}
+
+	@Test
+	void noErrorResponse() {
+		response.putObject("some_other_data");
+
+		FailedRequest failure = parser.parseFailedRequest(500, new ByteArrayInputStream(response.toPrettyString().getBytes()));
+		assertEquals(500, failure.getStatusCode());
+		assertEquals(
+			"Unexpected JSON in response body; did not find 'errorResponse' key; response: {some_other_data={}}",
+			failure.getMessage(),
+			"In the event that the user mistakenly sends a request to a non-REST-API server but still receives a JSON " +
+				"response body, the error should identify the issue and include the JSON response body to help the " +
+				"user realize that they likely sent the request to a non-REST-API server."
+		);
+	}
+}