From cb1e730d8d066c1fb3062211a4adc0e7215f8bd1 Mon Sep 17 00:00:00 2001 From: Babatunde Adeyemi Date: Fri, 24 Jul 2026 15:37:10 +0100 Subject: [PATCH 1/5] fix: Surface DHIS2 client errors when response body is not JSON --- src/main/java/org/hisp/dhis/BaseDhis2.java | 76 ++++++++++++++++++- .../java/org/hisp/dhis/BaseDhis2Test.java | 71 ++++++++++++++++- 2 files changed, 143 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/hisp/dhis/BaseDhis2.java b/src/main/java/org/hisp/dhis/BaseDhis2.java index d6ef3585..5f9a7ba0 100644 --- a/src/main/java/org/hisp/dhis/BaseDhis2.java +++ b/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 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
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,76 @@ 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) { + return StringUtils.EMPTY; + } } /** diff --git a/src/test/java/org/hisp/dhis/BaseDhis2Test.java b/src/test/java/org/hisp/dhis/BaseDhis2Test.java index 1b20104f..4b2f0499 100644 --- a/src/test/java/org/hisp/dhis/BaseDhis2Test.java +++ b/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; @@ -97,14 +104,14 @@ void testDeserializeDataElement() throws IOException { @Test void testWithMetadataImportParams() throws Exception { - Dhis2Config config = new Dhis2Config("https://server.org", "admin", "distrct"); - Dhis2 dhis2 = new Dhis2(new Dhis2Config("https://server.org", "admin", "distrct")); + Dhis2Config config = new Dhis2Config("https://company.com", "admin", "distrct"); + Dhis2 dhis2 = new Dhis2(new Dhis2Config("https://company.com", "admin", "distrct")); URIBuilder uriBuilder = config.getResolvedUriBuilder().appendPath("metadata"); URI expected = new URI( """ - https://server.org/api/metadata\ + https://company.com/api/metadata\ ?importStrategy=CREATE_AND_UPDATE\ &atomicMode=ALL\ &skipSharing=false\ @@ -151,4 +158,62 @@ void testWithAnalyticsQueryParams() { assertEquals(expected, decodedUrl); } + + @Test + void testHandleErrorsThrowsForGatewayHtmlResponse() { + Dhis2 dhis2 = new Dhis2(TestFixture.DEFAULT_CONFIG); + + BasicClassicHttpResponse response = new BasicClassicHttpResponse(502); + response.setEntity( + new StringEntity("502 Bad Gateway", ContentType.TEXT_HTML)); + + Dhis2ClientException ex = + assertThrows( + Dhis2ClientException.class, + () -> + dhis2.handleErrors( + response, "https://company.com/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() { + Dhis2 dhis2 = new Dhis2(TestFixture.DEFAULT_CONFIG); + + BasicClassicHttpResponse response = new BasicClassicHttpResponse(503); + response.setEntity(new StringEntity("Service Unavailable", (ContentType) null)); + + Dhis2ClientException ex = + assertThrows( + Dhis2ClientException.class, + () -> dhis2.handleErrors(response, "https://company.com/api/dataValueSets")); + + assertEquals(503, ex.getStatusCode()); + } + + @Test + void testHandleErrorsIgnoresJsonErrorResponse() { + Dhis2 dhis2 = new Dhis2(TestFixture.DEFAULT_CONFIG); + + 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://company.com/api/dataValueSets")); + } + + @Test + void testHandleErrorsIgnoresSuccessfulResponse() { + Dhis2 dhis2 = new Dhis2(TestFixture.DEFAULT_CONFIG); + + BasicClassicHttpResponse response = new BasicClassicHttpResponse(200); + response.setEntity(new StringEntity("{\"status\":\"OK\"}", ContentType.APPLICATION_JSON)); + + assertDoesNotThrow(() -> dhis2.handleErrors(response, "https://company.com/api/dataValueSets")); + } } From 550a479eccdd64e109c40edd3106d3e27db1d64f Mon Sep 17 00:00:00 2001 From: Babatunde Adeyemi Date: Fri, 24 Jul 2026 15:37:26 +0100 Subject: [PATCH 2/5] fix: Assert non-null response to resolve S2259 --- src/main/java/org/hisp/dhis/BaseDhis2.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/hisp/dhis/BaseDhis2.java b/src/main/java/org/hisp/dhis/BaseDhis2.java index 5f9a7ba0..2865f10b 100644 --- a/src/main/java/org/hisp/dhis/BaseDhis2.java +++ b/src/main/java/org/hisp/dhis/BaseDhis2.java @@ -1521,12 +1521,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; From 04ef6eb04751ef3ffa70e481e9233d9a72e0f328 Mon Sep 17 00:00:00 2001 From: Babatunde Adeyemi Date: Fri, 24 Jul 2026 15:37:27 +0100 Subject: [PATCH 3/5] chore: update test hostname --- src/test/java/org/hisp/dhis/BaseDhis2Test.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/test/java/org/hisp/dhis/BaseDhis2Test.java b/src/test/java/org/hisp/dhis/BaseDhis2Test.java index 4b2f0499..5adcbdc7 100644 --- a/src/test/java/org/hisp/dhis/BaseDhis2Test.java +++ b/src/test/java/org/hisp/dhis/BaseDhis2Test.java @@ -104,14 +104,14 @@ void testDeserializeDataElement() throws IOException { @Test void testWithMetadataImportParams() throws Exception { - Dhis2Config config = new Dhis2Config("https://company.com", "admin", "distrct"); - Dhis2 dhis2 = new Dhis2(new Dhis2Config("https://company.com", "admin", "distrct")); + Dhis2Config config = new Dhis2Config("https://server.org", "admin", "distrct"); + Dhis2 dhis2 = new Dhis2(new Dhis2Config("https://server.org", "admin", "distrct")); URIBuilder uriBuilder = config.getResolvedUriBuilder().appendPath("metadata"); URI expected = new URI( """ - https://company.com/api/metadata\ + https://server.org/api/metadata\ ?importStrategy=CREATE_AND_UPDATE\ &atomicMode=ALL\ &skipSharing=false\ @@ -172,7 +172,7 @@ void testHandleErrorsThrowsForGatewayHtmlResponse() { Dhis2ClientException.class, () -> dhis2.handleErrors( - response, "https://company.com/api/completeDataSetRegistrations")); + response, "https://server.org/api/completeDataSetRegistrations")); assertEquals(502, ex.getStatusCode()); assertTrue(ex.getMessage().contains("502"), ex.getMessage()); @@ -189,7 +189,7 @@ void testHandleErrorsThrowsForErrorResponseWithoutContentType() { Dhis2ClientException ex = assertThrows( Dhis2ClientException.class, - () -> dhis2.handleErrors(response, "https://company.com/api/dataValueSets")); + () -> dhis2.handleErrors(response, "https://server.org/api/dataValueSets")); assertEquals(503, ex.getStatusCode()); } @@ -204,7 +204,7 @@ void testHandleErrorsIgnoresJsonErrorResponse() { "{\"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://company.com/api/dataValueSets")); + assertDoesNotThrow(() -> dhis2.handleErrors(response, "https://server.org/api/dataValueSets")); } @Test @@ -214,6 +214,6 @@ void testHandleErrorsIgnoresSuccessfulResponse() { BasicClassicHttpResponse response = new BasicClassicHttpResponse(200); response.setEntity(new StringEntity("{\"status\":\"OK\"}", ContentType.APPLICATION_JSON)); - assertDoesNotThrow(() -> dhis2.handleErrors(response, "https://company.com/api/dataValueSets")); + assertDoesNotThrow(() -> dhis2.handleErrors(response, "https://server.org/api/dataValueSets")); } } From 2f8f8ad0ae18487b2805bb9231efb34275053abc Mon Sep 17 00:00:00 2001 From: Babatunde Adeyemi Date: Fri, 24 Jul 2026 15:37:27 +0100 Subject: [PATCH 4/5] chore: Use try-with-resources for BasicClassicHttpResponse in BaseDhis2Test --- .../java/org/hisp/dhis/BaseDhis2Test.java | 74 ++++++++++--------- 1 file changed, 40 insertions(+), 34 deletions(-) diff --git a/src/test/java/org/hisp/dhis/BaseDhis2Test.java b/src/test/java/org/hisp/dhis/BaseDhis2Test.java index 5adcbdc7..3dcba30a 100644 --- a/src/test/java/org/hisp/dhis/BaseDhis2Test.java +++ b/src/test/java/org/hisp/dhis/BaseDhis2Test.java @@ -160,60 +160,66 @@ void testWithAnalyticsQueryParams() { } @Test - void testHandleErrorsThrowsForGatewayHtmlResponse() { + void testHandleErrorsThrowsForGatewayHtmlResponse() throws IOException { Dhis2 dhis2 = new Dhis2(TestFixture.DEFAULT_CONFIG); - BasicClassicHttpResponse response = new BasicClassicHttpResponse(502); - response.setEntity( - new StringEntity("502 Bad Gateway", 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()); + try (BasicClassicHttpResponse response = new BasicClassicHttpResponse(502)) { + response.setEntity( + new StringEntity("502 Bad Gateway", 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() { + void testHandleErrorsThrowsForErrorResponseWithoutContentType() throws IOException { Dhis2 dhis2 = new Dhis2(TestFixture.DEFAULT_CONFIG); - BasicClassicHttpResponse response = new BasicClassicHttpResponse(503); - response.setEntity(new StringEntity("Service Unavailable", (ContentType) null)); + 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")); + Dhis2ClientException ex = + assertThrows( + Dhis2ClientException.class, + () -> dhis2.handleErrors(response, "https://server.org/api/dataValueSets")); - assertEquals(503, ex.getStatusCode()); + assertEquals(503, ex.getStatusCode()); + } } @Test - void testHandleErrorsIgnoresJsonErrorResponse() { + void testHandleErrorsIgnoresJsonErrorResponse() throws IOException { Dhis2 dhis2 = new Dhis2(TestFixture.DEFAULT_CONFIG); - BasicClassicHttpResponse response = new BasicClassicHttpResponse(409); - response.setEntity( - new StringEntity( - "{\"status\":\"ERROR\",\"httpStatusCode\":409}", ContentType.APPLICATION_JSON)); + 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")); + // 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() { + void testHandleErrorsIgnoresSuccessfulResponse() throws IOException { Dhis2 dhis2 = new Dhis2(TestFixture.DEFAULT_CONFIG); - BasicClassicHttpResponse response = new BasicClassicHttpResponse(200); - response.setEntity(new StringEntity("{\"status\":\"OK\"}", ContentType.APPLICATION_JSON)); + try (BasicClassicHttpResponse response = new BasicClassicHttpResponse(200)) { + response.setEntity(new StringEntity("{\"status\":\"OK\"}", ContentType.APPLICATION_JSON)); - assertDoesNotThrow(() -> dhis2.handleErrors(response, "https://server.org/api/dataValueSets")); + assertDoesNotThrow( + () -> dhis2.handleErrors(response, "https://server.org/api/dataValueSets")); + } } } From f257117a22748ba0c3a3009bcc49b806d33a1ead Mon Sep 17 00:00:00 2001 From: Babatunde Adeyemi Date: Fri, 24 Jul 2026 15:37:27 +0100 Subject: [PATCH 5/5] chore: log response body read exception --- src/main/java/org/hisp/dhis/BaseDhis2.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/org/hisp/dhis/BaseDhis2.java b/src/main/java/org/hisp/dhis/BaseDhis2.java index 2865f10b..b3add9d7 100644 --- a/src/main/java/org/hisp/dhis/BaseDhis2.java +++ b/src/main/java/org/hisp/dhis/BaseDhis2.java @@ -1239,6 +1239,8 @@ private String getBodySnippet(ClassicHttpResponse response) { 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; } }