In most cases, if the expected START_OBJECT is not detected, an IllegalArgumentException is thrown :
|
public static void ensureExpectedToken(Token expected, Token actual, Supplier<XContentLocation> location) { |
|
if (actual != expected) { |
|
String message = "Failed to parse object: expecting token of type [%s] but found [%s]"; |
|
throw new ParsingException(location.get(), String.format(Locale.ROOT, message, expected, actual)); |
|
} |
|
} |
However ObjectParser throws IllegalStateException :
|
if (token != XContentParser.Token.START_OBJECT) { |
|
throw new IllegalStateException("[" + name + "] Expected START_OBJECT but was: " + token); |
|
} |
When parsing mget two different exception types may be thrown :
ParsingException
|
if ((token = parser.nextToken()) != XContentParser.Token.START_OBJECT) { |
|
final String message = String.format( |
|
Locale.ROOT, |
|
"unexpected token [%s], expected [%s]", |
|
token, |
|
XContentParser.Token.START_OBJECT); |
|
throw new ParsingException(parser.getTokenLocation(), message); |
|
} |
IllegalArgumentException
|
if (token != XContentParser.Token.START_OBJECT) { |
|
throw new IllegalArgumentException("docs array element should include an object"); |
|
} |
while RepositoryData consistently throws ElasticsearchParseException :
|
throw new ElasticsearchParseException("start object expected"); |
I think that it would be much cleaner if only one exception type is used (at least from now on).
For example what exception type should be thrown when a stricter validation of query field names is introduced (#26013). A different one per request (based on the exception thrown till now in the request)? Or an exception of type X?
Or maybe some of the exception types should be changed? For example can IllegalStateException be replaced by IllegalArgumentException?
In most cases, if the expected
START_OBJECTis not detected, anIllegalArgumentExceptionis thrown :elasticsearch/core/src/main/java/org/elasticsearch/common/xcontent/XContentParserUtils.java
Lines 75 to 80 in b88dbe8
However
ObjectParserthrowsIllegalStateException:elasticsearch/core/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java
Lines 149 to 151 in b88dbe8
When parsing
mgettwo different exception types may be thrown :ParsingExceptionelasticsearch/core/src/main/java/org/elasticsearch/action/get/MultiGetRequest.java
Lines 323 to 330 in b88dbe8
IllegalArgumentExceptionelasticsearch/core/src/main/java/org/elasticsearch/action/get/MultiGetRequest.java
Lines 364 to 366 in b88dbe8
while
RepositoryDataconsistently throwsElasticsearchParseException:elasticsearch/core/src/main/java/org/elasticsearch/repositories/RepositoryData.java
Line 487 in b88dbe8
I think that it would be much cleaner if only one exception type is used (at least from now on).
For example what exception type should be thrown when a stricter validation of query field names is introduced (#26013). A different one per request (based on the exception thrown till now in the request)? Or an exception of type X?
Or maybe some of the exception types should be changed? For example can
IllegalStateExceptionbe replaced byIllegalArgumentException?