From cc858f503ea7b55a11a5bdc9876047254e3227c1 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Fri, 6 Oct 2023 15:59:20 -0400 Subject: [PATCH 01/23] wrap exceptions thrown by async transport implementations in IOException Signed-off-by: Andrew Parmet --- .../transport/aws/AwsSdk2Transport.java | 14 ++---- .../ApacheHttpClient5Transport.java | 4 +- .../integTest/AbstractAsyncStracktraceIT.java | 20 ++++++++ .../aws/AwsSdk2AsyncStacktraceIT.java | 6 +++ .../aws/AwsSdk2AsyncTransportSupport.java | 46 +++++++++++++++++++ .../httpclient5/AsyncStacktraceIT.java | 6 +++ 6 files changed, 83 insertions(+), 13 deletions(-) create mode 100644 java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractAsyncStracktraceIT.java create mode 100644 java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java create mode 100644 java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java create mode 100644 java-client/src/test/java/org/opensearch/client/opensearch/integTest/httpclient5/AsyncStacktraceIT.java diff --git a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java index 2e4145f3bb..eb3274a368 100644 --- a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java +++ b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java @@ -194,17 +194,11 @@ public ResponseT performRequest( try { return executeAsync((SdkAsyncHttpClient) httpClient, clientReq, requestBody, endpoint, options).get(); } catch (ExecutionException e) { - Throwable cause = e.getCause(); - if (cause != null) { - if (cause instanceof IOException) { - throw (IOException) cause; - } - if (cause instanceof RuntimeException) { - throw (RuntimeException) cause; - } - throw new RuntimeException(cause); + if (e.getCause() instanceof IOException) { + throw (IOException) e.getCause(); + } else { + throw new IOException(e.getCause()); } - throw new RuntimeException(e); } catch (InterruptedException e) { throw new IOException("HttpRequest was interrupted", e); } diff --git a/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5Transport.java b/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5Transport.java index c4142d7f85..8d3b14c9dd 100644 --- a/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5Transport.java +++ b/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5Transport.java @@ -143,9 +143,7 @@ public ResponseT performRequest( try { return performRequestAsync(request, endpoint, options).join(); } catch (final CompletionException ex) { - if (ex.getCause() instanceof RuntimeException) { - throw (RuntimeException) ex.getCause(); - } else if (ex.getCause() instanceof IOException) { + if (ex.getCause() instanceof IOException) { throw (IOException) ex.getCause(); } else { throw new IOException(ex.getCause()); diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractAsyncStracktraceIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractAsyncStracktraceIT.java new file mode 100644 index 0000000000..8f17127449 --- /dev/null +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractAsyncStracktraceIT.java @@ -0,0 +1,20 @@ +package org.opensearch.client.opensearch.integTest; + +import org.apache.logging.log4j.core.util.Throwables; +import org.junit.Test; + +public abstract class AbstractAsyncStracktraceIT extends OpenSearchJavaClientTestCase { + @Test + public void testFailureFromClientPreservesStacktraceOfCaller() throws Exception { + var thrown = assertThrows(Exception.class, () -> javaClient().indices().get(g -> g.index("nonexisting-index"))); + + var stacktraceElements = Throwables.toStringList(thrown); + + stacktraceElements.forEach(System.out::println); + + var someElementContainsCallerMethodName = + stacktraceElements.stream().anyMatch(it -> it.contains("testFailureFromClientPreservesStacktraceOfCaller")); + + assertTrue(someElementContainsCallerMethodName); + } +} diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java new file mode 100644 index 0000000000..56703ad271 --- /dev/null +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java @@ -0,0 +1,6 @@ +package org.opensearch.client.opensearch.integTest.aws; + +import org.opensearch.client.opensearch.integTest.AbstractAsyncStracktraceIT; + +public class AwsSdk2AsyncStacktraceIT extends AbstractAsyncStracktraceIT implements AwsSdk2AsyncTransportSupport { +} diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java new file mode 100644 index 0000000000..c1395f51e2 --- /dev/null +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java @@ -0,0 +1,46 @@ +package org.opensearch.client.opensearch.integTest.aws; + +import org.apache.hc.core5.http.HttpHost; +import org.opensearch.client.opensearch.integTest.OpenSearchTransportSupport; +import org.opensearch.client.transport.OpenSearchTransport; +import org.opensearch.client.transport.aws.AwsSdk2Transport; +import org.opensearch.client.transport.aws.AwsSdk2TransportOptions; +import org.opensearch.common.settings.Settings; +import software.amazon.awssdk.http.async.SdkAsyncHttpClient; +import software.amazon.awssdk.http.crt.AwsCrtAsyncHttpClient; +import software.amazon.awssdk.regions.Region; + +import java.io.IOException; + +interface AwsSdk2AsyncTransportSupport extends OpenSearchTransportSupport { + @Override + default OpenSearchTransport buildTransport(Settings settings, HttpHost[] hosts) throws IOException { + return new AwsSdk2Transport( + getAsyncHttpClient(), + getTestClusterHost(), + getTestClusterServiceName(), + getTestClusterRegion(), + getTransportOptions().build()); + } + + private String getTestClusterHost() { + return System.getProperty("tests.awsSdk2support.domainHost"); + } + + private String getTestClusterServiceName() { + return System.getProperty("tests.awsSdk2support.serviceName"); + } + + private Region getTestClusterRegion() { + String region = System.getProperty("tests.awsSdk2support.domainRegion"); + return region != null ? Region.of(region) : Region.US_EAST_1; + } + + private AwsSdk2TransportOptions.Builder getTransportOptions() { + return AwsSdk2TransportOptions.builder(); + } + + private SdkAsyncHttpClient getAsyncHttpClient() { + return AwsCrtAsyncHttpClient.create(); + } +} diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/httpclient5/AsyncStacktraceIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/httpclient5/AsyncStacktraceIT.java new file mode 100644 index 0000000000..2143180840 --- /dev/null +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/httpclient5/AsyncStacktraceIT.java @@ -0,0 +1,6 @@ +package org.opensearch.client.opensearch.integTest.httpclient5; + +import org.opensearch.client.opensearch.integTest.AbstractAsyncStracktraceIT; + +public class AsyncStacktraceIT extends AbstractAsyncStracktraceIT implements HttpClient5TransportSupport { +} From d93401fa32167bf67c0c90cb6fd352e6d0f62b92 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Fri, 6 Oct 2023 16:01:59 -0400 Subject: [PATCH 02/23] license headers Signed-off-by: Andrew Parmet --- .../opensearch/integTest/AbstractAsyncStracktraceIT.java | 8 ++++++++ .../integTest/aws/AwsSdk2AsyncStacktraceIT.java | 8 ++++++++ .../integTest/httpclient5/AsyncStacktraceIT.java | 8 ++++++++ 3 files changed, 24 insertions(+) diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractAsyncStracktraceIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractAsyncStracktraceIT.java index 8f17127449..841661d696 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractAsyncStracktraceIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractAsyncStracktraceIT.java @@ -1,3 +1,11 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + package org.opensearch.client.opensearch.integTest; import org.apache.logging.log4j.core.util.Throwables; diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java index 56703ad271..460044ce8b 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java @@ -1,3 +1,11 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + package org.opensearch.client.opensearch.integTest.aws; import org.opensearch.client.opensearch.integTest.AbstractAsyncStracktraceIT; diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/httpclient5/AsyncStacktraceIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/httpclient5/AsyncStacktraceIT.java index 2143180840..12ee26d9b0 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/httpclient5/AsyncStacktraceIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/httpclient5/AsyncStacktraceIT.java @@ -1,3 +1,11 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + package org.opensearch.client.opensearch.integTest.httpclient5; import org.opensearch.client.opensearch.integTest.AbstractAsyncStracktraceIT; From b75bfede969d549b2140cb6d7e919d563cd10a1b Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Fri, 6 Oct 2023 16:03:54 -0400 Subject: [PATCH 03/23] changelog Signed-off-by: Andrew Parmet --- CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 84398bbad5..ba71099b4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,7 +35,8 @@ This section is for maintaining a changelog for all breaking changes for the cli ### Dependencies ### Changed -Allow null values in arrays ([#687](https://github.com/opensearch-project/opensearch-java/pull/687)) +- Allow null values in arrays ([#687](https://github.com/opensearch-project/opensearch-java/pull/687)) +- Wrap RuntimeExceptions in IOException when synchronous callers use asynchronous transports ([#656](https://github.com/opensearch-project/opensearch-java/pull/656)) ### Deprecated @@ -225,4 +226,4 @@ Allow null values in arrays ([#687](https://github.com/opensearch-project/opense [2.5.0]: https://github.com/opensearch-project/opensearch-java/compare/v2.4.0...v2.5.0 [2.4.0]: https://github.com/opensearch-project/opensearch-java/compare/v2.3.0...v2.4.0 [2.3.0]: https://github.com/opensearch-project/opensearch-java/compare/v2.2.0...v2.3.0 -[2.2.0]: https://github.com/opensearch-project/opensearch-java/compare/v2.1.0...v2.2.0 \ No newline at end of file +[2.2.0]: https://github.com/opensearch-project/opensearch-java/compare/v2.1.0...v2.2.0 From db7f420d2b7a0ee84718126f0f77d20cdf4975cd Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Fri, 6 Oct 2023 16:09:09 -0400 Subject: [PATCH 04/23] tolerate null causes Signed-off-by: Andrew Parmet --- .../client/transport/aws/AwsSdk2Transport.java | 12 ++++++++---- .../httpclient5/ApacheHttpClient5Transport.java | 12 ++++++++---- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java index eb3274a368..737de115ff 100644 --- a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java +++ b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java @@ -194,11 +194,15 @@ public ResponseT performRequest( try { return executeAsync((SdkAsyncHttpClient) httpClient, clientReq, requestBody, endpoint, options).get(); } catch (ExecutionException e) { - if (e.getCause() instanceof IOException) { - throw (IOException) e.getCause(); - } else { - throw new IOException(e.getCause()); + Throwable cause = e.getCause(); + if (cause != null) { + if (cause instanceof IOException) { + throw (IOException) cause; + } else { + throw new IOException(cause); + } } + throw new IOException(e); } catch (InterruptedException e) { throw new IOException("HttpRequest was interrupted", e); } diff --git a/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5Transport.java b/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5Transport.java index 8d3b14c9dd..032e0fd204 100644 --- a/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5Transport.java +++ b/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5Transport.java @@ -143,11 +143,15 @@ public ResponseT performRequest( try { return performRequestAsync(request, endpoint, options).join(); } catch (final CompletionException ex) { - if (ex.getCause() instanceof IOException) { - throw (IOException) ex.getCause(); - } else { - throw new IOException(ex.getCause()); + Throwable cause = ex.getCause(); + if (cause != null) { + if (cause instanceof IOException) { + throw (IOException) cause; + } else { + throw new IOException(cause); + } } + throw new IOException(ex); } } From 22fd7f9231b0e0786336aecef4a3dafd9a6167ab Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Fri, 6 Oct 2023 16:21:52 -0400 Subject: [PATCH 05/23] fix tests Signed-off-by: Andrew Parmet --- .../integTest/AbstractClusterClientIT.java | 15 +++-- .../opensearch/integTest/AbstractCrudIT.java | 35 +++++++--- .../integTest/AbstractIndicesClientIT.java | 64 ++++++++++++++----- .../integTest/AbstractRequestIT.java | 26 +++++--- 4 files changed, 104 insertions(+), 36 deletions(-) diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractClusterClientIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractClusterClientIT.java index 02c24a94d6..2911bfcae8 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractClusterClientIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractClusterClientIT.java @@ -95,10 +95,17 @@ public void testClusterUpdateSettingNonExistent() throws IOException { try { openSearchClient.cluster().putSettings(request); fail(); - } 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")); + } catch (Exception 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")); } } diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java index 16a9d5ff57..fd0a4b6b36 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java @@ -118,12 +118,21 @@ public void testSourceExists() throws IOException { public void testGet() throws IOException { { - OpenSearchException exception = expectThrows( - OpenSearchException.class, - () -> javaClient().get(new GetRequest.Builder().index("index").id("id").build(), String.class) + Exception exception = expectThrows( + Exception.class, + () -> javaClient().get(new GetRequest.Builder().index("index").id("id").build(), String.class) ); - assertEquals(404, exception.status()); - assertEquals("Request failed: [index_not_found_exception] no such index [index]", exception.getMessage()); + 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()); } AppData appData = new AppData(); @@ -207,12 +216,20 @@ public void testUpdate() throws Exception { .build(); try { javaClient().update(updateRequest, AppData.class); - } catch (OpenSearchException e) { + } catch (Exception e) { + OpenSearchException openSearchException; + if (e instanceof OpenSearchException) { + openSearchException = (OpenSearchException) e; + } else { + assertTrue(e.getCause() instanceof OpenSearchException); + openSearchException = (OpenSearchException) e.getCause(); + } + // 1.x: [document_missing_exception] [_doc][does_not_exist]: document missing // 2.x: [document_missing_exception] [does_not_exist]: document missing - assertTrue(e.getMessage().contains("[document_missing_exception]")); - assertTrue(e.getMessage().contains("[does_not_exist]: document missing")); - assertEquals(404, e.status()); + assertTrue(openSearchException.getMessage().contains("[document_missing_exception]")); + assertTrue(openSearchException.getMessage().contains("[does_not_exist]: document missing")); + assertEquals(404, openSearchException.status()); } } { diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractIndicesClientIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractIndicesClientIT.java index a3f360d245..8417467509 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractIndicesClientIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractIndicesClientIT.java @@ -52,10 +52,19 @@ public void testIndicesExists() throws IOException { try { javaClient().indices().get(request); fail(); // should never execute - } catch (OpenSearchException ex) { - assertNotNull(ex); - assertEquals(ex.status(), 404); - assertEquals(ex.getMessage(), "Request failed: [index_not_found_exception] " + "no such index [non_existent_index]"); + } catch (Exception 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]"); } } @@ -102,10 +111,19 @@ public void testGetSettingsNonExistentIndex() throws IOException { try { javaClient().indices().getSettings(getIndicesSettingsRequest); fail(); - } 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]"); + } catch (Exception 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]"); } } @@ -174,9 +192,16 @@ public void testDataStream() throws IOException { try { javaClient().indices().getDataStream(b -> b.name(dataStreamName)); fail(); - } catch (OpenSearchException ex) { - assertNotNull(ex); - assertEquals(ex.status(), 404); + } catch (Exception ex) { + OpenSearchException openSearchException; + if (ex instanceof OpenSearchException) { + openSearchException = (OpenSearchException) ex; + } else { + assertTrue(ex.getCause() instanceof OpenSearchException); + openSearchException = (OpenSearchException) ex.getCause(); + } + + assertEquals(openSearchException.status(), 404); } } @@ -186,10 +211,19 @@ public void testGetNotExistingIndexAlias() throws Exception { try { GetAliasResponse response = javaClient().indices().getAlias(aliasRequest); fail(); - } catch (OpenSearchException ex) { - assertNotNull(ex); - assertEquals(ex.status(), 404); - assertEquals(ex.getMessage(), "Request failed: [string_error] " + "alias [alias_not_exists] missing"); + } catch (Exception 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"); } } } diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractRequestIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractRequestIT.java index ecfd9ff3aa..1da5b1ad12 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractRequestIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractRequestIT.java @@ -419,22 +419,32 @@ public void errorResponse() throws Exception { BooleanResponse exists = javaClient().exists(_0 -> _0.index("doesnotexist").id("reallynot")); assertFalse(exists.value()); - OpenSearchException ex = assertThrows(OpenSearchException.class, () -> { - GetResponse response = javaClient().get(_0 -> _0.index("doesnotexist").id("reallynot"), String.class); + Exception ex = assertThrows(Exception.class, () -> { + GetResponse response = javaClient().get( + _0 -> _0.index("doesnotexist").id("reallynot"), String.class + ); }); - assertEquals(404, ex.status()); - assertEquals("index_not_found_exception", ex.error().type()); - assertEquals("doesnotexist", ex.error().metadata().get("index").to(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)); ExecutionException ee = assertThrows(ExecutionException.class, () -> { OpenSearchAsyncClient aClient = new OpenSearchAsyncClient(javaClient()._transport()); GetResponse response = aClient.get(_0 -> _0.index("doesnotexist").id("reallynot"), String.class).get(); }); - ex = ((OpenSearchException) ee.getCause()); - assertEquals(404, ex.status()); - assertEquals("index_not_found_exception", ex.error().type()); + openSearchException = ((OpenSearchException) ee.getCause()); + assertEquals(404, openSearchException.status()); + assertEquals("index_not_found_exception", openSearchException.error().type()); } @Test From dd2fd6597adb159f3f8877037baab4a56f222bf1 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Fri, 6 Oct 2023 16:24:27 -0400 Subject: [PATCH 06/23] remove print Signed-off-by: Andrew Parmet --- .../opensearch/integTest/AbstractAsyncStracktraceIT.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractAsyncStracktraceIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractAsyncStracktraceIT.java index 841661d696..85eb8666f7 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractAsyncStracktraceIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractAsyncStracktraceIT.java @@ -17,9 +17,6 @@ public void testFailureFromClientPreservesStacktraceOfCaller() throws Exception var thrown = assertThrows(Exception.class, () -> javaClient().indices().get(g -> g.index("nonexisting-index"))); var stacktraceElements = Throwables.toStringList(thrown); - - stacktraceElements.forEach(System.out::println); - var someElementContainsCallerMethodName = stacktraceElements.stream().anyMatch(it -> it.contains("testFailureFromClientPreservesStacktraceOfCaller")); From 1125cc2b47b00a4da0233890f89c6013ec957b80 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Fri, 6 Oct 2023 16:27:00 -0400 Subject: [PATCH 07/23] one more license header Signed-off-by: Andrew Parmet --- .../integTest/aws/AwsSdk2AsyncTransportSupport.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java index c1395f51e2..48eec6d00b 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java @@ -1,3 +1,11 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + package org.opensearch.client.opensearch.integTest.aws; import org.apache.hc.core5.http.HttpHost; From 558b58b88e6a96f24169c1e539209fc4cee02a88 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Fri, 6 Oct 2023 16:32:13 -0400 Subject: [PATCH 08/23] fix last non-aws test Signed-off-by: Andrew Parmet --- .../opensearch/integTest/AbstractIndicesClientIT.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractIndicesClientIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractIndicesClientIT.java index 8417467509..c02ae01585 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractIndicesClientIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractIndicesClientIT.java @@ -80,8 +80,10 @@ public void testIndicesExists() throws IOException { try { javaClient().indices().get(request); fail(); // should never execute - } catch (OpenSearchException ex) { - assertNotNull(ex); + } catch (Exception ex) { + if (!(ex instanceof OpenSearchException)) { + assertTrue(ex.getCause() instanceof OpenSearchException); + } } } } From cca7890be680666ab4529d5ca361d044a24c9c35 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Fri, 6 Oct 2023 16:45:23 -0400 Subject: [PATCH 09/23] use multicatch to restrict caught exceptions Signed-off-by: Andrew Parmet --- .../opensearch/integTest/AbstractClusterClientIT.java | 2 +- .../client/opensearch/integTest/AbstractCrudIT.java | 2 +- .../opensearch/integTest/AbstractIndicesClientIT.java | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractClusterClientIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractClusterClientIT.java index 2911bfcae8..92b0b1aa7c 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractClusterClientIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractClusterClientIT.java @@ -95,7 +95,7 @@ public void testClusterUpdateSettingNonExistent() throws IOException { try { openSearchClient.cluster().putSettings(request); fail(); - } catch (Exception e) { + } catch (OpenSearchException | IOException e) { OpenSearchException openSearchException; if (e instanceof OpenSearchException) { openSearchException = (OpenSearchException) e; diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java index fd0a4b6b36..2bc9ee3bc1 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java @@ -216,7 +216,7 @@ public void testUpdate() throws Exception { .build(); try { javaClient().update(updateRequest, AppData.class); - } catch (Exception e) { + } catch (OpenSearchException | IOException e) { OpenSearchException openSearchException; if (e instanceof OpenSearchException) { openSearchException = (OpenSearchException) e; diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractIndicesClientIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractIndicesClientIT.java index c02ae01585..88edd75877 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractIndicesClientIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractIndicesClientIT.java @@ -52,7 +52,7 @@ public void testIndicesExists() throws IOException { try { javaClient().indices().get(request); fail(); // should never execute - } catch (Exception ex) { + } catch (OpenSearchException | IOException ex) { OpenSearchException openSearchException; if (ex instanceof OpenSearchException) { openSearchException = (OpenSearchException) ex; @@ -80,7 +80,7 @@ public void testIndicesExists() throws IOException { try { javaClient().indices().get(request); fail(); // should never execute - } catch (Exception ex) { + } catch (OpenSearchException | IOException ex) { if (!(ex instanceof OpenSearchException)) { assertTrue(ex.getCause() instanceof OpenSearchException); } @@ -113,7 +113,7 @@ public void testGetSettingsNonExistentIndex() throws IOException { try { javaClient().indices().getSettings(getIndicesSettingsRequest); fail(); - } catch (Exception ex) { + } catch (OpenSearchException | IOException ex) { OpenSearchException openSearchException; if (ex instanceof OpenSearchException) { openSearchException = (OpenSearchException) ex; @@ -194,7 +194,7 @@ public void testDataStream() throws IOException { try { javaClient().indices().getDataStream(b -> b.name(dataStreamName)); fail(); - } catch (Exception ex) { + } catch (OpenSearchException | IOException ex) { OpenSearchException openSearchException; if (ex instanceof OpenSearchException) { openSearchException = (OpenSearchException) ex; @@ -213,7 +213,7 @@ public void testGetNotExistingIndexAlias() throws Exception { try { GetAliasResponse response = javaClient().indices().getAlias(aliasRequest); fail(); - } catch (Exception ex) { + } catch (OpenSearchException | IOException ex) { OpenSearchException openSearchException; if (ex instanceof OpenSearchException) { openSearchException = (OpenSearchException) ex; From 03b214f1811d41d6b3f43f95f46cd36c507d8661 Mon Sep 17 00:00:00 2001 From: Andriy Redko Date: Fri, 27 Oct 2023 16:28:04 -0400 Subject: [PATCH 10/23] Replicate the RestClient exception wrapping for async invocation flow Signed-off-by: Andriy Redko --- .../ApacheHttpClient5Transport.java | 93 +++++++++++++++++-- .../integTest/AbstractAsyncStracktraceIT.java | 4 +- .../integTest/AbstractClusterClientIT.java | 15 +-- .../opensearch/integTest/AbstractCrudIT.java | 35 ++----- .../integTest/AbstractIndicesClientIT.java | 70 ++++---------- .../integTest/AbstractRequestIT.java | 26 ++---- .../aws/AwsSdk2AsyncStacktraceIT.java | 3 +- .../aws/AwsSdk2AsyncTransportSupport.java | 14 +-- .../httpclient5/AsyncStacktraceIT.java | 3 +- 9 files changed, 132 insertions(+), 131 deletions(-) diff --git a/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5Transport.java b/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5Transport.java index 032e0fd204..1feb9afeef 100644 --- a/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5Transport.java +++ b/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5Transport.java @@ -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; @@ -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; @@ -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; @@ -141,17 +147,16 @@ public 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); } } @@ -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); + } + } diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractAsyncStracktraceIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractAsyncStracktraceIT.java index 85eb8666f7..590c0b2539 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractAsyncStracktraceIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractAsyncStracktraceIT.java @@ -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); } diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractClusterClientIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractClusterClientIT.java index 92b0b1aa7c..02c24a94d6 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractClusterClientIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractClusterClientIT.java @@ -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")); } } diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java index 2bc9ee3bc1..16a9d5ff57 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractCrudIT.java @@ -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(); @@ -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()); } } { diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractIndicesClientIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractIndicesClientIT.java index 88edd75877..a3f360d245 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractIndicesClientIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractIndicesClientIT.java @@ -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]"); } } @@ -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); } } } @@ -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]"); } } @@ -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); } } @@ -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"); } } } diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractRequestIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractRequestIT.java index 1da5b1ad12..ecfd9ff3aa 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractRequestIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractRequestIT.java @@ -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 response = javaClient().get( - _0 -> _0.index("doesnotexist").id("reallynot"), String.class - ); + OpenSearchException ex = assertThrows(OpenSearchException.class, () -> { + GetResponse 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 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 diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java index 460044ce8b..865ca81da3 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java @@ -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 {} diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java index 48eec6d00b..699ae40310 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java @@ -8,6 +8,7 @@ package org.opensearch.client.opensearch.integTest.aws; +import java.io.IOException; import org.apache.hc.core5.http.HttpHost; import org.opensearch.client.opensearch.integTest.OpenSearchTransportSupport; import org.opensearch.client.transport.OpenSearchTransport; @@ -18,17 +19,16 @@ import software.amazon.awssdk.http.crt.AwsCrtAsyncHttpClient; import software.amazon.awssdk.regions.Region; -import java.io.IOException; - interface AwsSdk2AsyncTransportSupport extends OpenSearchTransportSupport { @Override default OpenSearchTransport buildTransport(Settings settings, HttpHost[] hosts) throws IOException { return new AwsSdk2Transport( - getAsyncHttpClient(), - getTestClusterHost(), - getTestClusterServiceName(), - getTestClusterRegion(), - getTransportOptions().build()); + getAsyncHttpClient(), + getTestClusterHost(), + getTestClusterServiceName(), + getTestClusterRegion(), + getTransportOptions().build() + ); } private String getTestClusterHost() { diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/httpclient5/AsyncStacktraceIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/httpclient5/AsyncStacktraceIT.java index 12ee26d9b0..1312c767fe 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/httpclient5/AsyncStacktraceIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/httpclient5/AsyncStacktraceIT.java @@ -10,5 +10,4 @@ import org.opensearch.client.opensearch.integTest.AbstractAsyncStracktraceIT; -public class AsyncStacktraceIT extends AbstractAsyncStracktraceIT implements HttpClient5TransportSupport { -} +public class AsyncStacktraceIT extends AbstractAsyncStracktraceIT implements HttpClient5TransportSupport {} From 0f2ba68458e62a502a92d2a72e4aa1f4564c4b5d Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Wed, 29 May 2024 21:59:43 -0400 Subject: [PATCH 11/23] replicate hc5 exception extraction strategy in aws transport Signed-off-by: Andrew Parmet --- .../transport/aws/AwsSdk2Transport.java | 75 +++++++++++++++++-- .../ApacheHttpClient5Transport.java | 1 + 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java index 921f14b7b5..8a999a483d 100644 --- a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java +++ b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java @@ -14,6 +14,8 @@ import java.io.IOException; import java.io.InputStream; import java.io.UnsupportedEncodingException; +import java.net.ConnectException; +import java.net.SocketTimeoutException; import java.net.URI; import java.net.URISyntaxException; import java.net.URLEncoder; @@ -30,6 +32,9 @@ import javax.annotation.CheckForNull; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import javax.net.ssl.SSLHandshakeException; +import org.apache.hc.client5.http.ConnectTimeoutException; +import org.apache.hc.core5.http.ConnectionClosedException; import org.opensearch.client.json.JsonpDeserializer; import org.opensearch.client.json.JsonpMapper; import org.opensearch.client.json.jackson.JacksonJsonpMapper; @@ -199,13 +204,12 @@ public ResponseT performRequest( try { return executeAsync((SdkAsyncHttpClient) httpClient, clientReq, requestBody, endpoint, options).get(); } catch (ExecutionException e) { - Throwable cause = e.getCause(); - if (cause != null) { - if (cause instanceof IOException) { - throw (IOException) cause; - } else { - throw new IOException(cause); - } + Exception cause = extractAndWrapCause(e); + if (cause instanceof IOException) { + throw (IOException) cause; + } + if (cause instanceof RuntimeException) { + throw (RuntimeException) cause; } throw new IOException(e); } catch (InterruptedException e) { @@ -613,4 +617,61 @@ private static Optional or(Optional opt, Supplier Date: Wed, 29 May 2024 22:06:11 -0400 Subject: [PATCH 12/23] move other tests Signed-off-by: Andrew Parmet --- .../client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java | 0 .../opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename java-client/src/test/{java => java11}/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java (100%) rename java-client/src/test/{java => java11}/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java (100%) diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java b/java-client/src/test/java11/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java similarity index 100% rename from java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java rename to java-client/src/test/java11/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java b/java-client/src/test/java11/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java similarity index 100% rename from java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java rename to java-client/src/test/java11/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java From 78a328a578f992578bb8f64173726f3ede612976 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Wed, 29 May 2024 22:09:23 -0400 Subject: [PATCH 13/23] lint Signed-off-by: Andrew Parmet --- .../org/opensearch/client/transport/aws/AwsSdk2Transport.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java index 8a999a483d..b70e54d971 100644 --- a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java +++ b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java @@ -651,7 +651,7 @@ private static Exception extractAndWrapCause(Exception exception) { } if (exception instanceof SSLHandshakeException) { SSLHandshakeException e = new SSLHandshakeException( - exception.getMessage() + "\nSee https://opensearch.org/docs/latest/clients/java/ for troubleshooting." + exception.getMessage() + "\nSee https://opensearch.org/docs/latest/clients/java/ for troubleshooting." ); e.initCause(exception); return e; From 0d12013ea416111701490a1431c283e14ab03462 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Wed, 29 May 2024 22:27:37 -0400 Subject: [PATCH 14/23] delete aws test for now; add support for OpenSearchClientException Signed-off-by: Andrew Parmet --- .../transport/aws/AwsSdk2Transport.java | 6 +++ .../ApacheHttpClient5Transport.java | 6 +++ .../aws/AwsSdk2AsyncStacktraceIT.java | 13 ----- .../aws/AwsSdk2AsyncTransportSupport.java | 54 ------------------- 4 files changed, 12 insertions(+), 67 deletions(-) delete mode 100644 java-client/src/test/java11/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java delete mode 100644 java-client/src/test/java11/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java diff --git a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java index b70e54d971..26461a8303 100644 --- a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java +++ b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java @@ -41,6 +41,7 @@ import org.opensearch.client.opensearch._types.ErrorCause; import org.opensearch.client.opensearch._types.ErrorResponse; import org.opensearch.client.opensearch._types.OpenSearchException; +import org.opensearch.client.opensearch.generic.OpenSearchClientException; import org.opensearch.client.transport.Endpoint; import org.opensearch.client.transport.GenericEndpoint; import org.opensearch.client.transport.JsonEndpoint; @@ -669,6 +670,11 @@ private static Exception extractAndWrapCause(Exception exception) { e.initCause(exception); return e; } + if (exception instanceof OpenSearchClientException) { + final OpenSearchClientException e = new OpenSearchClientException(((OpenSearchClientException) exception).response()); + e.initCause(exception); + return e; + } if (exception instanceof RuntimeException) { return new RuntimeException(exception.getMessage(), exception); } diff --git a/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5Transport.java b/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5Transport.java index 7d0758e42e..26f4c71128 100644 --- a/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5Transport.java +++ b/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5Transport.java @@ -84,6 +84,7 @@ import org.opensearch.client.json.JsonpMapper; import org.opensearch.client.json.NdJsonpSerializable; import org.opensearch.client.opensearch._types.OpenSearchException; +import org.opensearch.client.opensearch.generic.OpenSearchClientException; import org.opensearch.client.transport.Endpoint; import org.opensearch.client.transport.GenericEndpoint; import org.opensearch.client.transport.GenericSerializable; @@ -1153,6 +1154,11 @@ private static Exception extractAndWrapCause(Exception exception) { e.initCause(exception); return e; } + if (exception instanceof OpenSearchClientException) { + final OpenSearchClientException e = new OpenSearchClientException(((OpenSearchClientException) exception).response()); + e.initCause(exception); + return e; + } if (exception instanceof RuntimeException) { return new RuntimeException(exception.getMessage(), exception); } diff --git a/java-client/src/test/java11/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java b/java-client/src/test/java11/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java deleted file mode 100644 index 865ca81da3..0000000000 --- a/java-client/src/test/java11/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java +++ /dev/null @@ -1,13 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.client.opensearch.integTest.aws; - -import org.opensearch.client.opensearch.integTest.AbstractAsyncStracktraceIT; - -public class AwsSdk2AsyncStacktraceIT extends AbstractAsyncStracktraceIT implements AwsSdk2AsyncTransportSupport {} diff --git a/java-client/src/test/java11/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java b/java-client/src/test/java11/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java deleted file mode 100644 index 699ae40310..0000000000 --- a/java-client/src/test/java11/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncTransportSupport.java +++ /dev/null @@ -1,54 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.client.opensearch.integTest.aws; - -import java.io.IOException; -import org.apache.hc.core5.http.HttpHost; -import org.opensearch.client.opensearch.integTest.OpenSearchTransportSupport; -import org.opensearch.client.transport.OpenSearchTransport; -import org.opensearch.client.transport.aws.AwsSdk2Transport; -import org.opensearch.client.transport.aws.AwsSdk2TransportOptions; -import org.opensearch.common.settings.Settings; -import software.amazon.awssdk.http.async.SdkAsyncHttpClient; -import software.amazon.awssdk.http.crt.AwsCrtAsyncHttpClient; -import software.amazon.awssdk.regions.Region; - -interface AwsSdk2AsyncTransportSupport extends OpenSearchTransportSupport { - @Override - default OpenSearchTransport buildTransport(Settings settings, HttpHost[] hosts) throws IOException { - return new AwsSdk2Transport( - getAsyncHttpClient(), - getTestClusterHost(), - getTestClusterServiceName(), - getTestClusterRegion(), - getTransportOptions().build() - ); - } - - private String getTestClusterHost() { - return System.getProperty("tests.awsSdk2support.domainHost"); - } - - private String getTestClusterServiceName() { - return System.getProperty("tests.awsSdk2support.serviceName"); - } - - private Region getTestClusterRegion() { - String region = System.getProperty("tests.awsSdk2support.domainRegion"); - return region != null ? Region.of(region) : Region.US_EAST_1; - } - - private AwsSdk2TransportOptions.Builder getTransportOptions() { - return AwsSdk2TransportOptions.builder(); - } - - private SdkAsyncHttpClient getAsyncHttpClient() { - return AwsCrtAsyncHttpClient.create(); - } -} From 055c48f750bc7a22ffff13fff1a5bf23281fc668 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Wed, 29 May 2024 22:35:09 -0400 Subject: [PATCH 15/23] reintroduce an aws test, sadly, not extending the abstract case Signed-off-by: Andrew Parmet --- .../aws/AwsSdk2AsyncStacktraceIT.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java new file mode 100644 index 0000000000..9bb5e74f35 --- /dev/null +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java @@ -0,0 +1,31 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.client.opensearch.integTest.aws; + +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +import org.apache.logging.log4j.core.util.Throwables; +import org.junit.Test; +import org.opensearch.client.opensearch.OpenSearchClient; + +// It would be nice to extend AbstractAsyncStracktraceIT. +public class AwsSdk2AsyncStacktraceIT extends AwsSdk2TransportTestCase { + @Test + public void testFailureFromClientPreservesStacktraceOfCaller() throws Exception { + final OpenSearchClient client = getClient(false, null, null); + var thrown = assertThrows(Exception.class, () -> client.indices().get(g -> g.index("nonexisting-index"))); + + var stacktraceElements = Throwables.toStringList(thrown); + var someElementContainsCallerMethodName = stacktraceElements.stream() + .anyMatch(it -> it.contains("testFailureFromClientPreservesStacktraceOfCaller")); + + assertTrue(someElementContainsCallerMethodName); + } +} From 2aeaa0d7f74ad0c4a4c4389afae98d75c3f9965e Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Wed, 29 May 2024 22:37:55 -0400 Subject: [PATCH 16/23] java 8 Signed-off-by: Andrew Parmet --- .../opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java index 9bb5e74f35..d620bf133b 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2AsyncStacktraceIT.java @@ -11,6 +11,7 @@ import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; +import java.util.List; import org.apache.logging.log4j.core.util.Throwables; import org.junit.Test; import org.opensearch.client.opensearch.OpenSearchClient; @@ -20,10 +21,10 @@ public class AwsSdk2AsyncStacktraceIT extends AwsSdk2TransportTestCase { @Test public void testFailureFromClientPreservesStacktraceOfCaller() throws Exception { final OpenSearchClient client = getClient(false, null, null); - var thrown = assertThrows(Exception.class, () -> client.indices().get(g -> g.index("nonexisting-index"))); + Exception thrown = assertThrows(Exception.class, () -> client.indices().get(g -> g.index("nonexisting-index"))); - var stacktraceElements = Throwables.toStringList(thrown); - var someElementContainsCallerMethodName = stacktraceElements.stream() + List stacktraceElements = Throwables.toStringList(thrown); + boolean someElementContainsCallerMethodName = stacktraceElements.stream() .anyMatch(it -> it.contains("testFailureFromClientPreservesStacktraceOfCaller")); assertTrue(someElementContainsCallerMethodName); From 6c682612ab86cdb898761f3b8ead2597b3f2cc4c Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Wed, 29 May 2024 22:53:48 -0400 Subject: [PATCH 17/23] replicate ISE Signed-off-by: Andrew Parmet --- .../org/opensearch/client/transport/aws/AwsSdk2Transport.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java index 26461a8303..f2db2bb7de 100644 --- a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java +++ b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java @@ -212,7 +212,7 @@ public ResponseT performRequest( if (cause instanceof RuntimeException) { throw (RuntimeException) cause; } - throw new IOException(e); + throw new IllegalStateException("unexpected exception type: must be either RuntimeException or IOException", cause); } catch (InterruptedException e) { throw new IOException("HttpRequest was interrupted", e); } From a7b6eb00df3951514fc34acd5184b1f873741f68 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Wed, 29 May 2024 23:29:06 -0400 Subject: [PATCH 18/23] poke Signed-off-by: Andrew Parmet From b4f54d637eb64ecf70a2b6a561716e7b0cba536e Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Wed, 29 May 2024 23:50:43 -0400 Subject: [PATCH 19/23] handle some netty-specific channel exceptions Signed-off-by: Andrew Parmet --- CHANGELOG.md | 1 + java-client/build.gradle.kts | 9 +++--- .../transport/aws/AwsSdk2Transport.java | 30 +++++++++++++++++-- 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 146487cf16..761c7f9b3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ This section is for maintaining a changelog for all breaking changes for the cli ### Fixed - ApacheHttpClient5Transport requires Apache Commons Logging dependency ([#1003](https://github.com/opensearch-project/opensearch-java/pull/1003)) +- Preserve caller information in stack traces when synchronous callers use asynchronous transports ([#656](https://github.com/opensearch-project/opensearch-java/pull/656)) ### Security diff --git a/java-client/build.gradle.kts b/java-client/build.gradle.kts index cfd9ad3a9d..569dada74a 100644 --- a/java-client/build.gradle.kts +++ b/java-client/build.gradle.kts @@ -80,7 +80,7 @@ java { registerFeature("awsSdk2Support") { usingSourceSet(sourceSets.get("main")) } - + toolchain { languageVersion = JavaLanguageVersion.of(runtimeJavaVersion.majorVersion) vendor = JvmVendorSpec.ADOPTIUM @@ -205,6 +205,7 @@ dependencies { // For AwsSdk2Transport "awsSdk2SupportCompileOnly"("software.amazon.awssdk","sdk-core","[2.15,3.0)") "awsSdk2SupportCompileOnly"("software.amazon.awssdk","auth","[2.15,3.0)") + "awsSdk2SupportCompileOnly"("io.netty","netty-handler","4.1.108.Final") testImplementation("software.amazon.awssdk","sdk-core","[2.15,3.0)") testImplementation("software.amazon.awssdk","auth","[2.15,3.0)") testImplementation("software.amazon.awssdk","aws-crt-client","[2.15,3.0)") @@ -372,17 +373,17 @@ if (runtimeJavaVersion >= JavaVersion.VERSION_11) { targetCompatibility = JavaVersion.VERSION_11.toString() sourceCompatibility = JavaVersion.VERSION_11.toString() } - + tasks.named("compileTestJava") { targetCompatibility = JavaVersion.VERSION_11.toString() sourceCompatibility = JavaVersion.VERSION_11.toString() } - + tasks.named("integrationTest") { testClassesDirs += java11.output.classesDirs classpath = sourceSets["java11"].runtimeClasspath } - + tasks.named("unitTest") { testClassesDirs += java11.output.classesDirs classpath = sourceSets["java11"].runtimeClasspath diff --git a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java index f2db2bb7de..2569335331 100644 --- a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java +++ b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java @@ -8,6 +8,10 @@ package org.opensearch.client.transport.aws; +import io.netty.channel.ChannelPipelineException; +import io.netty.channel.EventLoopException; +import io.netty.handler.timeout.ReadTimeoutException; +import io.netty.handler.timeout.WriteTimeoutException; import jakarta.json.JsonObject; import jakarta.json.stream.JsonParser; import java.io.ByteArrayInputStream; @@ -662,9 +666,6 @@ private static Exception extractAndWrapCause(Exception exception) { e.initCause(exception); return e; } - 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); @@ -675,6 +676,29 @@ private static Exception extractAndWrapCause(Exception exception) { e.initCause(exception); return e; } + if (exception instanceof ReadTimeoutException) { + final ReadTimeoutException e = new ReadTimeoutException(exception.getMessage()); + e.initCause(exception); + return e; + } + if (exception instanceof WriteTimeoutException) { + final WriteTimeoutException e = new WriteTimeoutException(exception.getMessage()); + e.initCause(exception); + return e; + } + if (exception instanceof ChannelPipelineException) { + final ChannelPipelineException e = new ChannelPipelineException(exception.getMessage()); + e.initCause(exception); + return e; + } + if (exception instanceof EventLoopException) { + final EventLoopException e = new EventLoopException(exception.getMessage()); + e.initCause(exception); + return e; + } + if (exception instanceof IOException) { + return new IOException(exception.getMessage(), exception); + } if (exception instanceof RuntimeException) { return new RuntimeException(exception.getMessage(), exception); } From 7c235b86470e44da244ec6b01f12315c6077364d Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Wed, 29 May 2024 23:59:58 -0400 Subject: [PATCH 20/23] poke Signed-off-by: Andrew Parmet From f7814cd045c95652e7d655b9ff7a0e5fdad5234d Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Thu, 30 May 2024 09:13:17 -0400 Subject: [PATCH 21/23] nevermind netty Signed-off-by: Andrew Parmet --- java-client/build.gradle.kts | 9 ++++--- .../transport/aws/AwsSdk2Transport.java | 24 ------------------- 2 files changed, 4 insertions(+), 29 deletions(-) diff --git a/java-client/build.gradle.kts b/java-client/build.gradle.kts index 569dada74a..cfd9ad3a9d 100644 --- a/java-client/build.gradle.kts +++ b/java-client/build.gradle.kts @@ -80,7 +80,7 @@ java { registerFeature("awsSdk2Support") { usingSourceSet(sourceSets.get("main")) } - + toolchain { languageVersion = JavaLanguageVersion.of(runtimeJavaVersion.majorVersion) vendor = JvmVendorSpec.ADOPTIUM @@ -205,7 +205,6 @@ dependencies { // For AwsSdk2Transport "awsSdk2SupportCompileOnly"("software.amazon.awssdk","sdk-core","[2.15,3.0)") "awsSdk2SupportCompileOnly"("software.amazon.awssdk","auth","[2.15,3.0)") - "awsSdk2SupportCompileOnly"("io.netty","netty-handler","4.1.108.Final") testImplementation("software.amazon.awssdk","sdk-core","[2.15,3.0)") testImplementation("software.amazon.awssdk","auth","[2.15,3.0)") testImplementation("software.amazon.awssdk","aws-crt-client","[2.15,3.0)") @@ -373,17 +372,17 @@ if (runtimeJavaVersion >= JavaVersion.VERSION_11) { targetCompatibility = JavaVersion.VERSION_11.toString() sourceCompatibility = JavaVersion.VERSION_11.toString() } - + tasks.named("compileTestJava") { targetCompatibility = JavaVersion.VERSION_11.toString() sourceCompatibility = JavaVersion.VERSION_11.toString() } - + tasks.named("integrationTest") { testClassesDirs += java11.output.classesDirs classpath = sourceSets["java11"].runtimeClasspath } - + tasks.named("unitTest") { testClassesDirs += java11.output.classesDirs classpath = sourceSets["java11"].runtimeClasspath diff --git a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java index 2569335331..785bfef2a9 100644 --- a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java +++ b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java @@ -8,10 +8,6 @@ package org.opensearch.client.transport.aws; -import io.netty.channel.ChannelPipelineException; -import io.netty.channel.EventLoopException; -import io.netty.handler.timeout.ReadTimeoutException; -import io.netty.handler.timeout.WriteTimeoutException; import jakarta.json.JsonObject; import jakarta.json.stream.JsonParser; import java.io.ByteArrayInputStream; @@ -676,26 +672,6 @@ private static Exception extractAndWrapCause(Exception exception) { e.initCause(exception); return e; } - if (exception instanceof ReadTimeoutException) { - final ReadTimeoutException e = new ReadTimeoutException(exception.getMessage()); - e.initCause(exception); - return e; - } - if (exception instanceof WriteTimeoutException) { - final WriteTimeoutException e = new WriteTimeoutException(exception.getMessage()); - e.initCause(exception); - return e; - } - if (exception instanceof ChannelPipelineException) { - final ChannelPipelineException e = new ChannelPipelineException(exception.getMessage()); - e.initCause(exception); - return e; - } - if (exception instanceof EventLoopException) { - final EventLoopException e = new EventLoopException(exception.getMessage()); - e.initCause(exception); - return e; - } if (exception instanceof IOException) { return new IOException(exception.getMessage(), exception); } From 5996a19d017929ee08ab739d7d8124b9e3be1b5d Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Thu, 30 May 2024 09:14:40 -0400 Subject: [PATCH 22/23] io before rt Signed-off-by: Andrew Parmet --- .../opensearch/client/transport/aws/AwsSdk2Transport.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java index 785bfef2a9..f2db2bb7de 100644 --- a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java +++ b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java @@ -662,6 +662,9 @@ private static Exception extractAndWrapCause(Exception exception) { e.initCause(exception); return e; } + 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); @@ -672,9 +675,6 @@ private static Exception extractAndWrapCause(Exception exception) { e.initCause(exception); return e; } - if (exception instanceof IOException) { - return new IOException(exception.getMessage(), exception); - } if (exception instanceof RuntimeException) { return new RuntimeException(exception.getMessage(), exception); } From 2f95baad89ad54790ffbd0dc5b177a3dcab40e7f Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Thu, 30 May 2024 09:18:53 -0400 Subject: [PATCH 23/23] no hc5 Signed-off-by: Andrew Parmet --- .../client/transport/aws/AwsSdk2Transport.java | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java index f2db2bb7de..ad6718ec20 100644 --- a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java +++ b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java @@ -33,8 +33,6 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; import javax.net.ssl.SSLHandshakeException; -import org.apache.hc.client5.http.ConnectTimeoutException; -import org.apache.hc.core5.http.ConnectionClosedException; import org.opensearch.client.json.JsonpDeserializer; import org.opensearch.client.json.JsonpMapper; import org.opensearch.client.json.jackson.JacksonJsonpMapper; @@ -635,21 +633,11 @@ private static Exception extractAndWrapCause(Exception exception) { } 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."