Skip to content

Commit 106c800

Browse files
rashtaorashtao
authored
[DE-960] tolerate error responses with text content-type (#587)
1 parent b769205 commit 106c800

File tree

4 files changed

+164
-16
lines changed

4 files changed

+164
-16
lines changed

Diff for: ‎core/src/main/java/com/arangodb/internal/net/Communication.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import java.util.concurrent.TimeoutException;
2424
import java.util.concurrent.atomic.AtomicLong;
2525

26+
import static com.arangodb.internal.util.SerdeUtils.toJsonString;
27+
2628
@UsedInApi
2729
public abstract class Communication implements Closeable {
2830
private static final Logger LOGGER = LoggerFactory.getLogger(Communication.class);
@@ -57,8 +59,7 @@ private CompletableFuture<InternalResponse> doExecuteAsync(
5759
final InternalRequest request, final HostHandle hostHandle, final Host host, final int attemptCount, Connection connection, long reqId
5860
) {
5961
if (LOGGER.isDebugEnabled()) {
60-
String body = request.getBody() == null ? "" : serde.toJsonString(request.getBody());
61-
LOGGER.debug("Send Request [id={}]: {} {}", reqId, request, body);
62+
LOGGER.debug("Send Request [id={}]: {} {}", reqId, request, toJsonString(serde, request.getBody()));
6263
}
6364
final CompletableFuture<InternalResponse> rfuture = new CompletableFuture<>();
6465
try {
@@ -84,8 +85,7 @@ private CompletableFuture<InternalResponse> doExecuteAsync(
8485
handleException(isSafe(request), e, hostHandle, request, host, reqId, attemptCount, rfuture);
8586
} else {
8687
if (LOGGER.isDebugEnabled()) {
87-
String body = response.getBody() == null ? "" : serde.toJsonString(response.getBody());
88-
LOGGER.debug("Received Response [id={}]: {} {}", reqId, response, body);
88+
LOGGER.debug("Received Response [id={}]: {} {}", reqId, response, toJsonString(serde, response.getBody()));
8989
}
9090
ArangoDBException errorEntityEx = ResponseUtils.translateError(serde, response);
9191
if (errorEntityEx instanceof ArangoDBRedirectException) {

Diff for: ‎core/src/main/java/com/arangodb/internal/util/ResponseUtils.java

+48-12
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import com.arangodb.internal.net.ArangoDBUnavailableException;
2929
import com.arangodb.internal.serde.InternalSerde;
3030

31+
import java.nio.charset.Charset;
32+
import java.nio.charset.StandardCharsets;
3133
import java.util.concurrent.TimeoutException;
3234

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

4246
private ResponseUtils() {
4347
super();
4448
}
4549

46-
public static ArangoDBException translateError(final InternalSerde util, final InternalResponse response) {
50+
public static ArangoDBException translateError(InternalSerde serde, InternalResponse response) {
4751
final int responseCode = response.getResponseCode();
4852
if (responseCode < ERROR_STATUS) {
4953
return null;
@@ -52,17 +56,49 @@ public static ArangoDBException translateError(final InternalSerde util, final I
5256
return new ArangoDBRedirectException(String.format("Response Code: %s", responseCode),
5357
response.getMeta(HEADER_ENDPOINT));
5458
}
55-
if (response.getBody() != null) {
56-
final ErrorEntity errorEntity = util.deserialize(response.getBody(), ErrorEntity.class);
57-
if (errorEntity.getCode() == ERROR_INTERNAL && errorEntity.getErrorNum() == ERROR_INTERNAL) {
58-
return ArangoDBUnavailableException.from(errorEntity);
59-
}
60-
ArangoDBException e = new ArangoDBException(errorEntity);
61-
if (ArangoErrors.QUEUE_TIME_VIOLATED.equals(e.getErrorNum())) {
62-
return ArangoDBException.of(new TimeoutException().initCause(e));
63-
}
64-
return e;
59+
60+
byte[] body = response.getBody();
61+
if (body == null) {
62+
return new ArangoDBException(String.format("Response Code: %s", responseCode), responseCode);
63+
}
64+
65+
if (isTextPlain(response)) {
66+
String payload = new String(body, getContentTypeCharset(response));
67+
return new ArangoDBException("Response Code: " + responseCode + "[" + payload + "]", responseCode);
68+
}
69+
70+
ErrorEntity errorEntity;
71+
try {
72+
errorEntity = serde.deserialize(body, ErrorEntity.class);
73+
} catch (Exception e) {
74+
ArangoDBException adbEx = new ArangoDBException("Response Code: " + responseCode
75+
+ " [Unparsable data] Response: " + response, responseCode);
76+
adbEx.addSuppressed(e);
77+
return adbEx;
78+
}
79+
80+
if (errorEntity.getCode() == ERROR_INTERNAL && errorEntity.getErrorNum() == ERROR_INTERNAL) {
81+
return ArangoDBUnavailableException.from(errorEntity);
6582
}
66-
return new ArangoDBException(String.format("Response Code: %s", responseCode), responseCode);
83+
ArangoDBException e = new ArangoDBException(errorEntity);
84+
if (ArangoErrors.QUEUE_TIME_VIOLATED.equals(e.getErrorNum())) {
85+
return ArangoDBException.of(new TimeoutException().initCause(e));
86+
}
87+
return e;
88+
}
89+
90+
private static boolean isTextPlain(InternalResponse response) {
91+
String contentType = response.getMeta(CONTENT_TYPE);
92+
return contentType != null && contentType.startsWith(TEXT_PLAIN);
6793
}
94+
95+
private static Charset getContentTypeCharset(InternalResponse response) {
96+
String contentType = response.getMeta(CONTENT_TYPE);
97+
int paramIdx = contentType.indexOf("charset=");
98+
if (paramIdx == -1) {
99+
return StandardCharsets.UTF_8;
100+
}
101+
return Charset.forName(contentType.substring(paramIdx + 8));
102+
}
103+
68104
}

Diff for: ‎core/src/main/java/com/arangodb/internal/util/SerdeUtils.java

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package com.arangodb.internal.util;
2+
3+
import com.arangodb.internal.serde.InternalSerde;
4+
5+
public class SerdeUtils {
6+
private SerdeUtils() {
7+
}
8+
9+
public static String toJsonString(InternalSerde serde, byte[] data) {
10+
if (data == null) {
11+
return "";
12+
}
13+
try {
14+
return serde.toJsonString(data);
15+
} catch (Exception e) {
16+
return "[Unparsable data]";
17+
}
18+
}
19+
}

Diff for: ‎test-resilience/src/test/java/resilience/http/MockTest.java

+93
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,22 @@
22

33
import ch.qos.logback.classic.Level;
44
import com.arangodb.ArangoDB;
5+
import com.arangodb.ArangoDBException;
56
import com.arangodb.Protocol;
7+
import com.arangodb.internal.net.Communication;
8+
import com.fasterxml.jackson.core.JsonParseException;
69
import org.junit.jupiter.api.AfterEach;
710
import org.junit.jupiter.api.BeforeEach;
811
import org.junit.jupiter.api.Test;
912
import org.mockserver.integration.ClientAndServer;
1013
import org.mockserver.matchers.Times;
1114
import resilience.SingleServerTest;
1215

16+
import java.util.Collections;
1317
import java.util.concurrent.ExecutionException;
1418

1519
import static org.assertj.core.api.Assertions.assertThat;
20+
import static org.assertj.core.api.Assertions.catchThrowable;
1621
import static org.mockserver.integration.ClientAndServer.startClientAndServer;
1722
import static org.mockserver.model.HttpRequest.request;
1823
import static org.mockserver.model.HttpResponse.response;
@@ -22,6 +27,10 @@ class MockTest extends SingleServerTest {
2227
private ClientAndServer mockServer;
2328
private ArangoDB arangoDB;
2429

30+
public MockTest() {
31+
super(Collections.singletonMap(Communication.class, Level.DEBUG));
32+
}
33+
2534
@BeforeEach
2635
void before() {
2736
mockServer = startClientAndServer(getEndpoint().getHost(), getEndpoint().getPort());
@@ -85,4 +94,88 @@ void retryOn503Async() throws ExecutionException, InterruptedException {
8594
.filteredOn(e -> e.getLevel().equals(Level.WARN))
8695
.anyMatch(e -> e.getFormattedMessage().contains("Could not connect to host"));
8796
}
97+
98+
@Test
99+
void unparsableData() {
100+
arangoDB.getVersion();
101+
102+
mockServer
103+
.when(
104+
request()
105+
.withMethod("GET")
106+
.withPath("/.*/_api/version")
107+
)
108+
.respond(
109+
response()
110+
.withStatusCode(504)
111+
.withBody("upstream timed out")
112+
);
113+
114+
logs.reset();
115+
Throwable thrown = catchThrowable(() -> arangoDB.getVersion());
116+
assertThat(thrown)
117+
.isInstanceOf(ArangoDBException.class)
118+
.hasMessageContaining("[Unparsable data]")
119+
.hasMessageContaining("Response: {statusCode=504,");
120+
Throwable[] suppressed = thrown.getCause().getSuppressed();
121+
assertThat(suppressed).hasSize(1);
122+
assertThat(suppressed[0])
123+
.isInstanceOf(ArangoDBException.class)
124+
.cause()
125+
.isInstanceOf(JsonParseException.class);
126+
assertThat(logs.getLogs())
127+
.filteredOn(e -> e.getLevel().equals(Level.DEBUG))
128+
.anySatisfy(e -> assertThat(e.getFormattedMessage())
129+
.contains("Received Response")
130+
.contains("statusCode=504")
131+
.contains("[Unparsable data]")
132+
);
133+
}
134+
135+
@Test
136+
void textPlainData() {
137+
arangoDB.getVersion();
138+
139+
mockServer
140+
.when(
141+
request()
142+
.withMethod("GET")
143+
.withPath("/.*/_api/version")
144+
)
145+
.respond(
146+
response()
147+
.withStatusCode(504)
148+
.withHeader("Content-Type", "text/plain")
149+
.withBody("upstream timed out")
150+
);
151+
152+
Throwable thrown = catchThrowable(() -> arangoDB.getVersion());
153+
assertThat(thrown)
154+
.isInstanceOf(ArangoDBException.class)
155+
.hasMessageContaining("upstream timed out");
156+
}
157+
158+
@Test
159+
void textPlainDataWithCharset() {
160+
arangoDB.getVersion();
161+
162+
mockServer
163+
.when(
164+
request()
165+
.withMethod("GET")
166+
.withPath("/.*/_api/version")
167+
)
168+
.respond(
169+
response()
170+
.withStatusCode(504)
171+
.withHeader("Content-Type", "text/plain; charset=utf-8")
172+
.withBody("upstream timed out")
173+
);
174+
175+
Throwable thrown = catchThrowable(() -> arangoDB.getVersion());
176+
assertThat(thrown)
177+
.isInstanceOf(ArangoDBException.class)
178+
.hasMessageContaining("upstream timed out");
179+
}
180+
88181
}

0 commit comments

Comments
 (0)