fix: Surface DHIS2 client errors when response body is not JSON

src/main/java/org/hisp/dhis/BaseDhis2.java

@@ -58,6 +58,7 @@
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.Strings;
 import org.apache.hc.client5.http.HttpResponseException;
 import org.apache.hc.client5.http.classic.methods.HttpDelete;
 import org.apache.hc.client5.http.classic.methods.HttpGet;
@@ -150,6 +151,9 @@ public class BaseDhis2 {
   /** Error status codes for GET queries. */
   private static final Set<Integer> GET_ERROR_STATUS_CODES = Set.of(SC_BAD_REQUEST, SC_CONFLICT);
 
+  /** Maximum length of the response body snippet included in error messages. */
+  private static final int MAX_ERROR_BODY_SNIPPET_LENGTH = 500;
+
   // Headers
 
   protected static final Header HEADER_CONTENT_TYPE_JSON =
@@ -1153,7 +1157,7 @@ protected HttpGet getHttpGetRequest(URI url, List<Header> headers) {
    * @param url the request URL.
    * @throws Dhis2ClientException in the case of error status codes.
    */
-  private void handleErrors(HttpResponse response, String url) {
+  void handleErrors(ClassicHttpResponse response, String url) {
     int code = response.getCode();
 
     if (redirectedToLogin(response)) {
@@ -1167,6 +1171,78 @@ private void handleErrors(HttpResponse response, String url) {
 
       throw new Dhis2ClientException(message, code);
     }
+
+    // Gateway and proxy errors (e.g. 502, 503, 504) and unexpected login redirects
+    // typically return a non-JSON body such as an HTML error or login page. Detect these
+    // before attempting to parse the body as JSON, and surface the HTTP status code and a
+    // snippet of the response body so the underlying cause is immediately diagnosable.
+    if (isErrorStatusCode(code) && !isJsonResponse(response)) {
+      String snippet = getBodySnippet(response);
+      String message =
+          StringUtils.isNotBlank(snippet)
+              ? String.format("%s (%d): %s", getErrorMessage(code), code, snippet)
+              : String.format("%s (%d)", getErrorMessage(code), code);
+
+      log(code, "Error URL: '{}', response body: '{}'", url, snippet);
+
+      throw new Dhis2ClientException(message, code);
+    }
+  }
+
+  /**
+   * Indicates whether the given status code represents an error, i.e. a client (4xx) or server
+   * (5xx) error.
+   *
+   * @param code the HTTP status code.
+   * @return true if the status code represents an error.
+   */
+  private boolean isErrorStatusCode(int code) {
+    return code >= SC_BAD_REQUEST;
+  }
+
+  /**
+   * Indicates whether the given response has a JSON content type. A response without a content type
+   * header is not considered a JSON response.
+   *
+   * @param response the {@link ClassicHttpResponse}.
+   * @return true if the response content type is JSON.
+   */
+  private boolean isJsonResponse(ClassicHttpResponse response) {
+    HttpEntity entity = response.getEntity();
+
+    if (entity == null || StringUtils.isBlank(entity.getContentType())) {
+      return false;
+    }
+
+    String mimeType = ContentType.parse(entity.getContentType()).getMimeType();
+
+    return mimeType != null && Strings.CI.contains(mimeType, "json");
+  }
+
+  /**
+   * Returns a truncated, single-line snippet of the given response body, or an empty string if the
+   * body could not be read.
+   *
+   * @param response the {@link ClassicHttpResponse}.
+   * @return a response body snippet.
+   */
+  private String getBodySnippet(ClassicHttpResponse response) {
+    try {
+      HttpEntity entity = response.getEntity();
+
+      if (entity == null) {
+        return StringUtils.EMPTY;
+      }
+
+      String body = EntityUtils.toString(entity, StandardCharsets.UTF_8);
+
+      return StringUtils.abbreviate(
+          StringUtils.normalizeSpace(body), MAX_ERROR_BODY_SNIPPET_LENGTH);
+    } catch (IOException | ParseException ex) {
+      log.debug("Failed to read response body for snippet", ex);
+
+      return StringUtils.EMPTY;
+    }
   }
 
   /**
@@ -1447,12 +1523,11 @@ protected Response addToCollection(String path, String id, String collection, St
                 .appendPath(collection)
                 .appendPath(item));
 
-    Response response = executeRequest(new HttpPost(url));
+    Response response =
+        Objects.requireNonNull(executeRequest(new HttpPost(url)), "Response must not be null");
 
     Status status =
-        response != null
-                && response.getHttpStatus() != null
-                && response.getHttpStatus().is2xxSuccessful()
+        response.getHttpStatus() != null && response.getHttpStatus().is2xxSuccessful()
             ? Status.OK
             : Status.ERROR;
 

src/test/java/org/hisp/dhis/BaseDhis2Test.java

@@ -27,18 +27,25 @@
  */
 package org.hisp.dhis;
 
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.io.IOException;
 import java.net.URI;
 import java.util.List;
+import org.apache.hc.core5.http.ContentType;
+import org.apache.hc.core5.http.io.entity.StringEntity;
+import org.apache.hc.core5.http.message.BasicClassicHttpResponse;
 import org.apache.hc.core5.net.URIBuilder;
 import org.hisp.dhis.model.AggregationType;
 import org.hisp.dhis.model.DataDomain;
 import org.hisp.dhis.model.DataElement;
 import org.hisp.dhis.model.ValueType;
 import org.hisp.dhis.query.analytics.AnalyticsQuery;
+import org.hisp.dhis.response.Dhis2ClientException;
 import org.hisp.dhis.support.TestTags;
 import org.hisp.dhis.util.CodecUtils;
 import org.junit.jupiter.api.Tag;
@@ -151,4 +158,68 @@ void testWithAnalyticsQueryParams() {
 
     assertEquals(expected, decodedUrl);
   }
+
+  @Test
+  void testHandleErrorsThrowsForGatewayHtmlResponse() throws IOException {
+    Dhis2 dhis2 = new Dhis2(TestFixture.DEFAULT_CONFIG);
+
+    try (BasicClassicHttpResponse response = new BasicClassicHttpResponse(502)) {
+      response.setEntity(
+          new StringEntity("<html><body>502 Bad Gateway</body></html>", ContentType.TEXT_HTML));
+
+      Dhis2ClientException ex =
+          assertThrows(
+              Dhis2ClientException.class,
+              () ->
+                  dhis2.handleErrors(
+                      response, "https://server.org/api/completeDataSetRegistrations"));
+
+      assertEquals(502, ex.getStatusCode());
+      assertTrue(ex.getMessage().contains("502"), ex.getMessage());
+      assertTrue(ex.getMessage().contains("502 Bad Gateway"), ex.getMessage());
+    }
+  }
+
+  @Test
+  void testHandleErrorsThrowsForErrorResponseWithoutContentType() throws IOException {
+    Dhis2 dhis2 = new Dhis2(TestFixture.DEFAULT_CONFIG);
+
+    try (BasicClassicHttpResponse response = new BasicClassicHttpResponse(503)) {
+      response.setEntity(new StringEntity("Service Unavailable", (ContentType) null));
+
+      Dhis2ClientException ex =
+          assertThrows(
+              Dhis2ClientException.class,
+              () -> dhis2.handleErrors(response, "https://server.org/api/dataValueSets"));
+
+      assertEquals(503, ex.getStatusCode());
+    }
+  }
+
+  @Test
+  void testHandleErrorsIgnoresJsonErrorResponse() throws IOException {
+    Dhis2 dhis2 = new Dhis2(TestFixture.DEFAULT_CONFIG);
+
+    try (BasicClassicHttpResponse response = new BasicClassicHttpResponse(409)) {
+      response.setEntity(
+          new StringEntity(
+              "{\"status\":\"ERROR\",\"httpStatusCode\":409}", ContentType.APPLICATION_JSON));
+
+      // A JSON error body (e.g. a DHIS2 conflict WebMessage) must be left for the caller to parse.
+      assertDoesNotThrow(
+          () -> dhis2.handleErrors(response, "https://server.org/api/dataValueSets"));
+    }
+  }
+
+  @Test
+  void testHandleErrorsIgnoresSuccessfulResponse() throws IOException {
+    Dhis2 dhis2 = new Dhis2(TestFixture.DEFAULT_CONFIG);
+
+    try (BasicClassicHttpResponse response = new BasicClassicHttpResponse(200)) {
+      response.setEntity(new StringEntity("{\"status\":\"OK\"}", ContentType.APPLICATION_JSON));
+
+      assertDoesNotThrow(
+          () -> dhis2.handleErrors(response, "https://server.org/api/dataValueSets"));
+    }
+  }
 }