Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HLRC: Fix strict setting exception handling #37247

Merged
merged 9 commits into from
Jan 30, 2019
Original file line number Diff line number Diff line change
Expand Up @@ -1671,15 +1671,16 @@ protected final ElasticsearchStatusException parseResponseException(ResponseExce
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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
*/
public final class ResponseException extends IOException {

private Response response;
private final Response response;

public ResponseException(Response response) throws IOException {
super(buildMessage(response));
Expand All @@ -49,7 +49,7 @@ public ResponseException(Response response) throws 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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ public void completed(HttpResponse httpResponse) {
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);
}
Expand Down Expand Up @@ -686,6 +686,9 @@ Response get() throws IOException {
* 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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -421,9 +421,9 @@ private void assertDeprecationWarnings(List<String> warningHeaderTexts, List<Str
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));
}
Expand Down