Skip to content

Commit 77e4a74

Browse files
JEONGJAEIKmp911de
authored andcommitted
Fix RedisPipelineException double-wrapping in LettuceConnection.closePipeline().
The catch (Exception ex) block in closePipeline() was too broad, catching intentionally thrown RedisPipelineExceptions and re-wrapping them with the misleading default message "Pipeline contained one or more invalid commands". Introduce a dedicated catch (RedisPipelineException ex) block to propagate the original exception directly. Closes #3346 Original pull request: #3347 Signed-off-by: JEONGJAEIK <wodlr1207@naver.com>
1 parent df68a5d commit 77e4a74

2 files changed

Lines changed: 48 additions & 0 deletions

File tree

src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnection.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@
103103
* @author Tamil Selvan
104104
* @author ihaohong
105105
* @author John Blum
106+
* @author Jaeik Jeong
106107
*/
107108
@NullUnmarked
108109
public class LettuceConnection extends AbstractRedisConnection {
@@ -651,6 +652,8 @@ public void openPipeline() {
651652
}
652653

653654
throw new RedisPipelineException(new QueryTimeoutException("Redis command timed out"));
655+
} catch (RedisPipelineException ex) {
656+
throw ex;
654657
} catch (Exception ex) {
655658
throw new RedisPipelineException(ex);
656659
}

src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionUnitTests.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import java.util.List;
4040
import java.util.Map;
4141
import java.util.Map.Entry;
42+
import java.util.concurrent.TimeUnit;
4243

4344
import org.junit.jupiter.api.BeforeEach;
4445
import org.junit.jupiter.api.Disabled;
@@ -48,8 +49,11 @@
4849
import org.mockito.MockedStatic;
4950
import org.mockito.Mockito;
5051

52+
import org.springframework.dao.InvalidDataAccessApiUsageException;
5153
import org.springframework.dao.InvalidDataAccessResourceUsageException;
54+
import org.springframework.dao.QueryTimeoutException;
5255
import org.springframework.data.redis.connection.AbstractConnectionUnitTestBase;
56+
import org.springframework.data.redis.connection.RedisPipelineException;
5357
import org.springframework.data.redis.connection.RedisServerCommands.ShutdownOption;
5458
import org.springframework.data.redis.connection.RedisStreamCommands.TrimOptions;
5559
import org.springframework.data.redis.connection.RedisStreamCommands.XAddOptions;
@@ -428,6 +432,47 @@ protected void setOutput(ByteBuffer bytes) {
428432

429433
}
430434

435+
@Nested
436+
class ClosePipelineUnitTests {
437+
438+
@Test // GH-3346
439+
void closePipelineShouldNotDoubleWrapTimeoutException() {
440+
441+
connection.openPipeline();
442+
connection.set("foo".getBytes(), "bar".getBytes());
443+
444+
try (MockedStatic<LettuceFutures> lf = Mockito.mockStatic(LettuceFutures.class)) {
445+
lf.when(() -> LettuceFutures.awaitAll(anyLong(), any(TimeUnit.class), any())).thenReturn(false);
446+
447+
assertThatThrownBy(() -> connection.closePipeline())
448+
.isInstanceOf(RedisPipelineException.class)
449+
.hasCauseInstanceOf(QueryTimeoutException.class);
450+
}
451+
}
452+
453+
@SuppressWarnings({ "rawtypes", "unchecked" })
454+
@Test // GH-3346
455+
void closePipelineShouldNotDoubleWrapCommandException() {
456+
457+
Command<?, ?, ?> cmd = new Command<>(CommandType.SET, new StatusOutput<>(ByteArrayCodec.INSTANCE));
458+
AsyncCommand<?, ?, ?> future = new AsyncCommand<>(cmd);
459+
future.completeExceptionally(new RuntimeException("ERR some error"));
460+
461+
when(asyncCommandsMock.set(any(byte[].class), any(byte[].class))).thenReturn((RedisFuture) future);
462+
463+
connection.openPipeline();
464+
connection.set("foo".getBytes(), "bar".getBytes());
465+
466+
try (MockedStatic<LettuceFutures> lf = Mockito.mockStatic(LettuceFutures.class)) {
467+
lf.when(() -> LettuceFutures.awaitAll(anyLong(), any(TimeUnit.class), any())).thenReturn(true);
468+
469+
assertThatThrownBy(() -> connection.closePipeline())
470+
.isInstanceOf(RedisPipelineException.class)
471+
.hasCauseInstanceOf(InvalidDataAccessApiUsageException.class);
472+
}
473+
}
474+
}
475+
431476
@Nested
432477
class LettucePipelineConnectionUnitTests extends BasicUnitTests {
433478

0 commit comments

Comments
 (0)