Reactor-core: New operator for switching to an error with a predicate

[pluttrell]

One common use case that we don't see well supported with the existing operators is to review the stream and switch to an error _with_ the value being provided to the error mapper.

For example, imperatively:

String lookupX() {
  return "deadbeef";
}

String getAndCheck() {
  String value = lookupX();
  if (value.length() > 5) {
    throw new RuntimeException("The value '" + value + "' is greater than 5 chars");
  }
  return value;
}

If we _don't_ need access to the value, we can use filter and switchIfEmpty as follows:

Mono<String> lookupX() {
  return Mono.just("deadbeef");
}

Mono<String> getAndCheck() {
  return lookupX()
    .filter(value -> value.length() <= 5)
    .switchIfEmpty(Mono.error(new RuntimeException("The value is greater than 5 chars")));
}

If we _do_ need the value currently we can use flatMap but the results get pretty ugly especially as the post validations get more numerous or complex:

Mono<String> getAndCheck() {
  return lookupX()
    .flatMap(value -> {
      if (value.length() > 5) {
        return Mono.error(new RuntimeException("The value '" + value + "' is greater than 5 chars"));
      } else {
        return Mono.just(value);
      }
    });
}

Or we can use a map, a separate method/block and rely on the automatic conversion of thrown exceptions, such as this:

Mono<String> getAndCheck() {
  return lookupX()
    .map(this::check);
}

String check(String value) {
  if (value.length() > 5) {
      throw new RuntimeException("the value " + value + " is greater than 5 chars");
  }
  return value;
}

Both of the existing methods are a bit verbose and not very expressive. If we had an operator perhaps called review, reject, or switchToError with a signature such as:

public final <R> Mono<R> switchToError(final Predicate<? super R> tester, Function<? super R, ? extends Exception> exceptionMapper) {
  ...
}

Naturally it would check the Predicate and if positive, then switch the stream to an error provided by a mapper Function that provides access to the value getting rejected.

Then we'd be able to construct streams like this:

Mono<String> getAndCheck() {
  return lookupX()
    .review(value -> value.length() > 5, value -> new RuntimeException(""The value '" + value + "' is greater than 5 chars"))
    .review(value -> value.length() < 2, value -> new RuntimeException(""The value '" + value + "' is less than than 2 chars"))
    .map( ... safely use value ... );
}

[simonbasle]

what's the benefit compared to the map approach? If that is readability, maybe the fact that the same applies to filter would make for a good alternative?

Mono<String> getAndCheck() {
  return lookupX()
    .filter(ThrowingChecks::fiveOrLessChars)
    .filter(ThrowingChecks::twoOrMoreChars)
    .map(...safely use value...);
}

public class ThrowingChecks {
  public static boolean fiveOrLessChars(String value) {
    if (value.length() > 5) throw new RuntimeException("The value '" + value + "' is greater than 5 chars");
    return true;
  }

  public static boolean twoOrMoreChars(String value) {
    if (value.length() < 2) throw new RuntimeException("The value '" + value + "' is less than 2 chars");
    return true;
  }
}

The idea of Predicate+Function operator prevents using a simple single method reference like this, and doesn't save that much typing/boilerplate:

//          .review(value -> value.length() > 5, value -> new RuntimeException("The value '" + value + "' is greater than 5 chars"))
            .filter(value -> { if (value.length() > 5) throw new RuntimeException("The value '" + value + "' is greater than 5 chars"); return true;});

[pluttrell]

Regarding the use of filter() with Predicate's that throw Exceptions compared to the map() approach, a few benefits come to mind:

1. A singular modality. I don't typically see many functions that return a boolean that also throw an Exception, especially as part of their normal execution flow.
2. The map() operator appears to be intended to transform items, whereas the intent of this suggestion would be to review and build an error, such that when the Predicate is false, the item would flow through.
3. It feels a little weird for methods that review a value, must also return the value in order to use map().

Regarding using a Predicate + Function preventing users from providing a _single_ method reference, you are correct. But I'm not suggesting that filter() be removed, so those that want to provide a single, will still be able to. With both a Predicate and Function, users could chose to provide zero, one or two method references. You might also want to consider other permutations such as a review() method that simply takes a Predicate and operates as filter() does today.

This suggestion is indeed syntactic sugar, but I believe it would allow users to more effectively control how verbose and expressive their implementations are.

[OlegDokuka]

What about handle -> https://stackoverflow.com/questions/53595420/correct-way-of-throwing-exceptions-with-reactor/53596358#53596358

[mickeelm]

I think this suggestion is great. Another name idea is validate.

IMO, using the handle() operator is the solution today that feels most "correct", but it still feels overkill having to involve a sink at all, it's pretty much code and it's "low level". map() is, like has been said earlier, at least semantically intended for transformations and while it works it doesn't match with what we're trying to do, the readability of the code suffers. filter() with a Predicate that throws an Exception is of course possible too but it doesn't feel like the intended use of a Predicate, a bit unorthodox. Basically, I agree on all three points that @pluttrell wrote.

If one _does not_ need the value in order to form an exception, filter(predicate).switchIfEmpty(exception) of course works as stated earlier. But it still feels a bit "generic" to me (an error is just one of the possibilities in switchIfEmpty), I'd like it to be clearer in the code that I'm trying to _validate_ something.

My suggestion(s):

filter(Predicate<? super T> tester)

```java
validate(Predicate tester, Supplier errorSupplier)

```java
validate(Predicate<? super T> tester, Function<? super R, ? extends Exception> exceptionMapper)

I think filter() should stay the same. I'd like two new methods called _validate_, _review_, _audit_, _verify_, something like that. One where an errorSupplier that doesn't know the value is sufficient, and one where you get the value.

I think the SO-link posted by @OlegDokuka (great answer btw!) has a great point in the initial question, _"This example is probably silly and there are surely better ways of implementing this case"_, when in fact, his solution is more or less one that is normally used. To me this justifies the need for an operator that _feels right_, both name-wise and the amount of code required to use it.

[simonbasle]

we already have quite a big number of operators, and nowadays we're trying to limit the additions to operator behaviors that cannot be emulated by other more generic operators. map with throw is one way of getting the desired behavior, plus it is efficient (no overkill/overhead of a Sink) and IMO feels less "weird" than the filter solution (I recognize that throwing Predicate is counterintuitive).

I noted your suggestions @mickeelm but I don't think we'll implement it just yet...