You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2021/12/27 11:46:50 UTC
[tinkerpop] branch ci-fix updated: Revert "3.4 - Retain failure context in NoHostAvailableException"
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch ci-fix
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/ci-fix by this push:
new 19ea116 Revert "3.4 - Retain failure context in NoHostAvailableException"
19ea116 is described below
commit 19ea116cd0a70686edd6aeeaf489f89694dcc531
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Mon Dec 27 06:46:30 2021 -0500
Revert "3.4 - Retain failure context in NoHostAvailableException"
This reverts commit 6c1cc0c64c8e9f27baffdfbff4fefb3fa80c8a1b.
---
CHANGELOG.asciidoc | 1 -
.../apache/tinkerpop/gremlin/driver/Client.java | 27 ++++++--------
.../driver/exception/NoHostAvailableException.java | 4 ---
.../gremlin/server/GremlinDriverIntegrateTest.java | 8 ++---
.../server/GremlinServerAuthIntegrateTest.java | 4 +--
.../server/GremlinServerSslIntegrateTest.java | 42 +++++++++++++---------
.../gremlin/server/util/ExceptionTestUtils.java | 34 ------------------
7 files changed, 42 insertions(+), 78 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index ed87ae9..ea8300f 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -28,7 +28,6 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
* Improved error message for failed serialization for HTTP-based requests.
* Fixed a `NullPointerException` that could occur during a failed `Connection` initialization due to uninstantiated `AtomicInteger`.
* Minor changes to the initialization of Java driver `Cluster` and `Client` such that hosts are marked as available only after successfully initializing connection pools.
-* `NoHostAvailableException` now contains a cause for the failure.
* Bumped to Netty 4.1.72.
[[release-3-4-12]]
diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java
index a816286..69d6fa4 100644
--- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java
+++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java
@@ -19,7 +19,6 @@
package org.apache.tinkerpop.gremlin.driver;
import org.apache.commons.lang3.concurrent.BasicThreadFactory;
-import org.apache.commons.lang3.exception.ExceptionUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.tinkerpop.gremlin.driver.exception.ConnectionException;
@@ -68,7 +67,6 @@ public abstract class Client {
protected final Cluster cluster;
protected volatile boolean initialized;
protected final Client.Settings settings;
- protected Throwable initializationFailure = null;
Client(final Cluster cluster, final Client.Settings settings) {
this.cluster = cluster;
@@ -213,11 +211,7 @@ public abstract class Client {
// throw an error if no host is available even after initialization is complete.
if (cluster.availableHosts().isEmpty()) {
- if (this.initializationFailure != null) {
- throw new NoHostAvailableException(this.initializationFailure);
- } else {
- throw new NoHostAvailableException();
- }
+ throw new NoHostAvailableException();
}
initialized = true;
@@ -508,13 +502,8 @@ public abstract class Client {
// you can get no possible hosts in more than a few situations. perhaps the servers are just all down.
// or perhaps the client is not configured properly (disables ssl when ssl is enabled on the server).
- if (!possibleHosts.hasNext()) {
- if (this.initializationFailure != null) {
- throw new NoHostAvailableException(this.initializationFailure);
- } else {
- throw new NoHostAvailableException();
- }
- }
+ if (!possibleHosts.hasNext())
+ throw new NoHostAvailableException();
final Host bestHost = possibleHosts.next();
final ConnectionPool pool = hostConnectionPools.get(bestHost);
@@ -542,8 +531,13 @@ public abstract class Client {
.toArray(CompletableFuture[]::new))
.join();
} catch (CompletionException ex) {
- this.initializationFailure = ExceptionUtils.getRootCause(ex) != null ? ExceptionUtils.getRootCause(ex) : ex;
- logger.error("", this.initializationFailure);
+ Throwable cause;
+ Throwable result = ex;
+ if (null != (cause = ex.getCause())) {
+ result = cause;
+ }
+
+ logger.error("", result);
} finally {
hostExecutor.shutdown();
}
@@ -809,7 +803,6 @@ public abstract class Client {
selectedHost.makeAvailable();
} catch (RuntimeException ex) {
logger.error("Could not initialize client for {}", host, ex);
- this.initializationFailure = ex;
}
}
diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/exception/NoHostAvailableException.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/exception/NoHostAvailableException.java
index 1d2a70f..e8b3b87 100644
--- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/exception/NoHostAvailableException.java
+++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/exception/NoHostAvailableException.java
@@ -24,10 +24,6 @@ public class NoHostAvailableException extends RuntimeException {
super("All hosts are considered unavailable due to previous exceptions. Check the error log to find the actual reason.");
}
- public NoHostAvailableException(Throwable ex) {
- super(ex);
- }
-
@Override
public synchronized Throwable fillInStackTrace() {
return this;
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java
index 2eb6aa7..53f0a20 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java
@@ -63,7 +63,6 @@ import org.slf4j.LoggerFactory;
import java.awt.Color;
import java.io.File;
-import java.net.ConnectException;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
@@ -86,7 +85,6 @@ import java.util.stream.Collectors;
import java.util.stream.IntStream;
import static org.apache.tinkerpop.gremlin.process.traversal.AnonymousTraversalSource.traversal;
-import static org.apache.tinkerpop.gremlin.server.util.ExceptionTestUtils.assertExceptionChainContainsCause;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.AllOf.allOf;
import static org.hamcrest.core.IsInstanceOf.instanceOf;
@@ -434,7 +432,8 @@ public class GremlinDriverIntegrateTest extends AbstractGremlinServerIntegration
client.submit("1+1").all().join().get(0).getInt();
fail("Should not have gone through because the server is not running");
} catch (Exception i) {
- assertExceptionChainContainsCause(i, NoHostAvailableException.class);
+ final Throwable root = ExceptionUtils.getRootCause(i);
+ assertThat(root, instanceOf(NoHostAvailableException.class));
}
startServer();
@@ -469,7 +468,8 @@ public class GremlinDriverIntegrateTest extends AbstractGremlinServerIntegration
client.submit("1+1").all().join().get(0).getInt();
fail("Should not have gone through because the server is not running");
} catch (Exception i) {
- assertExceptionChainContainsCause(i, NoHostAvailableException.class);
+ final Throwable root = ExceptionUtils.getRootCause(i);
+ assertThat(root, instanceOf(NoHostAvailableException.class));
}
startServer();
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthIntegrateTest.java
index 60afa57..f1740c4 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthIntegrateTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerAuthIntegrateTest.java
@@ -37,7 +37,6 @@ import java.util.concurrent.TimeoutException;
import org.apache.tinkerpop.gremlin.driver.ser.Serializers;
-import static org.apache.tinkerpop.gremlin.server.util.ExceptionTestUtils.assertExceptionChainContainsCause;
import static org.hamcrest.CoreMatchers.startsWith;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.AnyOf.anyOf;
@@ -89,7 +88,8 @@ public class GremlinServerAuthIntegrateTest extends AbstractGremlinServerIntegra
client.submit("1+1").all().get();
fail("This should not succeed as the client did not enable SSL");
} catch(Exception ex) {
- assertExceptionChainContainsCause(ex, NoHostAvailableException.class);
+ final Throwable root = ExceptionUtils.getRootCause(ex);
+ assertEquals(NoHostAvailableException.class, root.getClass());
} finally {
cluster.close();
}
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSslIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSslIntegrateTest.java
index 35c0757..8e1e8fb 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSslIntegrateTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSslIntegrateTest.java
@@ -24,6 +24,7 @@ import io.netty.handler.ssl.SslContextBuilder;
import io.netty.handler.ssl.SslProvider;
import io.netty.handler.ssl.util.InsecureTrustManagerFactory;
import io.netty.handler.ssl.util.SelfSignedCertificate;
+import org.apache.commons.lang3.exception.ExceptionUtils;
import org.apache.tinkerpop.gremlin.driver.Client;
import org.apache.tinkerpop.gremlin.driver.Cluster;
import org.apache.tinkerpop.gremlin.driver.exception.NoHostAvailableException;
@@ -31,8 +32,10 @@ import org.junit.Test;
import java.util.Arrays;
import java.util.Collections;
+import java.util.concurrent.TimeoutException;
-import static org.apache.tinkerpop.gremlin.server.util.ExceptionTestUtils.assertExceptionChainContainsCause;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
@@ -119,8 +122,7 @@ public class GremlinServerSslIntegrateTest extends AbstractGremlinServerIntegrat
settings.ssl.keyStore = JKS_SERVER_KEY;
settings.ssl.keyStorePassword = KEY_PASS;
settings.ssl.keyStoreType = KEYSTORE_TYPE_JKS;
- // Server TLS version must be enabled by default in used JDK version or tests will fail
- settings.ssl.sslEnabledProtocols = Collections.singletonList("TLSv1.2");
+ settings.ssl.sslEnabledProtocols = Collections.singletonList("TLSv1.1");
break;
case "shouldEnableSslAndFailIfCiphersDontMatch":
settings.ssl = new Settings.SslSettings();
@@ -201,7 +203,8 @@ public class GremlinServerSslIntegrateTest extends AbstractGremlinServerIntegrat
client.submit("'test'").one();
fail("Should throw exception because ssl is enabled on the server but not on client");
} catch(Exception x) {
- assertExceptionChainContainsCause(x, NoHostAvailableException.class);
+ final Throwable root = ExceptionUtils.getRootCause(x);
+ assertThat(root, instanceOf(NoHostAvailableException.class));
} finally {
cluster.close();
}
@@ -230,7 +233,8 @@ public class GremlinServerSslIntegrateTest extends AbstractGremlinServerIntegrat
client.submit("'test'").one();
fail("Should throw exception because ssl client auth is enabled on the server but client does not have a cert");
} catch(Exception x) {
- assertExceptionChainContainsCause(x, NoHostAvailableException.class);
+ final Throwable root = ExceptionUtils.getRootCause(x);
+ assertThat(root, instanceOf(NoHostAvailableException.class));
} finally {
cluster.close();
}
@@ -247,7 +251,8 @@ public class GremlinServerSslIntegrateTest extends AbstractGremlinServerIntegrat
client.submit("'test'").one();
fail("Should throw exception because ssl client auth is enabled on the server but does not trust client's cert");
} catch(Exception x) {
- assertExceptionChainContainsCause(x, NoHostAvailableException.class);
+ final Throwable root = ExceptionUtils.getRootCause(x);
+ assertThat(root, instanceOf(NoHostAvailableException.class));
} finally {
cluster.close();
}
@@ -289,7 +294,8 @@ public class GremlinServerSslIntegrateTest extends AbstractGremlinServerIntegrat
client.submit("'test'").one();
fail("Should throw exception because ssl client auth is enabled on the server but client does not have a cert");
} catch (Exception x) {
- assertExceptionChainContainsCause(x, NoHostAvailableException.class);
+ final Throwable root = ExceptionUtils.getRootCause(x);
+ assertThat(root, instanceOf(NoHostAvailableException.class));
} finally {
cluster.close();
}
@@ -305,7 +311,8 @@ public class GremlinServerSslIntegrateTest extends AbstractGremlinServerIntegrat
client.submit("'test'").one();
fail("Should throw exception because ssl client auth is enabled on the server but does not trust client's cert");
} catch (Exception x) {
- assertExceptionChainContainsCause(x, NoHostAvailableException.class);
+ final Throwable root = ExceptionUtils.getRootCause(x);
+ assertThat(root, instanceOf(NoHostAvailableException.class));
} finally {
cluster.close();
}
@@ -313,16 +320,16 @@ public class GremlinServerSslIntegrateTest extends AbstractGremlinServerIntegrat
@Test
public void shouldEnableSslAndFailIfProtocolsDontMatch() {
- // Server TLS version must be enabled by default in used JDK version or this test will fail
final Cluster cluster = TestClientFactory.build().enableSsl(true).keyStore(JKS_SERVER_KEY).keyStorePassword(KEY_PASS)
- .sslSkipCertValidation(true).sslEnabledProtocols(Arrays.asList("TLSv1.3")).create();
+ .sslSkipCertValidation(true).sslEnabledProtocols(Arrays.asList("TLSv1.2")).create();
final Client client = cluster.connect();
try {
client.submit("'test'").one();
- fail("Should throw exception because ssl client requires higher TLS version than what the server is configured to support");
+ fail("Should throw exception because ssl client requires TLSv1.2 whereas server supports only TLSv1.1");
} catch (Exception x) {
- assertExceptionChainContainsCause(x, NoHostAvailableException.class);
+ final Throwable root = ExceptionUtils.getRootCause(x);
+ assertThat(root, instanceOf(NoHostAvailableException.class));
} finally {
cluster.close();
}
@@ -336,9 +343,10 @@ public class GremlinServerSslIntegrateTest extends AbstractGremlinServerIntegrat
try {
client.submit("'test'").one();
- fail("Should throw exception because ciphers don't match");
+ fail("Should throw exception because ssl client requires TLSv1.2 whereas server supports only TLSv1.1");
} catch (Exception x) {
- assertExceptionChainContainsCause(x, NoHostAvailableException.class);
+ final Throwable root = ExceptionUtils.getRootCause(x);
+ assertThat(root, instanceOf(NoHostAvailableException.class));
} finally {
cluster.close();
}
@@ -383,7 +391,8 @@ public class GremlinServerSslIntegrateTest extends AbstractGremlinServerIntegrat
String res = client.submit("'test'").one().getString();
fail("Should throw exception because incorrect keyStoreType is specified");
} catch (Exception x) {
- assertExceptionChainContainsCause(x, NoHostAvailableException.class);
+ final Throwable root = ExceptionUtils.getRootCause(x);
+ assertThat(root, instanceOf(NoHostAvailableException.class));
} finally {
cluster.close();
}
@@ -401,7 +410,8 @@ public class GremlinServerSslIntegrateTest extends AbstractGremlinServerIntegrat
client.submit("'test'").one();
fail("Should throw exception because incorrect trustStoreType is specified");
} catch (Exception x) {
- assertExceptionChainContainsCause(x, NoHostAvailableException.class);
+ final Throwable root = ExceptionUtils.getRootCause(x);
+ assertThat(root, instanceOf(NoHostAvailableException.class));
} finally {
cluster.close();
}
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/util/ExceptionTestUtils.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/util/ExceptionTestUtils.java
deleted file mode 100644
index 2006303..0000000
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/util/ExceptionTestUtils.java
+++ /dev/null
@@ -1,34 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.tinkerpop.gremlin.server.util;
-
-import static org.junit.Assert.fail;
-
-public class ExceptionTestUtils {
-
- static public void assertExceptionChainContainsCause(Throwable e, Class<?> expectedType) {
- while (e != null) {
- if (e.getClass() == expectedType) {
- return;
- }
- e = e.getCause();
- }
- fail("Should have exception of type " + expectedType.getSimpleName() + " as a cause somewhere in the exception chain");
- }
-}