Skip to content

Commit

Permalink
Replicate the RestClient exception wrapping for async invocation flow
Browse files Browse the repository at this point in the history 
Signed-off-by: Andriy Redko <[email protected]>
  • Loading branch information
@reta
reta committed Oct 27, 2023
1 parent cca7890 commit 03b214f
Show file tree
Hide file tree
Showing 9 changed files with 132 additions and 131 deletions.
93 changes: 83 additions & 10 deletions ...src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5Transport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.ConnectException;
import java.net.SocketTimeoutException;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.ArrayList;
Expand All @@ -35,12 +37,15 @@
import java.util.concurrent.CompletionException;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.zip.GZIPOutputStream;
import javax.annotation.Nullable;
import javax.net.ssl.SSLHandshakeException;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.hc.client5.http.ConnectTimeoutException;
import org.apache.hc.client5.http.auth.AuthCache;
import org.apache.hc.client5.http.auth.AuthScheme;
import org.apache.hc.client5.http.auth.AuthScope;
Expand All @@ -55,6 +60,7 @@
import org.apache.hc.client5.http.protocol.HttpClientContext;
import org.apache.hc.core5.concurrent.FutureCallback;
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.ConnectionClosedException;
import org.apache.hc.core5.http.ContentType;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpEntity;
Expand Down Expand Up @@ -141,17 +147,16 @@ public <RequestT, ResponseT, ErrorT> ResponseT performRequest(
TransportOptions options
) throws IOException {
try {
return performRequestAsync(request, endpoint, options).join();
} catch (final CompletionException ex) {
Throwable cause = ex.getCause();
if (cause != null) {
if (cause instanceof IOException) {
throw (IOException) cause;
} else {
throw new IOException(cause);
}
return performRequestAsync(request, endpoint, options).get();
} catch (final Exception ex) {
Exception cause = extractAndWrapCause(ex);
if (cause instanceof IOException) {
throw (IOException) cause;
}
if (cause instanceof RuntimeException) {
throw (RuntimeException) cause;
}
throw new IOException(ex);
throw new IllegalStateException("unexpected exception type: must be either RuntimeException or IOException", cause);
}
}

Expand Down Expand Up @@ -1015,4 +1020,72 @@ public InputStream asInput() {
return new ByteArrayInputStream(this.buf, 0, this.count);
}
}

/**
* Wrap the exception so the caller's signature shows up in the stack trace, taking care to copy the original type and message
* where possible so async and sync code don't have to check different exceptions.
*/
private static Exception extractAndWrapCause(Exception exception) {
if (exception instanceof InterruptedException) {
throw new RuntimeException("thread waiting for the response was interrupted", exception);
}
if (exception instanceof ExecutionException) {
ExecutionException executionException = (ExecutionException) exception;
Throwable t = executionException.getCause() == null ? executionException : executionException.getCause();
if (t instanceof Error) {
throw (Error) t;
}
exception = (Exception) t;
}
if (exception instanceof ConnectTimeoutException) {
ConnectTimeoutException e = new ConnectTimeoutException(exception.getMessage());
e.initCause(exception);
return e;
}
if (exception instanceof SocketTimeoutException) {
SocketTimeoutException e = new SocketTimeoutException(exception.getMessage());
e.initCause(exception);
return e;
}
if (exception instanceof ConnectionClosedException) {
ConnectionClosedException e = new ConnectionClosedException(exception.getMessage());
e.initCause(exception);
return e;
}
if (exception instanceof SSLHandshakeException) {
SSLHandshakeException e = new SSLHandshakeException(
exception.getMessage() + "\nSee https://opensearch.org/docs/latest/clients/java/ for troubleshooting."
);
e.initCause(exception);
return e;
}
if (exception instanceof ConnectException) {
ConnectException e = new ConnectException(exception.getMessage());
e.initCause(exception);
return e;
}
if (exception instanceof ResponseException) {
try {
ResponseException e = new ResponseException(((ResponseException) exception).getResponse());
e.initCause(exception);
return e;
} catch (final IOException ex) {
// We are not able to reconstruct the response, throw IOException instead
return new IOException(exception.getMessage(), exception);
}
}
if (exception instanceof IOException) {
return new IOException(exception.getMessage(), exception);
}
if (exception instanceof OpenSearchException) {
final OpenSearchException e = new OpenSearchException(((OpenSearchException) exception).response());
e.initCause(exception);
return e;
}
if (exception instanceof RuntimeException) {
return new RuntimeException(exception.getMessage(), exception);
}
return new RuntimeException("error while performing request", exception);
}

}
4 changes: 2 additions & 2 deletions .../src/test/java/org/opensearch/client/opensearch/integTest/AbstractAsyncStracktraceIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ public void testFailureFromClientPreservesStacktraceOfCaller() throws Exception
var thrown = assertThrows(Exception.class, () -> javaClient().indices().get(g -> g.index("nonexisting-index")));

var stacktraceElements = Throwables.toStringList(thrown);
var someElementContainsCallerMethodName =
stacktraceElements.stream().anyMatch(it -> it.contains("testFailureFromClientPreservesStacktraceOfCaller"));
var someElementContainsCallerMethodName = stacktraceElements.stream()
.anyMatch(it -> it.contains("testFailureFromClientPreservesStacktraceOfCaller"));

assertTrue(someElementContainsCallerMethodName);
}
Expand Down
15 changes: 4 additions & 11 deletions ...ent/src/test/java/org/opensearch/client/opensearch/integTest/AbstractClusterClientIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,17 +95,10 @@ public void testClusterUpdateSettingNonExistent() throws IOException {
try {
openSearchClient.cluster().putSettings(request);
fail();
} catch (OpenSearchException | IOException e) {
OpenSearchException openSearchException;
if (e instanceof OpenSearchException) {
openSearchException = (OpenSearchException) e;
} else {
assertTrue(e.getCause() instanceof OpenSearchException);
openSearchException = (OpenSearchException) e.getCause();
}

assertEquals(openSearchException.response().status(), 400);
assertTrue(openSearchException.getMessage().contains("transient setting [no_idea_what_you_are_talking_about], not recognized"));
} catch (OpenSearchException e) {
assertNotNull(e);
assertEquals(e.response().status(), 400);
assertTrue(e.getMessage().contains("transient setting [no_idea_what_you_are_talking_about], not recognized"));
}
}

Expand Down
35 changes: 9 additions & 26 deletions java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,21 +118,12 @@ public void testSourceExists() throws IOException {
public void testGet() throws IOException {

{
Exception exception = expectThrows(
Exception.class,
() -> javaClient().get(new GetRequest.Builder().index("index").id("id").build(), String.class)
OpenSearchException exception = expectThrows(
OpenSearchException.class,
() -> javaClient().get(new GetRequest.Builder().index("index").id("id").build(), String.class)
);
OpenSearchException openSearchException;
if (exception instanceof OpenSearchException) {
openSearchException = (OpenSearchException) exception;
} else {
assertTrue(exception.getCause() instanceof OpenSearchException);
openSearchException = (OpenSearchException) exception.getCause();
}

assertEquals(404, openSearchException.status());
assertEquals("Request failed: [index_not_found_exception] no such index [index]",
openSearchException.getMessage());
assertEquals(404, exception.status());
assertEquals("Request failed: [index_not_found_exception] no such index [index]", exception.getMessage());
}

AppData appData = new AppData();
Expand Down Expand Up @@ -216,20 +207,12 @@ public void testUpdate() throws Exception {
.build();
try {
javaClient().update(updateRequest, AppData.class);
} catch (OpenSearchException | IOException e) {
OpenSearchException openSearchException;
if (e instanceof OpenSearchException) {
openSearchException = (OpenSearchException) e;
} else {
assertTrue(e.getCause() instanceof OpenSearchException);
openSearchException = (OpenSearchException) e.getCause();
}

} catch (OpenSearchException e) {
// 1.x: [document_missing_exception] [_doc][does_not_exist]: document missing
// 2.x: [document_missing_exception] [does_not_exist]: document missing
assertTrue(openSearchException.getMessage().contains("[document_missing_exception]"));
assertTrue(openSearchException.getMessage().contains("[does_not_exist]: document missing"));
assertEquals(404, openSearchException.status());
assertTrue(e.getMessage().contains("[document_missing_exception]"));
assertTrue(e.getMessage().contains("[does_not_exist]: document missing"));
assertEquals(404, e.status());
}
}
{
Expand Down
70 changes: 17 additions & 53 deletions ...ent/src/test/java/org/opensearch/client/opensearch/integTest/AbstractIndicesClientIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,10 @@ public void testIndicesExists() throws IOException {
try {
javaClient().indices().get(request);
fail(); // should never execute
} catch (OpenSearchException | IOException ex) {
OpenSearchException openSearchException;
if (ex instanceof OpenSearchException) {
openSearchException = (OpenSearchException) ex;
} else {
assertTrue(ex.getCause() instanceof OpenSearchException);
openSearchException = (OpenSearchException) ex.getCause();
}

assertEquals(openSearchException.status(), 404);
assertEquals(openSearchException.getMessage(),
"Request failed: [index_not_found_exception] " +
"no such index [non_existent_index]");
} catch (OpenSearchException ex) {
assertNotNull(ex);
assertEquals(ex.status(), 404);
assertEquals(ex.getMessage(), "Request failed: [index_not_found_exception] " + "no such index [non_existent_index]");
}
}

Expand All @@ -80,10 +71,8 @@ public void testIndicesExists() throws IOException {
try {
javaClient().indices().get(request);
fail(); // should never execute
} catch (OpenSearchException | IOException ex) {
if (!(ex instanceof OpenSearchException)) {
assertTrue(ex.getCause() instanceof OpenSearchException);
}
} catch (OpenSearchException ex) {
assertNotNull(ex);
}
}
}
Expand Down Expand Up @@ -113,19 +102,10 @@ public void testGetSettingsNonExistentIndex() throws IOException {
try {
javaClient().indices().getSettings(getIndicesSettingsRequest);
fail();
} catch (OpenSearchException | IOException ex) {
OpenSearchException openSearchException;
if (ex instanceof OpenSearchException) {
openSearchException = (OpenSearchException) ex;
} else {
assertTrue(ex.getCause() instanceof OpenSearchException);
openSearchException = (OpenSearchException) ex.getCause();
}

assertEquals(openSearchException.status(), 404);
assertEquals(openSearchException.getMessage(),
"Request failed: [index_not_found_exception] " +
"no such index [index_that_doesnt_exist]");
} catch (OpenSearchException ex) {
assertNotNull(ex);
assertEquals(ex.status(), 404);
assertEquals(ex.getMessage(), "Request failed: [index_not_found_exception] " + "no such index [index_that_doesnt_exist]");
}
}

Expand Down Expand Up @@ -194,16 +174,9 @@ public void testDataStream() throws IOException {
try {
javaClient().indices().getDataStream(b -> b.name(dataStreamName));
fail();
} catch (OpenSearchException | IOException ex) {
OpenSearchException openSearchException;
if (ex instanceof OpenSearchException) {
openSearchException = (OpenSearchException) ex;
} else {
assertTrue(ex.getCause() instanceof OpenSearchException);
openSearchException = (OpenSearchException) ex.getCause();
}

assertEquals(openSearchException.status(), 404);
} catch (OpenSearchException ex) {
assertNotNull(ex);
assertEquals(ex.status(), 404);
}
}

Expand All @@ -213,19 +186,10 @@ public void testGetNotExistingIndexAlias() throws Exception {
try {
GetAliasResponse response = javaClient().indices().getAlias(aliasRequest);
fail();
} catch (OpenSearchException | IOException ex) {
OpenSearchException openSearchException;
if (ex instanceof OpenSearchException) {
openSearchException = (OpenSearchException) ex;
} else {
assertTrue(ex.getCause() instanceof OpenSearchException);
openSearchException = (OpenSearchException) ex.getCause();
}

assertEquals(openSearchException.status(), 404);
assertEquals(openSearchException.getMessage(),
"Request failed: [string_error] " +
"alias [alias_not_exists] missing");
} catch (OpenSearchException ex) {
assertNotNull(ex);
assertEquals(ex.status(), 404);
assertEquals(ex.getMessage(), "Request failed: [string_error] " + "alias [alias_not_exists] missing");
}
}
}
26 changes: 8 additions & 18 deletions java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractRequestIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -419,32 +419,22 @@ public void errorResponse() throws Exception {
BooleanResponse exists = javaClient().exists(_0 -> _0.index("doesnotexist").id("reallynot"));
assertFalse(exists.value());

Exception ex = assertThrows(Exception.class, () -> {
GetResponse<String> response = javaClient().get(
_0 -> _0.index("doesnotexist").id("reallynot"), String.class
);
OpenSearchException ex = assertThrows(OpenSearchException.class, () -> {
GetResponse<String> response = javaClient().get(_0 -> _0.index("doesnotexist").id("reallynot"), String.class);
});

OpenSearchException openSearchException;
if (ex instanceof OpenSearchException) {
openSearchException = (OpenSearchException) ex;
} else {
assertTrue(ex.getCause() instanceof OpenSearchException);
openSearchException = (OpenSearchException) ex.getCause();
}

assertEquals(404, openSearchException.status());
assertEquals("index_not_found_exception", openSearchException.error().type());
assertEquals("doesnotexist", openSearchException.error().metadata().get("index").to(String.class));
assertEquals(404, ex.status());
assertEquals("index_not_found_exception", ex.error().type());
assertEquals("doesnotexist", ex.error().metadata().get("index").to(String.class));

ExecutionException ee = assertThrows(ExecutionException.class, () -> {
OpenSearchAsyncClient aClient = new OpenSearchAsyncClient(javaClient()._transport());
GetResponse<String> response = aClient.get(_0 -> _0.index("doesnotexist").id("reallynot"), String.class).get();
});

openSearchException = ((OpenSearchException) ee.getCause());
assertEquals(404, openSearchException.status());
assertEquals("index_not_found_exception", openSearchException.error().type());
ex = ((OpenSearchException) ee.getCause());
assertEquals(404, ex.status());
assertEquals("index_not_found_exception", ex.error().type());
}

@Test
Expand Down
3 changes: 1 addition & 2 deletions ...rc/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,4 @@

import org.opensearch.client.opensearch.integTest.AbstractAsyncStracktraceIT;

public class AwsSdk2AsyncStacktraceIT extends AbstractAsyncStracktraceIT implements AwsSdk2AsyncTransportSupport {
}
public class AwsSdk2AsyncStacktraceIT extends AbstractAsyncStracktraceIT implements AwsSdk2AsyncTransportSupport {}
Loading

0 comments on commit 03b214f

Please sign in to comment.