Consider more explicit functional composition APIs

[jhalterman]

For some ideas, see:

• Even though I am using onFaliure at my retryPolicy, exception is getting thrown after my onFailure predicate execution #253 (comment)
• Even though I am using onFaliure at my retryPolicy, exception is getting thrown after my onFailure predicate execution #253 (comment)
• Even though I am using onFaliure at my retryPolicy, exception is getting thrown after my onFailure predicate execution #253 (comment)

[whiskeysierra]

FailsafeExecutor<R> fsexec = Failsafe.of(p1)
   .andThen(p2)
   .compose(p3);

@Tembrel Does it need to happen on the Failsafe API level? What about adding two default methods to Policy directly?

var failsafe = Failsafe.of(retry
    .andThen(fallback)
    .compose(timeout));

The API of Failsafe and even more importantly the internals wouldn't be touched because what it receives would still be a Policy, but now it's a composite implementation under the hood.

(Not sure if Policy lends itself to be composed directly or not. I haven't coded against the internals yet.)

[jhalterman]

Right now polices do not lend themselves to composition as easily, and really, a FailsafeExecutor represents a particular composition of policies (along with a few listeners). But, if it makes sense to change things we can.

[Tembrel]

I don't think you'd want to add composability to policies unless you also have a solid guarantee of immutability for policies (see #47), otherwise p3 = p1.andThen(p2) followed by, say, p1.mutativeMethod() might yield surprising results for p3.

The same danger exists for Failsafe.with(p2, p1) or Failsafe.of(p1).andThen(p2), of course, but I guess my expectations are different when it isn't the policies themselves doing the composing.

[jhalterman]

Another possibility that uses nesting, but in doing so, is somewhat clear about how things compose:

PolicyComposition composition = fallback.wrap(retryPolicy.wrap(circuitBreaker.wrap(timeout)));
Failsafe.with(composition).get(this::connect);

This mirrors the flattened out version nicely:

Failsafe.with(fallback, retryPolicy, circuitBreaker, timeout).get(this::connect);

[Tembrel]

Hard to imagine anyone writing out the nested version; it's hard to read, let alone write.

I've forgotten what you (@jhalterman) didn't like about:

result = Failsafe.with(timeout)
    .within(circuitBreaker)
    .within(retryPolicy)
    .within(fallback)
    .get(this::connect);

Or:

result = Failsafe.with(fallback)
    .around(retryPolicy)
    .around(circuitBreaker)
    .around(timeout)
    .get(this::connect);

Or both!

result = Failsafe.with(retryPolicy)
    .around(circuitBreaker)
    .around(timeout)
    .within(fallback)
    .get(this::connect);

(That's unambiguous but hard to understand. I imagine people would stick to uniform within or around, but it would be nice to have the option.)

A minor perq of the fluent form is that you can temporarily comment out a policy like so:

result = Failsafe.with(fallback)
    .around(retryPolicy)
    //.around(circuitBreaker)
    .around(timeout)
    .get(this::connect);

[jhalterman]

I've forgotten what you (@jhalterman) didn't like about:

Partly naming (I like @whiskeysierra's suggestion of borrowing Function's compose naming), but also some tradeoffs depending on the API approach. Also as pointed out by @whiskeysierra above, we have 2 general options for the composition API:

1. Define compositions via the policies:

Failsafe.with(fallback.compose(retryPolicy)
    .compose(circuitBreaker)
    .compose(timeout))
  .get(this::connect);

2. Define compositions via the Failsafe/FailsafeExecutor API:

Failsafe.with(fallback)
  .compose(retryPolicy)
  .compose(circuitbreaker)
  .compose(timeout)
  .get(this::connect);

I'd slightly lean towards option 2. Each .compose should probably return a new FailsafeExecutor so that an existing executor doesn't accidentally get mutated unexpectedly, as @Tembrel mentioned above. The only downside of this option that I can think of right now is the composition could possibly be mixed with other configuration, ex:

Failsafe.with(fallback)
  .compose(retryPolicy)
  .with(executorService)
  .compose(circuitBreaker)
  .onComplete(e -> ....)
  .compose(timeout)
  .get(this::connect);

...whereas option 1 wouldn't suffer from that problem. Any thoughts or preferences?

[jhalterman]

Pushed a PR for this. Feel free to weigh in there: #285.

[jhalterman]

3.0 has been released, which includes this improvement. The policy docs page also describes how composition works with a sequence diagram:

https://failsafe.dev/policies/#policy-composition