From b7d8eacdcf63fed693fa9a9fcd28dc46b0901616 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Thu, 17 Jul 2014 14:20:52 +0200 Subject: [PATCH] DATAREDIS-327 - Improve exception handling for all supported clients. We now consistently return null in implementations of RedisConnectionFactory#translateExceptionIfPossible(RuntimeException ex) in cases where we cannot translate the given exception into a more meaningful one complying with the spec of PersistenceExceptionTranslator. Introduced the ExceptionTranslationStrategy abstraction which allows to customise the translation of exceptions through the implementations PassThroughExceptionTranslationStrategy which returns null iif the Exception could not be translated and FallbackExceptionTranslationStrategy which returns a RedisSystemException wrapping the original Exception if it couldn't be translated properly. This fix also solves the issues DATAREDIS-295, DATAREDIS-325, DATAREDIS-326. Original pull request: #90. --- .../redis/ExceptionTranslationStrategy.java | 37 ++++++++++++++ .../FallbackExceptionTranslationStrategy.java | 50 +++++++++++++++++++ ...ssThroughExceptionTranslationStrategy.java | 39 +++++++++++++++ .../connection/jedis/JedisConnection.java | 10 +++- .../jedis/JedisConnectionFactory.java | 9 +++- .../connection/jedis/JedisConverters.java | 4 -- .../jedis/JedisExceptionConverter.java | 8 +-- .../connection/lettuce/LettuceConnection.java | 9 +++- .../lettuce/LettuceConnectionFactory.java | 10 +++- .../connection/lettuce/LettuceConverters.java | 4 -- .../lettuce/LettuceExceptionConverter.java | 11 +++- .../redis/connection/srp/SrpConnection.java | 7 ++- .../connection/srp/SrpConnectionFactory.java | 10 +++- .../redis/connection/srp/SrpConverters.java | 33 ++++++++---- 14 files changed, 208 insertions(+), 33 deletions(-) create mode 100644 src/main/java/org/springframework/data/redis/ExceptionTranslationStrategy.java create mode 100644 src/main/java/org/springframework/data/redis/FallbackExceptionTranslationStrategy.java create mode 100644 src/main/java/org/springframework/data/redis/PassThroughExceptionTranslationStrategy.java diff --git a/src/main/java/org/springframework/data/redis/ExceptionTranslationStrategy.java b/src/main/java/org/springframework/data/redis/ExceptionTranslationStrategy.java new file mode 100644 index 000000000..807829fb5 --- /dev/null +++ b/src/main/java/org/springframework/data/redis/ExceptionTranslationStrategy.java @@ -0,0 +1,37 @@ +/* + * Copyright 2014 the original author or authors. + * + * Licensed 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.springframework.data.redis; + +import org.springframework.dao.DataAccessException; + +/** + * Potentially translates an {@link Exception} into appropriate {@link DataAccessException}. + * + * @author Christoph Strobl + * @author Thomas Darimont + * @since 1.4 + */ +public interface ExceptionTranslationStrategy { + + /** + * Potentially translate the given {@link Exception} into {@link DataAccessException}. + * + * @param e + * @return + */ + DataAccessException translate(Exception e); + +} diff --git a/src/main/java/org/springframework/data/redis/FallbackExceptionTranslationStrategy.java b/src/main/java/org/springframework/data/redis/FallbackExceptionTranslationStrategy.java new file mode 100644 index 000000000..bc90a681c --- /dev/null +++ b/src/main/java/org/springframework/data/redis/FallbackExceptionTranslationStrategy.java @@ -0,0 +1,50 @@ +/* + * Copyright 2014 the original author or authors. + * + * Licensed 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.springframework.data.redis; + +import org.springframework.core.convert.converter.Converter; +import org.springframework.dao.DataAccessException; + +/** + * {@link FallbackExceptionTranslationStrategy} returns {@link RedisSystemException} for unknown {@link Exception}s. + * + * @author Christoph Strobl + * @author Thomas Darimont + * @since 1.4 + */ +public class FallbackExceptionTranslationStrategy extends PassThroughExceptionTranslationStrategy { + + public FallbackExceptionTranslationStrategy(Converter converter) { + super(converter); + } + + @Override + public DataAccessException translate(Exception e) { + + DataAccessException translated = super.translate(e); + return translated != null ? translated : getFallback(e); + } + + /** + * Returns a new {@link RedisSystemException} wrapping the given {@link Exception}. + * + * @param e + * @return + */ + protected RedisSystemException getFallback(Exception e) { + return new RedisSystemException("Unknown redis exception", e); + } +} diff --git a/src/main/java/org/springframework/data/redis/PassThroughExceptionTranslationStrategy.java b/src/main/java/org/springframework/data/redis/PassThroughExceptionTranslationStrategy.java new file mode 100644 index 000000000..7e27e1793 --- /dev/null +++ b/src/main/java/org/springframework/data/redis/PassThroughExceptionTranslationStrategy.java @@ -0,0 +1,39 @@ +/* + * Copyright 2014 the original author or authors. + * + * Licensed 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.springframework.data.redis; + +import org.springframework.core.convert.converter.Converter; +import org.springframework.dao.DataAccessException; + +/** + * {@link PassThroughExceptionTranslationStrategy} returns {@literal null} for unknown {@link Exception}s. + * + * @author Christoph Strobl + * @since 1.4 + */ +public class PassThroughExceptionTranslationStrategy implements ExceptionTranslationStrategy { + + private Converter converter; + + public PassThroughExceptionTranslationStrategy(Converter converter) { + this.converter = converter; + } + + @Override + public DataAccessException translate(Exception e) { + return this.converter.convert(e); + } +} diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java index b857d5d66..5d34279b9 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java @@ -31,7 +31,9 @@ import java.util.concurrent.TimeUnit; import org.springframework.core.convert.converter.Converter; import org.springframework.dao.DataAccessException; +import org.springframework.data.redis.ExceptionTranslationStrategy; import org.springframework.data.redis.RedisConnectionFailureException; +import org.springframework.data.redis.FallbackExceptionTranslationStrategy; import org.springframework.data.redis.connection.DataType; import org.springframework.data.redis.connection.FutureResult; import org.springframework.data.redis.connection.MessageListener; @@ -90,6 +92,9 @@ public class JedisConnection implements RedisConnection { private static final String SHUTDOWN_SCRIPT = "return redis.call('SHUTDOWN','%s')"; + private static final ExceptionTranslationStrategy EXCEPTION_TRANSLATION = new FallbackExceptionTranslationStrategy( + JedisConverters.exceptionConverter()); + static { CLIENT_FIELD = ReflectionUtils.findField(BinaryJedis.class, "client", Client.class); ReflectionUtils.makeAccessible(CLIENT_FIELD); @@ -177,14 +182,17 @@ public class JedisConnection implements RedisConnection { } protected DataAccessException convertJedisAccessException(Exception ex) { + if (ex instanceof NullPointerException) { // An NPE before flush will leave data in the OutputStream of a pooled connection broken = true; } - DataAccessException exception = JedisConverters.toDataAccessException(ex); + + DataAccessException exception = EXCEPTION_TRANSLATION.translate(ex); if (exception instanceof RedisConnectionFailureException) { broken = true; } + return exception; } diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactory.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactory.java index 2a69dcc9b..336fd49ae 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactory.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2013 the original author or authors. + * Copyright 2011-2014 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,6 +21,8 @@ import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.InitializingBean; import org.springframework.dao.DataAccessException; +import org.springframework.data.redis.ExceptionTranslationStrategy; +import org.springframework.data.redis.PassThroughExceptionTranslationStrategy; import org.springframework.data.redis.RedisConnectionFailureException; import org.springframework.data.redis.connection.RedisConnection; import org.springframework.data.redis.connection.RedisConnectionFactory; @@ -37,10 +39,13 @@ import redis.clients.jedis.Protocol; * Connection factory creating Jedis based connections. * * @author Costin Leau + * @author Thomas Darimont */ public class JedisConnectionFactory implements InitializingBean, DisposableBean, RedisConnectionFactory { private final static Log log = LogFactory.getLog(JedisConnectionFactory.class); + private static final ExceptionTranslationStrategy EXCEPTION_TRANSLATION = new PassThroughExceptionTranslationStrategy( + JedisConverters.exceptionConverter()); private JedisShardInfo shardInfo; private String hostName = "localhost"; @@ -148,7 +153,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean, } public DataAccessException translateExceptionIfPossible(RuntimeException ex) { - return JedisConverters.toDataAccessException(ex); + return EXCEPTION_TRANSLATION.translate(ex); } /** diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConverters.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConverters.java index 749f33688..20943b0b4 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConverters.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConverters.java @@ -152,10 +152,6 @@ abstract public class JedisConverters extends Converters { return STRING_TO_CLIENT_INFO_CONVERTER.convert(source.split("\\r?\\n")); } - public static DataAccessException toDataAccessException(Exception ex) { - return EXCEPTION_CONVERTER.convert(ex); - } - public static LIST_POSITION toListPosition(Position source) { Assert.notNull("list positions are mandatory"); return (Position.AFTER.equals(source) ? LIST_POSITION.AFTER : LIST_POSITION.BEFORE); diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisExceptionConverter.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisExceptionConverter.java index 99b169f2d..27444a2ce 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisExceptionConverter.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisExceptionConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-2014 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,7 +22,6 @@ import org.springframework.core.convert.converter.Converter; import org.springframework.dao.DataAccessException; import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.data.redis.RedisConnectionFailureException; -import org.springframework.data.redis.RedisSystemException; import redis.clients.jedis.exceptions.JedisConnectionException; import redis.clients.jedis.exceptions.JedisDataException; @@ -32,10 +31,12 @@ import redis.clients.jedis.exceptions.JedisException; * Converts Exceptions thrown from Jedis to {@link DataAccessException}s * * @author Jennifer Hickey + * @author Thomas Darimont */ public class JedisExceptionConverter implements Converter { public DataAccessException convert(Exception ex) { + if (ex instanceof DataAccessException) { return (DataAccessException) ex; } @@ -54,6 +55,7 @@ public class JedisExceptionConverter implements Converter CODEC = new BytesRedisCodec(); private static final TypeHints typeHints = new TypeHints(); @@ -268,7 +273,9 @@ public class LettuceConnection implements RedisConnection { } protected DataAccessException convertLettuceAccessException(Exception ex) { - DataAccessException exception = LettuceConverters.toDataAccessException(ex); + + DataAccessException exception = EXCEPTION_TRANSLATION.translate(ex); + if (exception instanceof RedisConnectionFailureException) { broken = true; } diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionFactory.java b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionFactory.java index 566044dbc..a0062681d 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionFactory.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2013 the original author or authors. + * Copyright 2011-2014 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,6 +23,8 @@ import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.InitializingBean; import org.springframework.dao.DataAccessException; +import org.springframework.data.redis.ExceptionTranslationStrategy; +import org.springframework.data.redis.PassThroughExceptionTranslationStrategy; import org.springframework.data.redis.RedisConnectionFailureException; import org.springframework.data.redis.connection.Pool; import org.springframework.data.redis.connection.RedisConnection; @@ -47,9 +49,13 @@ import com.lambdaworks.redis.RedisException; * * @author Costin Leau * @author Jennifer Hickey + * @author Thomas Darimont */ public class LettuceConnectionFactory implements InitializingBean, DisposableBean, RedisConnectionFactory { + private static final ExceptionTranslationStrategy EXCEPTION_TRANSLATION = new PassThroughExceptionTranslationStrategy( + LettuceConverters.exceptionConverter()); + private final Log log = LogFactory.getLog(getClass()); private String hostName = "localhost"; @@ -134,7 +140,7 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea } public DataAccessException translateExceptionIfPossible(RuntimeException ex) { - return LettuceConverters.toDataAccessException(ex); + return EXCEPTION_TRANSLATION.translate(ex); } /** diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConverters.java b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConverters.java index 489a2fcc3..c7b266981 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConverters.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConverters.java @@ -256,10 +256,6 @@ abstract public class LettuceConverters extends Converters { return SCORED_VALUE_TO_TUPLE.convert(source); } - public static DataAccessException toDataAccessException(Exception ex) { - return EXCEPTION_CONVERTER.convert(ex); - } - public static String toString(byte[] source) { return BYTES_TO_STRING.convert(source); } diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceExceptionConverter.java b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceExceptionConverter.java index 27d2271b3..51ab4d16a 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceExceptionConverter.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceExceptionConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-2014 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -31,25 +31,32 @@ import com.lambdaworks.redis.RedisException; * Converts Lettuce Exceptions to {@link DataAccessException}s * * @author Jennifer Hickey + * @author Thomas Darimont */ public class LettuceExceptionConverter implements Converter { public DataAccessException convert(Exception ex) { + if (ex instanceof DataAccessException) { return (DataAccessException) ex; } + if (ex instanceof RedisCommandInterruptedException) { return new RedisSystemException("Redis command interrupted", ex); } + if (ex instanceof RedisException) { return new RedisSystemException("Redis exception", ex); } + if (ex instanceof ChannelException) { return new RedisConnectionFailureException("Redis connection failed", ex); } + if (ex instanceof TimeoutException) { return new QueryTimeoutException("Redis command timed out", ex); } - return new RedisSystemException("Unknown Lettuce exception", ex); + + return null; } } diff --git a/src/main/java/org/springframework/data/redis/connection/srp/SrpConnection.java b/src/main/java/org/springframework/data/redis/connection/srp/SrpConnection.java index df6b05504..c8d23f8c3 100644 --- a/src/main/java/org/springframework/data/redis/connection/srp/SrpConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/srp/SrpConnection.java @@ -31,7 +31,9 @@ import java.util.concurrent.Future; import org.springframework.core.convert.converter.Converter; import org.springframework.dao.DataAccessException; +import org.springframework.data.redis.ExceptionTranslationStrategy; import org.springframework.data.redis.RedisConnectionFailureException; +import org.springframework.data.redis.FallbackExceptionTranslationStrategy; import org.springframework.data.redis.connection.DataType; import org.springframework.data.redis.connection.FutureResult; import org.springframework.data.redis.connection.MessageListener; @@ -69,6 +71,9 @@ import com.google.common.util.concurrent.ListenableFuture; */ public class SrpConnection implements RedisConnection { + private static final ExceptionTranslationStrategy EXCEPTION_TRANSLATION = new FallbackExceptionTranslationStrategy( + SrpConverters.exceptionConverter()); + private static final Object[] EMPTY_PARAMS_ARRAY = new Object[0]; private static final byte[] WITHSCORES = "WITHSCORES".getBytes(Charsets.UTF_8); private static final byte[] BY = "BY".getBytes(Charsets.UTF_8); @@ -241,7 +246,7 @@ public class SrpConnection implements RedisConnection { } protected DataAccessException convertSrpAccessException(Exception ex) { - return SrpConverters.toDataAccessException(ex); + return EXCEPTION_TRANSLATION.translate(ex); } public Object execute(String command, byte[]... args) { diff --git a/src/main/java/org/springframework/data/redis/connection/srp/SrpConnectionFactory.java b/src/main/java/org/springframework/data/redis/connection/srp/SrpConnectionFactory.java index efdb17426..9f187a737 100644 --- a/src/main/java/org/springframework/data/redis/connection/srp/SrpConnectionFactory.java +++ b/src/main/java/org/springframework/data/redis/connection/srp/SrpConnectionFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2013 the original author or authors. + * Copyright 2011-2014 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,6 +22,8 @@ import java.util.concurrent.BlockingQueue; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.InitializingBean; import org.springframework.dao.DataAccessException; +import org.springframework.data.redis.ExceptionTranslationStrategy; +import org.springframework.data.redis.PassThroughExceptionTranslationStrategy; import org.springframework.data.redis.connection.RedisConnection; import org.springframework.data.redis.connection.RedisConnectionFactory; @@ -29,9 +31,13 @@ import org.springframework.data.redis.connection.RedisConnectionFactory; * Connection factory creating Redis Protocol based connections. * * @author Costin Leau + * @author Thomas Darimont */ public class SrpConnectionFactory implements InitializingBean, DisposableBean, RedisConnectionFactory { + private static final ExceptionTranslationStrategy EXCEPTION_TRANSLATION = new PassThroughExceptionTranslationStrategy( + SrpConverters.exceptionConverter()); + private String hostName = "localhost"; private int port = 6379; private BlockingQueue trackedConnections = new ArrayBlockingQueue(50); @@ -75,7 +81,7 @@ public class SrpConnectionFactory implements InitializingBean, DisposableBean, R } public DataAccessException translateExceptionIfPossible(RuntimeException ex) { - return SrpConverters.toDataAccessException(ex); + return EXCEPTION_TRANSLATION.translate(ex); } /** diff --git a/src/main/java/org/springframework/data/redis/connection/srp/SrpConverters.java b/src/main/java/org/springframework/data/redis/connection/srp/SrpConverters.java index 68276a1f9..c036a8229 100644 --- a/src/main/java/org/springframework/data/redis/connection/srp/SrpConverters.java +++ b/src/main/java/org/springframework/data/redis/connection/srp/SrpConverters.java @@ -73,6 +73,7 @@ abstract public class SrpConverters extends Converters { private static final Converter> BYTEARRAY_T0_LIST_OF_CLIENT_INFO; private static final Converter INTEGER_REPLY_TO_BOOLEAN; private static final Converter LONG_TO_BOOLEAN = new LongToBooleanConverter(); + private static final Converter EXCEPTION_CONVERTER; static { @@ -237,6 +238,23 @@ abstract public class SrpConverters extends Converters { return source.data() == 1; } }; + + EXCEPTION_CONVERTER = new Converter() { + + @Override + public DataAccessException convert(Exception ex) { + + if (ex instanceof RedisException) { + return new RedisSystemException("redis exception", ex); + } + + if (ex instanceof IOException) { + return new RedisConnectionFailureException("Redis connection failed", (IOException) ex); + } + + return null; + } + }; } public static Converter> repliesToBytesList() { @@ -354,17 +372,6 @@ abstract public class SrpConverters extends Converters { return op.name().toUpperCase().getBytes(Charsets.UTF_8); } - public static DataAccessException toDataAccessException(Exception ex) { - if (ex instanceof RedisException) { - return new RedisSystemException("redis exception", ex); - } - if (ex instanceof IOException) { - return new RedisConnectionFailureException("Redis connection failed", (IOException) ex); - } - - return new RedisSystemException("Unknown SRP exception", ex); - } - public static byte[][] toByteArrays(Map source) { byte[][] result = new byte[source.size() * 2][]; int index = 0; @@ -402,4 +409,8 @@ abstract public class SrpConverters extends Converters { public static Boolean toBoolean(IntegerReply reply) { return INTEGER_REPLY_TO_BOOLEAN.convert(reply); } + + public static Converter exceptionConverter() { + return EXCEPTION_CONVERTER; + } }