DATAREDIS-603 - Fall back to RedisSystemException for non translateable exceptions during cluster execution.
We now use a fallback exception translation strategy in JedisClusterConnection to map all non-mapped exceptions to RedisSystemException. This change prevents null pointer exceptions caused by potentially throwing null. Original Pull Request: #275
This commit is contained in:
committed by
Christoph Strobl
parent
2d49661e2d
commit
f5c9e75d97
@@ -137,7 +137,7 @@ public class ClusterCommandExecutor implements DisposableBean {
|
||||
return new NodeResult<>(node, cmd.doInCluster(client));
|
||||
} catch (RuntimeException ex) {
|
||||
|
||||
RuntimeException translatedException = convertToDataAccessExeption(ex);
|
||||
RuntimeException translatedException = convertToDataAccessException(ex);
|
||||
if (translatedException instanceof ClusterRedirectException) {
|
||||
ClusterRedirectException cre = (ClusterRedirectException) translatedException;
|
||||
return executeCommandOnSingleNode(cmd,
|
||||
@@ -234,13 +234,13 @@ public class ClusterCommandExecutor implements DisposableBean {
|
||||
}
|
||||
} catch (ExecutionException e) {
|
||||
|
||||
RuntimeException ex = convertToDataAccessExeption((Exception) e.getCause());
|
||||
RuntimeException ex = convertToDataAccessException((Exception) e.getCause());
|
||||
exceptions.put(entry.getKey().getNode(), ex != null ? ex : e.getCause());
|
||||
} catch (InterruptedException e) {
|
||||
|
||||
Thread.currentThread().interrupt();
|
||||
|
||||
RuntimeException ex = convertToDataAccessExeption((Exception) e.getCause());
|
||||
RuntimeException ex = convertToDataAccessException((Exception) e.getCause());
|
||||
exceptions.put(entry.getKey().getNode(), ex != null ? ex : e.getCause());
|
||||
break;
|
||||
}
|
||||
@@ -315,7 +315,7 @@ public class ClusterCommandExecutor implements DisposableBean {
|
||||
return new NodeResult<>(node, cmd.doInCluster(client, key), key);
|
||||
} catch (RuntimeException ex) {
|
||||
|
||||
RuntimeException translatedException = convertToDataAccessExeption(ex);
|
||||
RuntimeException translatedException = convertToDataAccessException(ex);
|
||||
throw translatedException != null ? translatedException : ex;
|
||||
} finally {
|
||||
this.resourceProvider.returnResourceForSpecificNode(node, client);
|
||||
@@ -326,8 +326,7 @@ public class ClusterCommandExecutor implements DisposableBean {
|
||||
return this.topologyProvider.getTopology();
|
||||
}
|
||||
|
||||
@Nullable
|
||||
private DataAccessException convertToDataAccessExeption(Exception e) {
|
||||
private DataAccessException convertToDataAccessException(Exception e) {
|
||||
return exceptionTranslationStrategy.translate(e);
|
||||
}
|
||||
|
||||
|
||||
@@ -40,7 +40,8 @@ import org.springframework.dao.DataAccessResourceFailureException;
|
||||
import org.springframework.dao.InvalidDataAccessApiUsageException;
|
||||
import org.springframework.data.redis.ClusterStateFailureException;
|
||||
import org.springframework.data.redis.ExceptionTranslationStrategy;
|
||||
import org.springframework.data.redis.PassThroughExceptionTranslationStrategy;
|
||||
import org.springframework.data.redis.FallbackExceptionTranslationStrategy;
|
||||
import org.springframework.data.redis.RedisSystemException;
|
||||
import org.springframework.data.redis.connection.*;
|
||||
import org.springframework.data.redis.connection.ClusterCommandExecutor.ClusterCommandCallback;
|
||||
import org.springframework.data.redis.connection.ClusterCommandExecutor.MultiKeyClusterCommandCallback;
|
||||
@@ -63,7 +64,7 @@ import org.springframework.util.Assert;
|
||||
*/
|
||||
public class JedisClusterConnection implements DefaultedRedisClusterConnection {
|
||||
|
||||
private static final ExceptionTranslationStrategy EXCEPTION_TRANSLATION = new PassThroughExceptionTranslationStrategy(
|
||||
private static final ExceptionTranslationStrategy EXCEPTION_TRANSLATION = new FallbackExceptionTranslationStrategy(
|
||||
JedisConverters.exceptionConverter());
|
||||
|
||||
private final Log log = LogFactory.getLog(getClass());
|
||||
@@ -653,7 +654,10 @@ public class JedisClusterConnection implements DefaultedRedisClusterConnection {
|
||||
*/
|
||||
|
||||
protected DataAccessException convertJedisAccessException(Exception ex) {
|
||||
return EXCEPTION_TRANSLATION.translate(ex);
|
||||
|
||||
DataAccessException translated = EXCEPTION_TRANSLATION.translate(ex);
|
||||
|
||||
return translated != null ? translated : new RedisSystemException(ex.getMessage(), ex);
|
||||
}
|
||||
|
||||
/*
|
||||
|
||||
@@ -43,6 +43,7 @@ import org.springframework.dao.InvalidDataAccessApiUsageException;
|
||||
import org.springframework.data.redis.ExceptionTranslationStrategy;
|
||||
import org.springframework.data.redis.FallbackExceptionTranslationStrategy;
|
||||
import org.springframework.data.redis.RedisConnectionFailureException;
|
||||
import org.springframework.data.redis.RedisSystemException;
|
||||
import org.springframework.data.redis.connection.*;
|
||||
import org.springframework.data.redis.connection.convert.TransactionResultConverter;
|
||||
import org.springframework.lang.Nullable;
|
||||
@@ -207,7 +208,7 @@ public class JedisConnection extends AbstractRedisConnection {
|
||||
broken = true;
|
||||
}
|
||||
|
||||
return exception;
|
||||
return exception != null ? exception : new RedisSystemException(ex.getMessage(), ex);
|
||||
}
|
||||
|
||||
/*
|
||||
|
||||
@@ -422,6 +422,7 @@ public class LettuceConnection extends AbstractRedisConnection {
|
||||
/**
|
||||
* 'Native' or 'raw' execution of the given command along-side the given arguments.
|
||||
*
|
||||
* @see RedisCommands#execute(String, byte[]...)
|
||||
* @param command Command to execute
|
||||
* @param commandOutputTypeHint Type of Output to use, may be (may be {@literal null}).
|
||||
* @param args Possible command arguments (may be {@literal null})
|
||||
|
||||
@@ -16,6 +16,7 @@
|
||||
package org.springframework.data.redis.connection.jedis;
|
||||
|
||||
import static org.hamcrest.Matchers.*;
|
||||
import static org.hamcrest.core.Is.is;
|
||||
import static org.junit.Assert.*;
|
||||
import static org.mockito.Mockito.*;
|
||||
import static org.springframework.data.redis.connection.ClusterTestVariables.*;
|
||||
@@ -47,6 +48,7 @@ import org.mockito.junit.MockitoJUnitRunner;
|
||||
import org.mockito.stubbing.Answer;
|
||||
import org.springframework.dao.DataAccessResourceFailureException;
|
||||
import org.springframework.data.redis.ClusterStateFailureException;
|
||||
import org.springframework.data.redis.RedisSystemException;
|
||||
import org.springframework.data.redis.connection.ClusterInfo;
|
||||
import org.springframework.data.redis.connection.RedisClusterCommands.AddSlots;
|
||||
import org.springframework.data.redis.connection.RedisClusterNode;
|
||||
@@ -107,7 +109,7 @@ public class JedisClusterConnectionUnitTests {
|
||||
}
|
||||
|
||||
@Test // DATAREDIS-315
|
||||
public void thowsExceptionWhenClusterCommandExecturorIsNull() {
|
||||
public void throwsExceptionWhenClusterCommandExecutorIsNull() {
|
||||
|
||||
expectedException.expect(IllegalArgumentException.class);
|
||||
|
||||
@@ -352,6 +354,20 @@ public class JedisClusterConnectionUnitTests {
|
||||
new JedisClusterTopologyProvider(clusterMock).getTopology();
|
||||
}
|
||||
|
||||
@Test // DATAREDIS-603
|
||||
public void translatesUnknownExceptions() {
|
||||
|
||||
IllegalArgumentException exception = new IllegalArgumentException("Aw, snap!");
|
||||
|
||||
expectedException.expect(RedisSystemException.class);
|
||||
expectedException.expectMessage(exception.getMessage());
|
||||
expectedException.expectCause(is(exception));
|
||||
|
||||
doThrow(exception).when(clusterMock).set("foo".getBytes(), "bar".getBytes());
|
||||
|
||||
connection.set("foo".getBytes(), "bar".getBytes());
|
||||
}
|
||||
|
||||
static class StubJedisCluster extends JedisCluster {
|
||||
|
||||
JedisClusterConnectionHandler connectionHandler;
|
||||
|
||||
@@ -15,13 +15,19 @@
|
||||
*/
|
||||
package org.springframework.data.redis.connection.lettuce;
|
||||
|
||||
import static org.hamcrest.Matchers.*;
|
||||
import static org.hamcrest.core.Is.is;
|
||||
import static org.junit.Assert.*;
|
||||
import static org.mockito.Mockito.*;
|
||||
import static org.mockito.Mockito.any;
|
||||
|
||||
import io.lettuce.core.RedisClient;
|
||||
import io.lettuce.core.RedisFuture;
|
||||
import io.lettuce.core.api.StatefulRedisConnection;
|
||||
import io.lettuce.core.api.async.RedisAsyncCommands;
|
||||
import io.lettuce.core.api.sync.RedisCommands;
|
||||
import io.lettuce.core.codec.RedisCodec;
|
||||
import io.lettuce.core.protocol.RedisCommand;
|
||||
|
||||
import java.lang.reflect.InvocationTargetException;
|
||||
|
||||
@@ -29,6 +35,7 @@ import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
import org.junit.runners.Suite;
|
||||
import org.mockito.Mockito;
|
||||
import org.springframework.dao.InvalidDataAccessResourceUsageException;
|
||||
import org.springframework.data.redis.connection.AbstractConnectionUnitTestBase;
|
||||
import org.springframework.data.redis.connection.RedisServerCommands.ShutdownOption;
|
||||
@@ -136,6 +143,43 @@ public class LettuceConnectionUnitTestSuite {
|
||||
|
||||
verify(syncCommandsMock, times(1)).select(1);
|
||||
}
|
||||
|
||||
@Test // DATAREDIS-603
|
||||
public void translatesUnknownExceptions() {
|
||||
|
||||
IllegalArgumentException exception = new IllegalArgumentException("Aw, snap!");
|
||||
|
||||
when(syncCommandsMock.set(any(), any())).thenThrow(exception);
|
||||
connection = new LettuceConnection(null, 0, clientMock, null, 1);
|
||||
|
||||
try {
|
||||
connection.set("foo".getBytes(), "bar".getBytes());
|
||||
} catch (Exception e) {
|
||||
|
||||
assertThat(e.getMessage(), containsString(exception.getMessage()));
|
||||
assertThat(e.getCause(), is((Throwable) exception));
|
||||
}
|
||||
}
|
||||
|
||||
@Test // DATAREDIS-603
|
||||
public void translatesPipelineUnknownExceptions() throws Exception {
|
||||
|
||||
IllegalArgumentException exception = new IllegalArgumentException("Aw, snap!");
|
||||
|
||||
RedisCommand future = mock(RedisCommand.class, Mockito.withSettings().extraInterfaces(RedisFuture.class));
|
||||
|
||||
when(((RedisFuture) future).get()).thenThrow(exception);
|
||||
when(asyncCommandsMock.set(any(byte[].class), any(byte[].class))).thenReturn((RedisFuture) future);
|
||||
connection = new LettuceConnection(null, 0, clientMock, null, 1);
|
||||
connection.openPipeline();
|
||||
|
||||
try {
|
||||
connection.set("foo".getBytes(), "bar".getBytes());
|
||||
} catch (Exception e) {
|
||||
assertThat(e.getMessage(), containsString(exception.getMessage()));
|
||||
assertThat(e.getCause(), is((Throwable) exception));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
public static class LettucePipelineConnectionUnitTests extends LettuceConnectionUnitTests {
|
||||
|
||||
Reference in New Issue
Block a user