diff --git a/src/main/java/org/hisp/dhis/BaseDhis2.java b/src/main/java/org/hisp/dhis/BaseDhis2.java index d6ef3585..b3add9d7 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,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; diff --git a/src/test/java/org/hisp/dhis/BaseDhis2Test.java b/src/test/java/org/hisp/dhis/BaseDhis2Test.java index 1b20104f..3dcba30a 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; @@ -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("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() 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")); + } + } }