Skip to content

[DE-960] tolerate error responses with text content-type #587

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 28, 2024
Merged

[DE-960] tolerate error responses with text content-type #587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
adf023a
improved error reporting of responses with unparsable body
rashtao Nov 27, 2024
e92853d
tests
rashtao Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions core/src/main/java/com/arangodb/internal/net/Communication.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicLong;

import static com.arangodb.internal.util.SerdeUtils.toJsonString;

@UsedInApi
public abstract class Communication implements Closeable {
private static final Logger LOGGER = LoggerFactory.getLogger(Communication.class);
Expand Down Expand Up @@ -57,8 +59,7 @@ private CompletableFuture<InternalResponse> doExecuteAsync(
final InternalRequest request, final HostHandle hostHandle, final Host host, final int attemptCount, Connection connection, long reqId
) {
if (LOGGER.isDebugEnabled()) {
String body = request.getBody() == null ? "" : serde.toJsonString(request.getBody());
LOGGER.debug("Send Request [id={}]: {} {}", reqId, request, body);
LOGGER.debug("Send Request [id={}]: {} {}", reqId, request, toJsonString(serde, request.getBody()));
}
final CompletableFuture<InternalResponse> rfuture = new CompletableFuture<>();
try {
Expand All @@ -84,8 +85,7 @@ private CompletableFuture<InternalResponse> doExecuteAsync(
handleException(isSafe(request), e, hostHandle, request, host, reqId, attemptCount, rfuture);
} else {
if (LOGGER.isDebugEnabled()) {
String body = response.getBody() == null ? "" : serde.toJsonString(response.getBody());
LOGGER.debug("Received Response [id={}]: {} {}", reqId, response, body);
LOGGER.debug("Received Response [id={}]: {} {}", reqId, response, toJsonString(serde, response.getBody()));
}
ArangoDBException errorEntityEx = ResponseUtils.translateError(serde, response);
if (errorEntityEx instanceof ArangoDBRedirectException) {
Expand Down
60 changes: 48 additions & 12 deletions core/src/main/java/com/arangodb/internal/util/ResponseUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
import com.arangodb.internal.net.ArangoDBUnavailableException;
import com.arangodb.internal.serde.InternalSerde;

import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.concurrent.TimeoutException;

/**
Expand All @@ -38,12 +40,14 @@ public final class ResponseUtils {
private static final int ERROR_STATUS = 300;
private static final int ERROR_INTERNAL = 503;
private static final String HEADER_ENDPOINT = "x-arango-endpoint";
private static final String CONTENT_TYPE = "content-type";
private static final String TEXT_PLAIN = "text/plain";

private ResponseUtils() {
super();
}

public static ArangoDBException translateError(final InternalSerde util, final InternalResponse response) {
public static ArangoDBException translateError(InternalSerde serde, InternalResponse response) {
final int responseCode = response.getResponseCode();
if (responseCode < ERROR_STATUS) {
return null;
Expand All @@ -52,17 +56,49 @@ public static ArangoDBException translateError(final InternalSerde util, final I
return new ArangoDBRedirectException(String.format("Response Code: %s", responseCode),
response.getMeta(HEADER_ENDPOINT));
}
if (response.getBody() != null) {
final ErrorEntity errorEntity = util.deserialize(response.getBody(), ErrorEntity.class);
if (errorEntity.getCode() == ERROR_INTERNAL && errorEntity.getErrorNum() == ERROR_INTERNAL) {
return ArangoDBUnavailableException.from(errorEntity);
}
ArangoDBException e = new ArangoDBException(errorEntity);
if (ArangoErrors.QUEUE_TIME_VIOLATED.equals(e.getErrorNum())) {
return ArangoDBException.of(new TimeoutException().initCause(e));
}
return e;

byte[] body = response.getBody();
if (body == null) {
return new ArangoDBException(String.format("Response Code: %s", responseCode), responseCode);
}

if (isTextPlain(response)) {
String payload = new String(body, getContentTypeCharset(response));
return new ArangoDBException("Response Code: " + responseCode + "[" + payload + "]", responseCode);
}

ErrorEntity errorEntity;
try {
errorEntity = serde.deserialize(body, ErrorEntity.class);
} catch (Exception e) {
ArangoDBException adbEx = new ArangoDBException("Response Code: " + responseCode
+ " [Unparsable data] Response: " + response, responseCode);
adbEx.addSuppressed(e);
return adbEx;
}

if (errorEntity.getCode() == ERROR_INTERNAL && errorEntity.getErrorNum() == ERROR_INTERNAL) {
return ArangoDBUnavailableException.from(errorEntity);
}
return new ArangoDBException(String.format("Response Code: %s", responseCode), responseCode);
ArangoDBException e = new ArangoDBException(errorEntity);
if (ArangoErrors.QUEUE_TIME_VIOLATED.equals(e.getErrorNum())) {
return ArangoDBException.of(new TimeoutException().initCause(e));
}
return e;
}

private static boolean isTextPlain(InternalResponse response) {
String contentType = response.getMeta(CONTENT_TYPE);
return contentType != null && contentType.startsWith(TEXT_PLAIN);
}

private static Charset getContentTypeCharset(InternalResponse response) {
String contentType = response.getMeta(CONTENT_TYPE);
int paramIdx = contentType.indexOf("charset=");
if (paramIdx == -1) {
return StandardCharsets.UTF_8;
}
return Charset.forName(contentType.substring(paramIdx + 8));
}

}
19 changes: 19 additions & 0 deletions core/src/main/java/com/arangodb/internal/util/SerdeUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package com.arangodb.internal.util;

import com.arangodb.internal.serde.InternalSerde;

public class SerdeUtils {
private SerdeUtils() {
}

public static String toJsonString(InternalSerde serde, byte[] data) {
if (data == null) {
return "";
}
try {
return serde.toJsonString(data);
} catch (Exception e) {
return "[Unparsable data]";
}
}
}
93 changes: 93 additions & 0 deletions test-resilience/src/test/java/resilience/http/MockTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,22 @@

import ch.qos.logback.classic.Level;
import com.arangodb.ArangoDB;
import com.arangodb.ArangoDBException;
import com.arangodb.Protocol;
import com.arangodb.internal.net.Communication;
import com.fasterxml.jackson.core.JsonParseException;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockserver.integration.ClientAndServer;
import org.mockserver.matchers.Times;
import resilience.SingleServerTest;

import java.util.Collections;
import java.util.concurrent.ExecutionException;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.catchThrowable;
import static org.mockserver.integration.ClientAndServer.startClientAndServer;
import static org.mockserver.model.HttpRequest.request;
import static org.mockserver.model.HttpResponse.response;
Expand All @@ -22,6 +27,10 @@ class MockTest extends SingleServerTest {
private ClientAndServer mockServer;
private ArangoDB arangoDB;

public MockTest() {
super(Collections.singletonMap(Communication.class, Level.DEBUG));
}

@BeforeEach
void before() {
mockServer = startClientAndServer(getEndpoint().getHost(), getEndpoint().getPort());
Expand Down Expand Up @@ -85,4 +94,88 @@ void retryOn503Async() throws ExecutionException, InterruptedException {
.filteredOn(e -> e.getLevel().equals(Level.WARN))
.anyMatch(e -> e.getFormattedMessage().contains("Could not connect to host"));
}

@Test
void unparsableData() {
arangoDB.getVersion();

mockServer
.when(
request()
.withMethod("GET")
.withPath("/.*/_api/version")
)
.respond(
response()
.withStatusCode(504)
.withBody("upstream timed out")
);

logs.reset();
Throwable thrown = catchThrowable(() -> arangoDB.getVersion());
assertThat(thrown)
.isInstanceOf(ArangoDBException.class)
.hasMessageContaining("[Unparsable data]")
.hasMessageContaining("Response: {statusCode=504,");
Throwable[] suppressed = thrown.getCause().getSuppressed();
assertThat(suppressed).hasSize(1);
assertThat(suppressed[0])
.isInstanceOf(ArangoDBException.class)
.cause()
.isInstanceOf(JsonParseException.class);
assertThat(logs.getLogs())
.filteredOn(e -> e.getLevel().equals(Level.DEBUG))
.anySatisfy(e -> assertThat(e.getFormattedMessage())
.contains("Received Response")
.contains("statusCode=504")
.contains("[Unparsable data]")
);
}

@Test
void textPlainData() {
arangoDB.getVersion();

mockServer
.when(
request()
.withMethod("GET")
.withPath("/.*/_api/version")
)
.respond(
response()
.withStatusCode(504)
.withHeader("Content-Type", "text/plain")
.withBody("upstream timed out")
);

Throwable thrown = catchThrowable(() -> arangoDB.getVersion());
assertThat(thrown)
.isInstanceOf(ArangoDBException.class)
.hasMessageContaining("upstream timed out");
}

@Test
void textPlainDataWithCharset() {
arangoDB.getVersion();

mockServer
.when(
request()
.withMethod("GET")
.withPath("/.*/_api/version")
)
.respond(
response()
.withStatusCode(504)
.withHeader("Content-Type", "text/plain; charset=utf-8")
.withBody("upstream timed out")
);

Throwable thrown = catchThrowable(() -> arangoDB.getVersion());
assertThat(thrown)
.isInstanceOf(ArangoDBException.class)
.hasMessageContaining("upstream timed out");
}

}