Expose whole ElasticsearchResponse within ElasticsearchResponseException

The only small problem is that the response gets closed straightaway and its body read immediately into a string. Should be ok to load it all into memory eagerly though in case of errors. Otherwise it becomes cumbersome to have an exception implement Closeable...
This commit is contained in:
javanna 2016-05-18 13:51:38 +02:00 committed by Luca Cavanna
parent 16ab491016
commit 9a38d81bec
3 changed files with 31 additions and 33 deletions

View File

@ -22,6 +22,7 @@ package org.elasticsearch.client;
import org.apache.http.HttpHost;
import org.apache.http.RequestLine;
import org.apache.http.StatusLine;
import org.apache.http.util.EntityUtils;
import java.io.IOException;
@ -30,17 +31,21 @@ import java.io.IOException;
*/
public class ElasticsearchResponseException extends IOException {
private final HttpHost host;
private final RequestLine requestLine;
private final StatusLine statusLine;
private ElasticsearchResponse elasticsearchResponse;
private final String responseBody;
public ElasticsearchResponseException(RequestLine requestLine, HttpHost host, StatusLine statusLine, String responseBody) {
super(buildMessage(requestLine, host, statusLine));
this.host = host;
this.requestLine = requestLine;
this.responseBody = responseBody;
this.statusLine = statusLine;
public ElasticsearchResponseException(ElasticsearchResponse elasticsearchResponse) throws IOException {
super(buildMessage(elasticsearchResponse.getRequestLine(), elasticsearchResponse.getHost(), elasticsearchResponse.getStatusLine()));
this.elasticsearchResponse = elasticsearchResponse;
try {
if (elasticsearchResponse.getEntity() == null) {
this.responseBody = null;
} else {
this.responseBody = EntityUtils.toString(elasticsearchResponse.getEntity());
}
} finally {
elasticsearchResponse.close();
}
}
private static String buildMessage(RequestLine requestLine, HttpHost host, StatusLine statusLine) {
@ -48,23 +53,17 @@ public class ElasticsearchResponseException extends IOException {
}
/**
* Returns the {@link HttpHost} that returned the error
* Returns the {@link ElasticsearchResponse} that caused this exception to be thrown
*/
public HttpHost getHost() {
return host;
public ElasticsearchResponse getElasticsearchResponse() {
return elasticsearchResponse;
}
/**
* Returns the {@link RequestLine} that triggered the error
* Returns the response body as a string or null if there wasn't any.
* The body is eagerly consumed when an ElasticsearchResponseException gets created, and its corresponding ElasticsearchResponse
* gets closed straightaway so this method is the only way to get back the response body that was returned.
*/
public RequestLine getRequestLine() {
return requestLine;
}
public StatusLine getStatusLine() {
return statusLine;
}
public String getResponseBody() {
return responseBody;
}

View File

@ -34,7 +34,6 @@ import org.apache.http.client.utils.URIBuilder;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
import org.apache.http.util.EntityUtils;
import java.io.Closeable;
import java.io.IOException;
@ -115,20 +114,17 @@ public final class RestClient implements Closeable {
lastSeenException = addSuppressedException(lastSeenException, e);
continue;
}
ElasticsearchResponse elasticsearchResponse = new ElasticsearchResponse(request.getRequestLine(),
connection.getHost(), response);
int statusCode = response.getStatusLine().getStatusCode();
//TODO make ignore status code configurable. rest-spec and tests support that parameter (ignore_missing)
if (statusCode < 300 || (request.getMethod().equals(HttpHead.METHOD_NAME) && statusCode == 404) ) {
RequestLogger.log(logger, "request succeeded", request, connection.getHost(), response);
onSuccess(connection);
return new ElasticsearchResponse(request.getRequestLine(), connection.getHost(), response);
return elasticsearchResponse;
} else {
RequestLogger.log(logger, "request failed", request, connection.getHost(), response);
String responseBody = null;
if (response.getEntity() != null) {
responseBody = EntityUtils.toString(response.getEntity());
}
ElasticsearchResponseException elasticsearchResponseException = new ElasticsearchResponseException(
request.getRequestLine(), connection.getHost(), response.getStatusLine(), responseBody);
ElasticsearchResponseException elasticsearchResponseException = new ElasticsearchResponseException(elasticsearchResponse);
lastSeenException = addSuppressedException(lastSeenException, elasticsearchResponseException);
//clients don't retry on 500 because elasticsearch still misuses it instead of 400 in some places
if (statusCode == 502 || statusCode == 503 || statusCode == 504) {

View File

@ -30,6 +30,7 @@ import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.RecordedRequest;
import org.apache.http.HttpHost;
import org.apache.lucene.util.LuceneTestCase;
import org.elasticsearch.client.ElasticsearchResponse;
import org.elasticsearch.client.ElasticsearchResponseException;
import org.elasticsearch.client.RestClient;
import org.junit.After;
@ -100,15 +101,17 @@ public class HostsSnifferTests extends LuceneTestCase {
assertEquals(sniffedHost, responseHostsIterator.next());
}
} catch(ElasticsearchResponseException e) {
ElasticsearchResponse response = e.getElasticsearchResponse();
if (sniffResponse.isFailure) {
assertThat(e.getMessage(), containsString("GET http://localhost:" + server.getPort() +
"/_nodes/http?timeout=" + sniffRequestTimeout));
assertThat(e.getMessage(), containsString(Integer.toString(sniffResponse.nodesInfoResponseCode)));
assertThat(e.getHost(), equalTo(httpHost));
assertThat(e.getStatusLine().getStatusCode(), equalTo(sniffResponse.nodesInfoResponseCode));
assertThat(e.getRequestLine().toString(), equalTo("GET /_nodes/http?timeout=" + sniffRequestTimeout + "ms HTTP/1.1"));
assertThat(response.getHost(), equalTo(httpHost));
assertThat(response.getStatusLine().getStatusCode(), equalTo(sniffResponse.nodesInfoResponseCode));
assertThat(response.getRequestLine().toString(),
equalTo("GET /_nodes/http?timeout=" + sniffRequestTimeout + "ms HTTP/1.1"));
} else {
fail("sniffNodes should have succeeded: " + e.getStatusLine());
fail("sniffNodes should have succeeded: " + response.getStatusLine());
}
}
}