Skip to content

Commit 1f12546

Browse files
eledhweneledhwen
committed
fix(resilience4j): remove intermediate error wrapping clashing with resilience4j filtering
1 parent 8bea08f commit 1f12546

5 files changed

Lines changed: 52 additions & 49 deletions

File tree

‎resilience4j/src/main/java/tech/illuin/pipeline/resilience4j/step/wrapper/circuitbreaker/CircuitBreakerStep.java‎

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,8 @@
66
import org.slf4j.MDC;
77
import tech.illuin.pipeline.context.LocalContext;
88
import tech.illuin.pipeline.input.indexer.Indexable;
9-
import tech.illuin.pipeline.resilience4j.execution.wrapper.CircuitBreakerException;
109
import tech.illuin.pipeline.resilience4j.execution.wrapper.config.circuitbreaker.CircuitBreakerStepHandler;
1110
import tech.illuin.pipeline.step.Step;
12-
import tech.illuin.pipeline.step.execution.wrapper.StepWrapperException;
1311
import tech.illuin.pipeline.step.result.Result;
1412
import tech.illuin.pipeline.step.result.ResultView;
1513

@@ -47,25 +45,15 @@ public Result execute(T object, I input, Object payload, ResultView view, LocalC
4745
this.onSuccess(object, input, payload, view, context);
4846
return result;
4947
}
50-
catch (StepWrapperException e) {
51-
this.onError(object, input, payload, view, context, e);
52-
throw (Exception) e.getCause();
53-
}
5448
catch (Exception e) {
5549
this.onError(object, input, payload, view, context, e);
56-
throw new CircuitBreakerException(e.getMessage(), e);
50+
throw e;
5751
}
5852
}
5953

60-
@SuppressWarnings("IllegalCatch")
61-
private Result executeStep(T object, I input, Object payload, ResultView view, LocalContext context) throws StepWrapperException
54+
private Result executeStep(T object, I input, Object payload, ResultView view, LocalContext context) throws Exception
6255
{
63-
try {
64-
return this.step.execute(object, input, payload, view, context);
65-
}
66-
catch (Exception e) {
67-
throw new StepWrapperException(e);
68-
}
56+
return this.step.execute(object, input, payload, view, context);
6957
}
7058

7159
private void onSuccess(T object, I input, Object payload, ResultView view, LocalContext context)

‎resilience4j/src/main/java/tech/illuin/pipeline/resilience4j/step/wrapper/retry/RetryStep.java‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,8 @@
99
import org.slf4j.MDC;
1010
import tech.illuin.pipeline.context.LocalContext;
1111
import tech.illuin.pipeline.input.indexer.Indexable;
12-
import tech.illuin.pipeline.resilience4j.execution.wrapper.RetryException;
1312
import tech.illuin.pipeline.resilience4j.execution.wrapper.config.retry.RetryStepHandler;
1413
import tech.illuin.pipeline.step.Step;
15-
import tech.illuin.pipeline.step.execution.wrapper.StepWrapperException;
1614
import tech.illuin.pipeline.step.result.Result;
1715
import tech.illuin.pipeline.step.result.ResultView;
1816

@@ -64,18 +62,13 @@ public Result execute(T object, I input, Object payload, ResultView view, LocalC
6462
this.onSuccess(object, input, payload, view, context);
6563
return result;
6664
}
67-
catch (StepWrapperException e) {
68-
this.onError(object, input, payload, view, context, e);
69-
throw (Exception) e.getCause();
70-
}
7165
catch (Exception e) {
7266
this.onError(object, input, payload, view, context, e);
73-
throw new RetryException(e.getMessage(), e);
67+
throw e;
7468
}
7569
}
7670

77-
@SuppressWarnings("IllegalCatch")
78-
private Result executeStep(T object, I input, Object payload, ResultView view, LocalContext context) throws StepWrapperException
71+
private Result executeStep(T object, I input, Object payload, ResultView view, LocalContext context) throws Exception
7972
{
8073
try {
8174
this.onAttempt(object, input, payload, view, context);
@@ -86,7 +79,7 @@ private Result executeStep(T object, I input, Object payload, ResultView view, L
8679
}
8780
catch (Exception e) {
8881
counter(RETRY_FAILURE_KEY, context, Tag.of("error", e.getClass().getName())).increment();
89-
throw new StepWrapperException(e);
82+
throw e;
9083
}
9184
}
9285

‎resilience4j/src/main/java/tech/illuin/pipeline/resilience4j/step/wrapper/timelimiter/TimeLimiterStep.java‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public Result execute(T object, I input, Object payload, ResultView view, LocalC
4444
Map<String, String> mdc = MDC.getCopyOfContextMap();
4545
Result result = this.limiter.executeFutureSupplier(() -> this.executor.submit(() -> {
4646
MDC.setContextMap(mdc);
47-
return executeStep(object, input, payload, view, context);
47+
return this.step.execute(object, input, payload, view, context);
4848
}));
4949

5050
this.onSuccess(object, input, payload, view, context);
@@ -61,17 +61,6 @@ public Result execute(T object, I input, Object payload, ResultView view, LocalC
6161
}
6262
}
6363

64-
@SuppressWarnings("IllegalCatch")
65-
private Result executeStep(T object, I input, Object payload, ResultView view, LocalContext context) throws StepWrapperException
66-
{
67-
try {
68-
return this.step.execute(object, input, payload, view, context);
69-
}
70-
catch (Exception e) {
71-
throw new StepWrapperException(e);
72-
}
73-
}
74-
7564
private void onSuccess(T object, I input, Object payload, ResultView view, LocalContext context)
7665
{
7766
logger.trace(

‎resilience4j/src/test/java/tech/illuin/pipeline/generic/pipeline/step/TestStep.java‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,18 @@
99
import tech.illuin.pipeline.step.result.ResultView;
1010
import tech.illuin.pipeline.step.variant.IndexableStep;
1111

12-
import java.util.function.Function;
13-
1412
/**
1513
* @author Pierre Lecerf (pierre.lecerf@illuin.tech)
1614
*/
1715
@StepConfig(id = "test-step")
1816
public class TestStep<T extends Indexable> implements IndexableStep<T>
1917
{
2018
private final String name;
21-
private final Function<T, String> function;
19+
private final Action<T, String> function;
2220

2321
private static final Logger logger = LoggerFactory.getLogger(TestStep.class);
2422

25-
public TestStep(String name, Function<T, String> function)
23+
public TestStep(String name, Action<T, String> function)
2624
{
2725
this.name = name;
2826
this.function = function;
@@ -34,9 +32,15 @@ public TestStep(String name, String status)
3432
}
3533

3634
@Override
37-
public Result execute(T data, ResultView results, Context context)
35+
public Result execute(T data, ResultView results, Context context) throws Exception
3836
{
3937
logger.info("test:{}: {}", this.name, data);
4038
return new TestResult(this.name, this.function.apply(data));
4139
}
40+
41+
@FunctionalInterface
42+
public interface Action<T, R>
43+
{
44+
R apply(T t) throws Exception;
45+
}
4246
}

‎resilience4j/src/test/java/tech/illuin/pipeline/step/StepWrapperRetryTest.java‎

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,19 @@
44
import org.junit.jupiter.api.Assertions;
55
import org.junit.jupiter.api.Test;
66
import tech.illuin.pipeline.Pipeline;
7-
import tech.illuin.pipeline.context.LocalContext;
8-
import tech.illuin.pipeline.input.uid_generator.UIDGenerator;
9-
import tech.illuin.pipeline.resilience4j.execution.wrapper.RetryWrapper;
7+
import tech.illuin.pipeline.PipelineException;
108
import tech.illuin.pipeline.generic.TestFactory;
119
import tech.illuin.pipeline.generic.model.A;
1210
import tech.illuin.pipeline.generic.pipeline.step.TestStep;
1311
import tech.illuin.pipeline.input.indexer.SingleIndexer;
1412
import tech.illuin.pipeline.output.Output;
13+
import tech.illuin.pipeline.resilience4j.execution.wrapper.RetryWrapper;
1514

1615
import java.time.Duration;
1716
import java.util.List;
1817
import java.util.concurrent.atomic.AtomicInteger;
18+
import java.util.function.Predicate;
19+
import java.util.function.Supplier;
1920

2021
import static tech.illuin.pipeline.generic.Tests.getResultTypes;
2122

@@ -27,7 +28,24 @@ public class StepWrapperRetryTest
2728
@Test
2829
public void testPipeline_shouldRetryException()
2930
{
30-
Pipeline<Void> pipeline = Assertions.assertDoesNotThrow(StepWrapperRetryTest::createErrorRetryPipeline);
31+
Pipeline<Void> pipeline = Assertions.assertDoesNotThrow(() -> StepWrapperRetryTest.createErrorRetryPipeline(() -> new RuntimeException("Some error"), ex -> true));
32+
Output output = Assertions.assertDoesNotThrow(() -> pipeline.run());
33+
Assertions.assertDoesNotThrow(pipeline::close);
34+
35+
Assertions.assertIterableEquals(List.of("1", "2", "3"), getResultTypes(output, output.payload(A.class)));
36+
37+
Assertions.assertEquals(1, output.results().size());
38+
Assertions.assertEquals(3, output.results().current().count());
39+
40+
Assertions.assertTrue(output.results().current("1").isPresent());
41+
Assertions.assertTrue(output.results().current("2").isPresent());
42+
Assertions.assertTrue(output.results().current("3").isPresent());
43+
}
44+
45+
@Test
46+
public void testPipeline_shouldRetryException_withTypeCondition_shouldMatch()
47+
{
48+
Pipeline<Void> pipeline = Assertions.assertDoesNotThrow(() -> StepWrapperRetryTest.createErrorRetryPipeline(TestException::new, ex -> ex instanceof TestException));
3149
Output output = Assertions.assertDoesNotThrow(() -> pipeline.run());
3250
Assertions.assertDoesNotThrow(pipeline::close);
3351

@@ -41,26 +59,37 @@ public void testPipeline_shouldRetryException()
4159
Assertions.assertTrue(output.results().current("3").isPresent());
4260
}
4361

44-
public static Pipeline<Void> createErrorRetryPipeline()
62+
@Test
63+
public void testPipeline_shouldRetryException_withTypeCondition_shouldNotMatch()
64+
{
65+
Pipeline<Void> pipeline = Assertions.assertDoesNotThrow(() -> StepWrapperRetryTest.createErrorRetryPipeline(TestException::new, ex -> ex instanceof RuntimeException));
66+
PipelineException ex = Assertions.assertThrows(PipelineException.class, pipeline::run);
67+
Assertions.assertTrue(ex.getCause() instanceof TestException);
68+
}
69+
70+
public static Pipeline<Void> createErrorRetryPipeline(Supplier<Exception> errorSupplier, Predicate<Throwable> retryPredicate)
4571
{
4672
var counter = new AtomicInteger(0);
47-
return Pipeline.of("test-error-retry", (Void input, LocalContext context, UIDGenerator generator) -> TestFactory.initializerOfEmpty(input, context, generator))
73+
return Pipeline.of("test-error-retry", TestFactory::initializerOfEmpty)
4874
.registerIndexer(SingleIndexer.auto())
4975
.registerStep(new TestStep<>("1", "ok"))
5076
.registerStep(builder -> builder
5177
.step(new TestStep<>("2", b -> {
5278
if (counter.getAndIncrement() < 4)
53-
throw new RuntimeException("Some error");
79+
throw errorSupplier.get();
5480
return "ok";
5581
}))
5682
.withWrapper(new RetryWrapper<>(RetryConfig.custom()
5783
.maxAttempts(5)
5884
.waitDuration(Duration.ofMillis(500))
85+
.retryOnException(retryPredicate)
5986
.build()
6087
))
6188
)
6289
.registerStep(new TestStep<>("3", "ok"))
6390
.build()
6491
;
6592
}
93+
94+
private static class TestException extends Exception {}
6695
}

0 commit comments

Comments
 (0)