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

HLRC: Fix strict setting exception handling (#37247) 

The LLRC's exception handling for strict mode was previously throwing an
exception the HLRC assumed was an error response. This is not the case
if the result is valid in strict mode, as it will return the proper
response wrapped in an exception with warnings. This commit fixes the
HLRC such that it no longer spews if it encounters a strict LLRC
response.

Closes #37090
This commit is contained in:
Michael Basnight 2019-01-30 11:31:59 -06:00 committed by GitHub
parent e97718245d
commit 2cbc6888a2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 142 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java
View File

@ -1671,15 +1671,16 @@ public class RestHighLevelClient implements Closeable {
Response response = responseException.getResponse();
HttpEntity entity = response.getEntity();
ElasticsearchStatusException elasticsearchException;
RestStatus restStatus = RestStatus.fromCode(response.getStatusLine().getStatusCode());
if (entity == null) {
elasticsearchException = new ElasticsearchStatusException(
responseException.getMessage(), RestStatus.fromCode(response.getStatusLine().getStatusCode()), responseException);
responseException.getMessage(), restStatus, responseException);
} else {
try {
elasticsearchException = parseEntity(entity, BytesRestResponse::errorFromXContent);
elasticsearchException.addSuppressed(responseException);
} catch (Exception e) {
RestStatus restStatus = RestStatus.fromCode(response.getStatusLine().getStatusCode());
elasticsearchException = new ElasticsearchStatusException("Unable to parse response body", restStatus, responseException);
elasticsearchException.addSuppressed(e);
}

73
client/rest-high-level/src/test/java/org/elasticsearch/client/MockRestHighLevelTests.java Normal file
View File

@ -0,0 +1,73 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.client;
import org.apache.http.HttpHost;
import org.apache.http.ProtocolVersion;
import org.apache.http.RequestLine;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.message.BasicRequestLine;
import org.apache.http.message.BasicStatusLine;
import org.elasticsearch.test.ESTestCase;
import org.junit.Before;
import java.io.IOException;
import java.util.Collections;
import java.util.List;
import static org.hamcrest.Matchers.equalTo;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
public class MockRestHighLevelTests extends ESTestCase {
private RestHighLevelClient client;
private static final List<String> WARNINGS = Collections.singletonList("Some Warning");
@Before
private void setupClient() throws IOException {
final RestClient mockClient = mock(RestClient.class);
final Response mockResponse = mock(Response.class);
when(mockResponse.getHost()).thenReturn(new HttpHost("localhost", 9200));
when(mockResponse.getWarnings()).thenReturn(WARNINGS);
ProtocolVersion protocol = new ProtocolVersion("HTTP", 1, 1);
when(mockResponse.getStatusLine()).thenReturn(new BasicStatusLine(protocol, 200, "OK"));
RequestLine requestLine = new BasicRequestLine(HttpGet.METHOD_NAME, "/_blah", protocol);
when(mockResponse.getRequestLine()).thenReturn(requestLine);
WarningFailureException expectedException = new WarningFailureException(mockResponse);
doThrow(expectedException).when(mockClient).performRequest(any());
client = new RestHighLevelClient(mockClient, RestClient::close, Collections.emptyList());
}
public void testWarningFailure() {
WarningFailureException exception = expectThrows(WarningFailureException.class,
() -> client.info(RequestOptions.DEFAULT));
assertThat(exception.getMessage(), equalTo("method [GET], host [http://localhost:9200], URI [/_blah], " +
"status line [HTTP/1.1 200 OK]"));
assertNull(exception.getCause());
assertThat(exception.getResponse().getWarnings(), equalTo(WARNINGS));
}
}

4
client/rest/src/main/java/org/elasticsearch/client/ResponseException.java
View File

@ -32,7 +32,7 @@ import java.util.Locale;
*/
public final class ResponseException extends IOException {
private Response response;
private final Response response;
public ResponseException(Response response) throws IOException {
super(buildMessage(response));
@ -49,7 +49,7 @@ public final class ResponseException extends IOException {
this.response = e.getResponse();
}
private static String buildMessage(Response response) throws IOException {
static String buildMessage(Response response) throws IOException {
String message = String.format(Locale.ROOT,
"method [%s], host [%s], URI [%s], status line [%s]",
response.getRequestLine().getMethod(),

5
client/rest/src/main/java/org/elasticsearch/client/RestClient.java
View File

@ -301,7 +301,7 @@ public class RestClient implements Closeable {
if (isSuccessfulResponse(statusCode) || ignoreErrorCodes.contains(response.getStatusLine().getStatusCode())) {
onResponse(node);
if (thisWarningsHandler.warningsShouldFailRequest(response.getWarnings())) {
listener.onDefinitiveFailure(new ResponseException(response));
listener.onDefinitiveFailure(new WarningFailureException(response));
} else {
listener.onSuccess(response);
}
@ -686,6 +686,9 @@ public class RestClient implements Closeable {
* like the asynchronous API. We wrap the exception so that the caller's
* signature shows up in any exception we throw.
*/
if (exception instanceof WarningFailureException) {
throw new WarningFailureException((WarningFailureException) exception);
}
if (exception instanceof ResponseException) {
throw new ResponseException((ResponseException) exception);
}

58
client/rest/src/main/java/org/elasticsearch/client/WarningFailureException.java Normal file
View File

@ -0,0 +1,58 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.client;
import java.io.IOException;
import static org.elasticsearch.client.ResponseException.buildMessage;
/**
* This exception is used to indicate that one or more {@link Response#getWarnings()} exist
* and is typically used when the {@link RestClient} is set to fail by setting
* {@link RestClientBuilder#setStrictDeprecationMode(boolean)} to `true`.
*/
// This class extends RuntimeException in order to deal with wrapping that is done in FutureUtils on exception.
// if the exception is not of type ElasticsearchException or RuntimeException it will be wrapped in a UncategorizedExecutionException
public final class WarningFailureException extends RuntimeException {
private final Response response;
public WarningFailureException(Response response) throws IOException {
super(buildMessage(response));
this.response = response;
}
/**
* Wrap a {@linkplain WarningFailureException} with another one with the current
* stack trace. This is used during synchronous calls so that the caller
* ends up in the stack trace of the exception thrown.
*/
WarningFailureException(WarningFailureException e) {
super(e.getMessage(), e);
this.response = e.getResponse();
}
/**
* Returns the {@link Response} that caused this exception to be thrown.
*/
public Response getResponse() {
return response;
}
}

4
client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java
View File

@ -421,9 +421,9 @@ public class RestClientSingleHostTests extends RestClientTestCase {
if (expectFailure) {
try {
restClient.performRequest(request);
fail("expected ResponseException from warnings");
fail("expected WarningFailureException from warnings");
return;
} catch (ResponseException e) {
} catch (WarningFailureException e) {
if (false == warningBodyTexts.isEmpty()) {
assertThat(e.getMessage(), containsString("\nWarnings: " + warningBodyTexts));
}