honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SQL: Relax RemoteFailure parsing so that it also parses error metadata (elastic/x-pack-elasticsearch#3938) 

The current parsing of errors in the RemoteFailure class is strict and
fails on any field that is not one of [caused_by, reason, root_cause,
stack_trace, type]. Sadly some exceptions adds more headers or
metadata when they are printed as XContent and such fields can't be
easily ignored at parsing time.

This commit changes the RemoteFailure.parseFromResponse() method so that
it parses errors using the same behavior as the high level rest client:
it parses any unknown field as a metadata if it's string value or an
array of string and just ignores and skips everything else without
throwing an exception.

Original commit: elastic/x-pack-elasticsearch@1348706807
This commit is contained in:
Tanguy Leroux 2018-02-14 21:05:23 +01:00 committed by GitHub
parent 89f15ed9d3
commit 00dec27d9f
2 changed files with 148 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

66
plugin/sql/sql-shared-client/src/main/java/org/elasticsearch/xpack/sql/client/shared/RemoteFailure.java
View File

@ -10,16 +10,23 @@ import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.JsonParseException;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;
import java.io.BufferedInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.Reader;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonList;
import static java.util.Collections.unmodifiableList;
import static java.util.Collections.unmodifiableMap;
/**
* A failure that happened on the remote server.
@ -77,13 +84,20 @@ public class RemoteFailure {
private final String reason;
private final String remoteTrace;
private final Map<String, String> headers;
private final Map<String, List<String>> metadata;
private final RemoteFailure cause;
RemoteFailure(String type, String reason, String remoteTrace, Map<String, String> headers, RemoteFailure cause) {
RemoteFailure(String type,
String reason,
String remoteTrace,
Map<String, String> headers,
Map<String, List<String>> metadata,
RemoteFailure cause) {
this.type = type;
this.reason = reason;
this.remoteTrace = remoteTrace;
this.headers = headers;
this.metadata = metadata;
this.cause = cause;
}
@ -110,6 +124,13 @@ public class RemoteFailure {
return headers;
}
/**
* Metadata sent by the remote failure.
*/
public Map<String, List<String>> metadata() {
return metadata;
}
/**
* Cause of the remote failure. Mostly just useful for dbuegging errors that happen to be bugs.
*/
@ -162,6 +183,7 @@ public class RemoteFailure {
String remoteTrace = null;
Map<String, String> headers = emptyMap();
RemoteFailure cause = null;
final Map<String, List<String>> metadata = new LinkedHashMap<>();
JsonToken token;
String fieldName = null;
@ -212,8 +234,7 @@ public class RemoteFailure {
type = parser.getText();
break;
default:
throw new IOException("Expected one of [caused_by, reason, root_cause, stack_trace, type] but got ["
+ fieldName + "]");
metadata.putAll(parseMetadata(parser));
}
}
}
@ -223,7 +244,7 @@ public class RemoteFailure {
if (remoteTrace == null) {
throw new IOException("expected [stack_trace] cannot but didn't see it");
}
return new RemoteFailure(type, reason, remoteTrace, headers, cause);
return new RemoteFailure(type, reason, remoteTrace, headers, metadata, cause);
}
private static Map<String, String> parseHeaders(JsonParser parser) throws IOException {
@ -246,6 +267,41 @@ public class RemoteFailure {
return headers;
}
private static Map<String, List<String>> parseMetadata(final JsonParser parser) throws IOException {
final Map<String, List<String>> metadata = new HashMap<>();
final String currentFieldName = parser.getCurrentName();
JsonToken token = parser.currentToken();
if (token == JsonToken.VALUE_STRING) {
metadata.put(currentFieldName, singletonList(parser.getText()));
} else if (token == JsonToken.START_ARRAY) {
// Parse the array and add each item to the corresponding list of metadata.
// Arrays of objects are not supported yet and just ignored and skipped.
final List<String> values = new ArrayList<>();
while ((token = parser.nextToken()) != JsonToken.END_ARRAY) {
if (token ==JsonToken.VALUE_STRING) {
values.add(parser.getText());
} else {
parser.skipChildren();
}
}
if (values.size() > 0) {
if (metadata.containsKey(currentFieldName)) {
values.addAll(metadata.get(currentFieldName));
}
metadata.put(currentFieldName, unmodifiableList(values));
}
} else {
// Any additional metadata object added by the metadataToXContent method is ignored
// and skipped, so that the parser does not fail on unknown fields. The parser only
// support metadata key-pairs and metadata arrays of values.
parser.skipChildren();
}
return unmodifiableMap(metadata);
}
/**
* Build an error message from a parse failure.
*/
@ -276,6 +332,6 @@ public class RemoteFailure {
parserLocation = " at [line " + parser.getTokenLocation().getLineNr()
+ " col " + parser.getTokenLocation().getColumnNr() + "]";
}
return "Can't parse error from Elasticearch [" + message + "]" + parserLocation + ". " + responseMessage;
return "Can't parse error from Elasticsearch [" + message + "]" + parserLocation + ". " + responseMessage;
}
}

103
plugin/sql/sql-shared-client/src/test/java/org/elasticsearch/xpack/sql/client/shared/RemoteFailureTests.java
View File

@ -11,9 +11,11 @@ import org.elasticsearch.test.ESTestCase;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.util.Arrays;
import java.util.Locale;
import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonList;
import static java.util.Collections.singletonMap;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.startsWith;
@ -57,45 +59,45 @@ public class RemoteFailureTests extends ESTestCase {
failure.headers());
}
public void testNoError() throws IOException {
public void testNoError() {
IOException e = expectThrows(IOException.class, () -> parse("no_error.json"));
assertEquals(
"Can't parse error from Elasticearch [Expected [error] but didn't see it.] at [line 1 col 2]. Response:\n{}",
"Can't parse error from Elasticsearch [Expected [error] but didn't see it.] at [line 1 col 2]. Response:\n{}",
e.getMessage());
}
public void testBogusError() throws IOException {
public void testBogusError() {
IOException e = expectThrows(IOException.class, () -> parse("bogus_error.json"));
assertEquals(
"Can't parse error from Elasticearch [Expected [error] to be an object but was [VALUE_STRING][bogus]] "
"Can't parse error from Elasticsearch [Expected [error] to be an object but was [VALUE_STRING][bogus]] "
+ "at [line 1 col 12]. Response:\n"
+ "{ \"error\": \"bogus\" }",
e.getMessage());
}
public void testNoStack() throws IOException {
public void testNoStack() {
IOException e = expectThrows(IOException.class, () -> parse("no_stack.json"));
assertThat(e.getMessage(),
startsWith("Can't parse error from Elasticearch [expected [stack_trace] cannot but "
startsWith("Can't parse error from Elasticsearch [expected [stack_trace] cannot but "
+ "didn't see it] at [line 5 col 3]. Response:\n{"));
}
public void testNoType() throws IOException {
public void testNoType() {
IOException e = expectThrows(IOException.class, () -> parse("no_type.json"));
assertThat(e.getMessage(),
startsWith("Can't parse error from Elasticearch [expected [type] but didn't see it] at [line 5 col 3]. Response:\n{"));
startsWith("Can't parse error from Elasticsearch [expected [type] but didn't see it] at [line 5 col 3]. Response:\n{"));
}
public void testInvalidJson() throws IOException {
public void testInvalidJson() {
IOException e = expectThrows(IOException.class, () -> parse("invalid_json.txt"));
assertEquals(
"Can't parse error from Elasticearch [Unrecognized token 'I': was expecting 'null', 'true', 'false' or NaN] "
"Can't parse error from Elasticsearch [Unrecognized token 'I': was expecting 'null', 'true', 'false' or NaN] "
+ "at [line 1 col 1]. Response:\n"
+ "I'm not json at all",
e.getMessage());
}
public void testExceptionBuildingParser() throws IOException {
public void testExceptionBuildingParser() {
IOException e = expectThrows(IOException.class, () -> RemoteFailure.parseFromResponse(new InputStream() {
@Override
public int read() throws IOException {
@ -103,22 +105,22 @@ public class RemoteFailureTests extends ESTestCase {
}
}));
assertEquals(
"Can't parse error from Elasticearch [Testing error]. Attempted to include response but failed because [Testing error].",
"Can't parse error from Elasticsearch [Testing error]. Attempted to include response but failed because [Testing error].",
e.getMessage());
}
public void testTotalGarbage() throws IOException {
public void testTotalGarbage() {
IOException e = expectThrows(IOException.class, () ->
RemoteFailure.parseFromResponse(new BytesArray(new byte[] {
(byte) 0xEF, (byte) 0xBB, (byte) 0xBF, // The UTF-8 BOM
(byte) 0xFF // An invalid UTF-8 character
}).streamInput()));
assertThat(e.getMessage(),
startsWith("Can't parse error from Elasticearch [Unrecognized token 'ÿ': "
startsWith("Can't parse error from Elasticsearch [Unrecognized token 'ÿ': "
+ "was expecting ('true', 'false' or 'null')] at [line 1 col 1]. Response:\n"));
}
public void testTooBig() throws IOException {
public void testTooBig() {
StringBuilder tooBig = new StringBuilder(RemoteFailure.MAX_RAW_RESPONSE);
tooBig.append("{\n");
tooBig.append("\"error\" : {\n");
@ -136,11 +138,80 @@ public class RemoteFailureTests extends ESTestCase {
IOException e = expectThrows(IOException.class, () ->
RemoteFailure.parseFromResponse(new BytesArray(tooBig.toString()).streamInput()));
assertEquals(
"Can't parse error from Elasticearch [expected [stack_trace] cannot but didn't see it] "
"Can't parse error from Elasticsearch [expected [stack_trace] cannot but didn't see it] "
+ "at [line 7951 col 1]. Attempted to include response but failed because [Response too large].",
e.getMessage());
}
public void testFailureWithMetadata() throws IOException {
final StringBuilder json = new StringBuilder();
json.append("{");
json.append("\"error\":{");
json.append(" \"root_cause\":[],");
json.append(" \"type\":\"search_phase_execution_exception\",");
json.append(" \"reason\":\"all shards failed\",");
json.append(" \"phase\":\"query\",");
json.append(" \"grouped\":true,");
json.append(" \"failed_shards\":[],");
json.append(" \"stack_trace\":\"Failed to execute phase [query], all shards failed at...\"");
json.append(" },");
json.append(" \"status\":503");
json.append("}");
RemoteFailure failure = RemoteFailure.parseFromResponse(new BytesArray(json.toString()).streamInput());
assertEquals("search_phase_execution_exception", failure.type());
assertEquals("all shards failed", failure.reason());
assertThat(failure.remoteTrace(), containsString("Failed to execute phase [query], all shards failed"));
assertNull(failure.cause());
assertEquals(emptyMap(), failure.headers());
assertNotNull(failure.metadata());
assertEquals(1, failure.metadata().size());
assertEquals(singletonList("query"), failure.metadata().get("phase"));
}
public void testFailureWithMetadataAndRootCause() throws IOException {
final StringBuilder json = new StringBuilder();
json.append("{");
json.append("\"error\":{");
json.append(" \"caused_by\":{");
json.append(" \"type\":\"invalid_index_name_exception\",");
json.append(" \"reason\":\"Invalid index name [_root], must not start with '_'.\",");
json.append(" \"index_uuid\":\"_na_root\",");
json.append(" \"index\":[\"_root\",\"_foo\"],");
json.append(" \"stack_trace\":\"[_root] InvalidIndexNameException[Invalid index name [_root], must not start with '_'.]\"");
json.append(" },");
json.append(" \"type\":\"invalid_index_name_exception\",");
json.append(" \"reason\":\"Invalid index name [_foo], must not start with '_'.\",");
json.append(" \"index_uuid\":\"_na_foo\",");
json.append(" \"index\":[\"_foo\"],");
json.append(" \"stack_trace\":\"[_foo] InvalidIndexNameException[Invalid index name [_foo], must not start with '_'.]\"");
json.append(" },");
json.append(" \"status\":400");
json.append("}");
RemoteFailure failure = RemoteFailure.parseFromResponse(new BytesArray(json.toString()).streamInput());
assertEquals("invalid_index_name_exception", failure.type());
assertEquals("Invalid index name [_foo], must not start with '_'.", failure.reason());
assertThat(failure.remoteTrace(),
containsString("[_foo] InvalidIndexNameException[Invalid index name [_foo], must not start with '_'.]"));
assertEquals(emptyMap(), failure.headers());
assertNotNull(failure.metadata());
assertEquals(2, failure.metadata().size());
assertEquals(singletonList("_na_foo"), failure.metadata().get("index_uuid"));
assertEquals(singletonList("_foo"), failure.metadata().get("index"));
RemoteFailure cause = failure.cause();
assertEquals("invalid_index_name_exception", cause.type());
assertEquals("Invalid index name [_root], must not start with '_'.", cause.reason());
assertThat(cause.remoteTrace(),
containsString("[_root] InvalidIndexNameException[Invalid index name [_root], must not start with '_'.]"));
assertEquals(emptyMap(), failure.headers());
assertNotNull(cause.metadata());
assertEquals(2, cause.metadata().size());
assertEquals(singletonList("_na_root"), cause.metadata().get("index_uuid"));
assertEquals(Arrays.asList("_root", "_foo"), cause.metadata().get("index"));
}
private RemoteFailure parse(String fileName) throws IOException {
try (InputStream in = Files.newInputStream(getDataPath("/remote_failure/" + fileName))) {
return RemoteFailure.parseFromResponse(in);