Skip to content

Commit 21258db

Browse files
alespouralespour
authored
fix: V3 error response handling (#351)
* fix: handle structured error response * test: fix long lineds * test: fix content types * test: add invalid JSON error body test * test: add more error body tests * refactor: paramterized tests * fix: simplify error checks * docs: update CHANGELOG * test: fix content-type overrides in tests * test: more coverage * fix: var shadowing * fix: remove redundant guard * test: coverage * fix: remove never-happen guard * test: coverage
1 parent 9704517 commit 21258db

File tree

6 files changed

+416
-26
lines changed

6 files changed

+416
-26
lines changed

‎CHANGELOG.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@
22

33
### Features
44

5-
1. [#352](https://github.com/InfluxCommunity/influxdb3-java/pull/352): Update Apache Flight client dependencies.
5+
1. [#352](https://github.com/InfluxCommunity/influxdb3-java/pull/352): Upgrade Arrow Flight client dependencies.
6+
7+
### Bug Fixes
8+
9+
1. [#351](https://github.com/InfluxCommunity/influxdb3-java/pull/351): Enterprise/Core structured errors handling.
610

711
### CI
812

‎pom.xml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,13 @@
317317
<scope>test</scope>
318318
</dependency>
319319

320+
<dependency>
321+
<groupId>org.junit.jupiter</groupId>
322+
<artifactId>junit-jupiter-params</artifactId>
323+
<version>${junit-jupiter.version}</version>
324+
<scope>test</scope>
325+
</dependency>
326+
320327
<dependency>
321328
<groupId>org.assertj</groupId>
322329
<artifactId>assertj-core</artifactId>

‎src/main/java/com/influxdb/v3/client/internal/RestClient.java‎

Lines changed: 102 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -217,22 +217,12 @@ HttpResponse<String> request(@Nonnull final String path,
217217

218218
int statusCode = response.statusCode();
219219
if (statusCode < 200 || statusCode >= 300) {
220-
String reason = "";
220+
String reason;
221221
String body = response.body();
222-
if (!body.isEmpty()) {
223-
try {
224-
final JsonNode root = objectMapper.readTree(body);
225-
final List<String> possibilities = List.of("message", "error_message", "error");
226-
for (final String field : possibilities) {
227-
final JsonNode node = root.findValue(field);
228-
if (node != null) {
229-
reason = node.asText();
230-
break;
231-
}
232-
}
233-
} catch (JsonProcessingException e) {
234-
LOG.debug("Can't parse msg from response {}", response);
235-
}
222+
reason = formatErrorMessage(body, response.headers().firstValue("Content-Type").orElse(null));
223+
224+
if (reason == null) {
225+
reason = "";
236226
}
237227

238228
if (reason.isEmpty()) {
@@ -257,6 +247,103 @@ HttpResponse<String> request(@Nonnull final String path,
257247
return response;
258248
}
259249

250+
@Nullable
251+
private String formatErrorMessage(@Nonnull final String body, @Nullable final String contentType) {
252+
if (body.isEmpty()) {
253+
return null;
254+
}
255+
256+
if (contentType != null
257+
&& !contentType.isEmpty()
258+
&& !contentType.regionMatches(true, 0, "application/json", 0, "application/json".length())) {
259+
return null;
260+
}
261+
262+
try {
263+
final JsonNode root = objectMapper.readTree(body);
264+
if (!root.isObject()) {
265+
return null;
266+
}
267+
268+
final String rootMessage = errNonEmptyField(root, "message");
269+
if (rootMessage != null) {
270+
return rootMessage;
271+
}
272+
273+
final String error = errNonEmptyField(root, "error");
274+
final JsonNode dataNode = root.get("data");
275+
276+
// InfluxDB 3 Core/Enterprise write error format:
277+
// {"error":"...","data":[{"error_message":"...","line_number":2,"original_line":"..."}]}
278+
if (error != null && dataNode != null && dataNode.isArray()) {
279+
final StringBuilder message = new StringBuilder(error);
280+
boolean hasDetails = false;
281+
for (JsonNode item : dataNode) {
282+
final String detail = errFormatDataArrayDetail(item);
283+
if (detail == null) {
284+
continue;
285+
}
286+
if (!hasDetails) {
287+
message.append(':');
288+
hasDetails = true;
289+
}
290+
message.append("\n\t").append(detail);
291+
}
292+
return message.toString();
293+
}
294+
295+
// Core/Enterprise object format:
296+
// {"error":"...","data":{"error_message":"..."}}
297+
final String errorMessage = errNonEmptyField(dataNode, "error_message");
298+
if (errorMessage != null) {
299+
return errorMessage;
300+
}
301+
302+
return error;
303+
} catch (JsonProcessingException e) {
304+
LOG.debug("Can't parse msg from response body {}", body, e);
305+
return null;
306+
}
307+
}
308+
309+
@Nullable
310+
private String errNonEmptyText(@Nullable final JsonNode node) {
311+
if (node == null || node.isNull()) {
312+
return null;
313+
}
314+
final String value = node.asText();
315+
return value.isEmpty() ? null : value;
316+
}
317+
318+
@Nullable
319+
private String errNonEmptyField(@Nullable final JsonNode object, @Nonnull final String fieldName) {
320+
if (object == null || !object.isObject()) {
321+
return null;
322+
}
323+
return errNonEmptyText(object.get(fieldName));
324+
}
325+
326+
@Nullable
327+
private String errFormatDataArrayDetail(@Nullable final JsonNode item) {
328+
if (!item.isObject()) {
329+
return null;
330+
}
331+
332+
final String errorMessage = errNonEmptyField(item, "error_message");
333+
if (errorMessage == null) {
334+
return null;
335+
}
336+
337+
if (item.hasNonNull("line_number")) {
338+
final String originalLine = errNonEmptyField(item, "original_line");
339+
if (originalLine != null) {
340+
final String lineNumber = item.get("line_number").asText();
341+
return "line " + lineNumber + ": " + errorMessage + " (" + originalLine + ")";
342+
}
343+
}
344+
return errorMessage;
345+
}
346+
260347
private X509TrustManager getX509TrustManagerFromFile(@Nonnull final String filePath) {
261348
try {
262349
KeyStore trustStore = KeyStore.getInstance(KeyStore.getDefaultType());

‎src/test/java/com/influxdb/v3/client/AbstractMockServerTest.java‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,23 @@ protected MockResponse createResponse(final int responseCode,
6565
@Nullable final Map<String, String> headers,
6666
@Nullable final String body) {
6767

68+
return createResponse(responseCode, "text/csv; charset=utf-8", headers, body);
69+
}
70+
71+
@Nonnull
72+
protected MockResponse createResponse(final int responseCode,
73+
@Nullable final String contentType,
74+
@Nullable final Map<String, String> headers,
75+
@Nullable final String body) {
76+
6877
MockResponse.Builder mrb = new MockResponse.Builder();
6978
mrb.code(responseCode);
70-
Map<String, String> effectiveHeaders = new HashMap<>(Map.of("Content-Type", "text/csv; charset=utf-8",
71-
"Date", "Tue, 26 Jun 2018 13:15:01 GMT"));
79+
Map<String, String> effectiveHeaders = new HashMap<>(Map.of(
80+
"Date", "Tue, 26 Jun 2018 13:15:01 GMT"
81+
));
82+
if (contentType != null) {
83+
effectiveHeaders.put("Content-Type", contentType);
84+
}
7285
if (headers != null) {
7386
effectiveHeaders.putAll(headers);
7487
}
@@ -85,9 +98,6 @@ protected MockResponse createResponse(final int responseCode,
8598
@Nonnull
8699
protected MockResponse createResponse(final int responseCode) {
87100

88-
return createResponse(responseCode, Map.of(
89-
"Content-Type", "text/csv; charset=utf-8",
90-
"Date", "Tue, 26 Jun 2018 13:15:01 GMT"
91-
), null);
101+
return createResponse(responseCode, "text/csv; charset=utf-8", null, null);
92102
}
93103
}

‎src/test/java/com/influxdb/v3/client/InfluxDBClientWriteTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,8 @@ void defaultTags() throws InterruptedException {
517517
@Test
518518
public void retryHandled429Test() {
519519
mockServer.enqueue(createResponse(429,
520-
Map.of("retry-after", "42", "content-type", "application/json"),
520+
"application/json",
521+
Map.of("retry-after", "42"),
521522
"{ \"message\" : \"Too Many Requests\" }"));
522523

523524
Point point = Point.measurement("mem")

0 commit comments

Comments
 (0)