failsafe-lib / failsafe

Fault tolerance and resilience patterns for the JVM
https://failsafe.dev
Apache License 2.0
4.16k stars 295 forks source link

FailsafeExecutor.compose and .with(Executor) have surprising composition behavior #380

Open stevenschlansker opened 5 months ago

stevenschlansker commented 5 months ago

Hi Failsafe fans,

We use a Failsafe retry executor to retry potentially failing network requests. In response to an incident, we decide to .compose(CircuitBreaker)

Our original code:

failsafe = Failsafe.with(retryPolicy).with(asyncExecutor);

We updated the code:

failsafe = Failsafe.with(retryPolicy).with(asyncExecutor).compose(circuitBreakerPolicy);

However, this leads to surprising behavior - the .compose call creates a new executor but only copies the policies and not any other fields. This means your scheduler, executor, and any configured handlers are lost.

The correct version is:

failsafe = Failsafe.with(retryPolicy).compose(circuitBreakerPolicy).with(asyncExecutor)

I did not see any mention of this in the documentation. Should compose retain the other configurations on a FailsafeExecutor? Or, if that is not desirable, can we document .compose more specifically that it loses this configuration?