This is an automated email from the ASF dual-hosted git repository.
jbarrett pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new ed00393 GEODE-9574: Sets cause to null on certain exceptions.
ed00393 is described below
commit ed003935689c157fdc52e9750bdbef21bd406569
Author: Jacob Barrett <jbarr...@pivotal.io>
AuthorDate: Wed Sep 1 16:44:10 2021 -0700
GEODE-9574: Sets cause to null on certain exceptions.
* Reverts some changes to exception handling in OpExecutorImpl that
specifically depends on cause being null.
* Coverts the handleException method to static for easier testing.
* Adds test to assert these two exceptions get thrown without cause.
---
.../cache/client/internal/OpExecutorImpl.java | 29 +++++++---
.../cache/client/internal/OpExecutorImplTest.java | 64 ++++++++++++++++++++++
2 files changed, 84 insertions(+), 9 deletions(-)
diff --git
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/OpExecutorImpl.java
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/OpExecutorImpl.java
index c5fe32a..6d75ece 100644
---
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/OpExecutorImpl.java
+++
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/OpExecutorImpl.java
@@ -497,10 +497,10 @@ public class OpExecutorImpl implements ExecutablePool {
*/
protected void handleException(Throwable e, Connection conn, int retryCount,
boolean finalAttempt) {
- handleException(e, conn, retryCount, finalAttempt, false);
+ handleException(e, conn, retryCount, finalAttempt, false, cancelCriterion,
endpointManager);
}
- protected void handleException(Op op, Throwable e, Connection conn, int
retryCount,
+ private void handleException(Op op, Throwable e, Connection conn, int
retryCount,
boolean finalAttempt, boolean timeoutFatal) throws CacheRuntimeException
{
if (op instanceof AuthenticateUserOp.AuthenticateUserOpImpl) {
if (e instanceof GemFireSecurityException) {
@@ -509,12 +509,15 @@ public class OpExecutorImpl implements ExecutablePool {
throw (ServerRefusedConnectionException) e;
}
}
- handleException(e, conn, retryCount, finalAttempt, timeoutFatal);
+ handleException(e, conn, retryCount, finalAttempt, timeoutFatal,
cancelCriterion,
+ endpointManager);
}
- protected void handleException(final Throwable throwable, final Connection
connection,
+ static void handleException(final @NotNull Throwable throwable,
+ final @NotNull Connection connection,
final int retryCount, final boolean finalAttempt,
- final boolean timeoutFatal) throws CacheRuntimeException {
+ final boolean timeoutFatal, final @NotNull CancelCriterion
cancelCriterion,
+ final @NotNull EndpointManager endpointManager) throws
CacheRuntimeException {
cancelCriterion.checkCancelInProgress(throwable);
@@ -536,6 +539,8 @@ public class OpExecutorImpl implements ExecutablePool {
boolean invalidateServer = true;
boolean warn = true;
boolean forceThrow = false;
+ Throwable cause = throwable;
+
if (throwable instanceof MessageTooLargeException) {
title = null;
exceptionToThrow = new GemFireIOException("message is too large to
transmit", throwable);
@@ -581,9 +586,13 @@ public class OpExecutorImpl implements ExecutablePool {
} else if (throwable instanceof SocketTimeoutException) {
invalidateServer = timeoutFatal;
title = "socket timed out on client";
+ // specific cause checks will fail if cause is not null here
+ cause = null;
} else if (throwable instanceof ConnectionDestroyedException) {
invalidateServer = false;
title = "connection was asynchronously destroyed";
+ // specific cause checks will fail if cause is not null here
+ cause = null;
} else if (throwable instanceof java.io.EOFException) {
title = "closed socket on server";
} else if (throwable instanceof IOException) {
@@ -611,7 +620,8 @@ public class OpExecutorImpl implements ExecutablePool {
|| (t instanceof SerializationException) || (t instanceof
CopyException)
|| (t instanceof GemFireSecurityException) || (t instanceof
ServerOperationException)
|| (t instanceof TransactionException) || (t instanceof
CancelException)) {
- handleException(t, connection, retryCount, finalAttempt, timeoutFatal);
+ handleException(t, connection, retryCount, finalAttempt, timeoutFatal,
cancelCriterion,
+ endpointManager);
return;
} else if (throwable instanceof ServerOperationException) {
title = null; // no message
@@ -620,7 +630,8 @@ public class OpExecutorImpl implements ExecutablePool {
} else if (throwable instanceof FunctionException) {
if (t instanceof InternalFunctionInvocationTargetException) {
// Client server to re-execute for node failure
- handleException(t, connection, retryCount, finalAttempt,
timeoutFatal);
+ handleException(t, connection, retryCount, finalAttempt,
timeoutFatal, cancelCriterion,
+ endpointManager);
return;
} else {
title = null; // no message
@@ -655,7 +666,7 @@ public class OpExecutorImpl implements ExecutablePool {
}
}
if (forceThrow || finalAttempt) {
- exceptionToThrow = new ServerConnectivityException(msg, throwable);
+ exceptionToThrow = new ServerConnectivityException(msg, cause);
}
}
}
@@ -664,7 +675,7 @@ public class OpExecutorImpl implements ExecutablePool {
}
}
- private StringBuilder getExceptionMessage(final String exceptionName, final
int retryCount,
+ private static StringBuilder getExceptionMessage(final String exceptionName,
final int retryCount,
final boolean finalAttempt, final Connection connection) {
final StringBuilder message = new StringBuilder(200);
message.append("Pool unexpected ").append(exceptionName);
diff --git
a/geode-core/src/test/java/org/apache/geode/cache/client/internal/OpExecutorImplTest.java
b/geode-core/src/test/java/org/apache/geode/cache/client/internal/OpExecutorImplTest.java
new file mode 100644
index 0000000..fd737b6
--- /dev/null
+++
b/geode-core/src/test/java/org/apache/geode/cache/client/internal/OpExecutorImplTest.java
@@ -0,0 +1,64 @@
+/*
+ * 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.geode.cache.client.internal;
+
+import static org.assertj.core.api.Assertions.assertThatNoException;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.mock;
+
+import java.net.SocketTimeoutException;
+
+import org.junit.Test;
+
+import org.apache.geode.CancelCriterion;
+import org.apache.geode.cache.client.ServerConnectivityException;
+import
org.apache.geode.cache.client.internal.pooling.ConnectionDestroyedException;
+
+public class OpExecutorImplTest {
+
+ @Test
+ public void handleExceptionWithSocketTimeoutExceptionDoesNotThrowException()
{
+ assertThatNoException()
+ .isThrownBy(() -> OpExecutorImpl.handleException(new
SocketTimeoutException(),
+ mock(Connection.class), 0, false,
+ false, mock(CancelCriterion.class), mock(EndpointManager.class)));
+ }
+
+ @Test
+ public void
handleExceptionWithSocketTimeoutExceptionAndFinalAttemptThrowsServerConnectivityExceptionWithoutCause()
{
+ assertThatThrownBy(() -> OpExecutorImpl.handleException(new
SocketTimeoutException(),
+ mock(Connection.class), 0, true,
+ false, mock(CancelCriterion.class), mock(EndpointManager.class)))
+ .isInstanceOf(ServerConnectivityException.class).hasNoCause();
+ }
+
+ @Test
+ public void
handleExceptionWithConnectionDestroyedExceptionDoesNotThrowException() {
+ assertThatNoException()
+ .isThrownBy(() -> OpExecutorImpl.handleException(new
ConnectionDestroyedException(),
+ mock(Connection.class), 0, false,
+ false, mock(CancelCriterion.class), mock(EndpointManager.class)));
+ }
+
+ @Test
+ public void
handleExceptionWithConnectionDestroyedExceptionAndFinalAttemptThrowsServerConnectivityExceptionWithoutCause()
{
+ assertThatThrownBy(() -> OpExecutorImpl.handleException(new
ConnectionDestroyedException(),
+ mock(Connection.class), 0, true,
+ false, mock(CancelCriterion.class), mock(EndpointManager.class)))
+ .isInstanceOf(ServerConnectivityException.class).hasNoCause();
+ }
+
+}
