Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce assorted Reactor error handling Refaster rules #318

Merged
merged 2 commits into from
Oct 30, 2022
Merged

Introduce assorted Reactor error handling Refaster rules #318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
710520e
Drop redundant `::isInstance` in `onErrorResume`
Oct 27, 2022
c73bd48
Suggestions
Stephan202 Oct 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
256 changes: 256 additions & 0 deletions error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ReactorRules.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.HashMap;
import java.util.Optional;
import java.util.concurrent.Callable;
import java.util.function.BiConsumer;
import java.util.function.BiFunction;
import java.util.function.Consumer;
import java.util.function.Function;
Expand Down Expand Up @@ -650,6 +651,42 @@ Flux<T> after(Flux<? extends Iterable<T>> flux, int prefetch) {
}
}

/**
* Prefer {@link Mono#doOnError(Class, Consumer)} over {@link Mono#doOnError(Predicate, Consumer)}
* where possible.
*/
static final class MonoDoOnError<T> {
@BeforeTemplate
Mono<T> before(
Mono<T> mono, Class<? extends Throwable> clazz, Consumer<? super Throwable> onError) {
return mono.doOnError(clazz::isInstance, onError);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Next to clazz::isInstance users may also write t -> t instanceof clazz. Rather than adding Refaster.anyOf in a bunch of places, let's instead add a BugChecker that maps the latter to the former. (For technical reasons IIUC we can't do this with Refaster.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll work on this on a separate PR 👌

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and that PR is #323 :)

}

@AfterTemplate
Mono<T> after(
Mono<T> mono, Class<? extends Throwable> clazz, Consumer<? super Throwable> onError) {
return mono.doOnError(clazz, onError);
}
}

/**
* Prefer {@link Flux#doOnError(Class, Consumer)} over {@link Flux#doOnError(Predicate, Consumer)}
* where possible.
*/
static final class FluxDoOnError<T> {
@BeforeTemplate
Flux<T> before(
Flux<T> flux, Class<? extends Throwable> clazz, Consumer<? super Throwable> onError) {
return flux.doOnError(clazz::isInstance, onError);
}

@AfterTemplate
Flux<T> after(
Flux<T> flux, Class<? extends Throwable> clazz, Consumer<? super Throwable> onError) {
return flux.doOnError(clazz, onError);
}
}

/** Prefer {@link Mono#onErrorComplete()} over more contrived alternatives. */
static final class MonoOnErrorComplete<T> {
@BeforeTemplate
Expand All @@ -676,6 +713,225 @@ Flux<T> after(Flux<T> flux) {
}
}

/** Prefer {@link Mono#onErrorComplete(Class)}} over more contrived alternatives. */
static final class MonoOnErrorCompleteClass<T> {
@BeforeTemplate
Mono<T> before(Mono<T> mono, Class<? extends Throwable> clazz) {
return Refaster.anyOf(
mono.onErrorComplete(clazz::isInstance), mono.onErrorResume(clazz, e -> Mono.empty()));
}

@AfterTemplate
Mono<T> after(Mono<T> mono, Class<? extends Throwable> clazz) {
return mono.onErrorComplete(clazz);
}
}

/** Prefer {@link Flux#onErrorComplete(Class)}} over more contrived alternatives. */
static final class FluxOnErrorCompleteClass<T> {
@BeforeTemplate
Flux<T> before(Flux<T> flux, Class<? extends Throwable> clazz) {
return Refaster.anyOf(
flux.onErrorComplete(clazz::isInstance),
flux.onErrorResume(clazz, e -> Refaster.anyOf(Mono.empty(), Flux.empty())));
}

@AfterTemplate
Flux<T> after(Flux<T> flux, Class<? extends Throwable> clazz) {
return flux.onErrorComplete(clazz);
}
}

/** Prefer {@link Mono#onErrorComplete(Predicate)}} over more contrived alternatives. */
static final class MonoOnErrorCompletePredicate<T> {
@BeforeTemplate
Mono<T> before(Mono<T> mono, Predicate<? super Throwable> predicate) {
return mono.onErrorResume(predicate, e -> Mono.empty());
}

@AfterTemplate
Mono<T> after(Mono<T> mono, Predicate<? super Throwable> predicate) {
return mono.onErrorComplete(predicate);
}
}

/** Prefer {@link Flux#onErrorComplete(Predicate)}} over more contrived alternatives. */
static final class FluxOnErrorCompletePredicate<T> {
@BeforeTemplate
Flux<T> before(Flux<T> flux, Predicate<? super Throwable> predicate) {
return flux.onErrorResume(predicate, e -> Refaster.anyOf(Mono.empty(), Flux.empty()));
}

@AfterTemplate
Flux<T> after(Flux<T> flux, Predicate<? super Throwable> predicate) {
return flux.onErrorComplete(predicate);
}
}

/**
* Prefer {@link Mono#onErrorContinue(Class, BiConsumer)} over {@link
* Mono#onErrorContinue(Predicate, BiConsumer)} where possible.
*/
static final class MonoOnErrorContinue<T> {
@BeforeTemplate
Mono<T> before(
Mono<T> mono,
Class<? extends Throwable> clazz,
BiConsumer<Throwable, Object> errorConsumer) {
return mono.onErrorContinue(clazz::isInstance, errorConsumer);
}

@AfterTemplate
Mono<T> after(
Mono<T> mono,
Class<? extends Throwable> clazz,
BiConsumer<Throwable, Object> errorConsumer) {
return mono.onErrorContinue(clazz, errorConsumer);
}
}

/**
* Prefer {@link Flux#onErrorContinue(Class, BiConsumer)} over {@link
* Flux#onErrorContinue(Predicate, BiConsumer)} where possible.
*/
static final class FluxOnErrorContinue<T> {
@BeforeTemplate
Flux<T> before(
Flux<T> flux,
Class<? extends Throwable> clazz,
BiConsumer<Throwable, Object> errorConsumer) {
return flux.onErrorContinue(clazz::isInstance, errorConsumer);
}

@AfterTemplate
Flux<T> after(
Flux<T> flux,
Class<? extends Throwable> clazz,
BiConsumer<Throwable, Object> errorConsumer) {
return flux.onErrorContinue(clazz, errorConsumer);
}
}

/**
* Prefer {@link Mono#onErrorMap(Class, Function)} over {@link Mono#onErrorMap(Predicate,
* Function)} where possible.
*/
static final class MonoOnErrorMap<T> {
@BeforeTemplate
Mono<T> before(
Mono<T> mono,
Class<? extends Throwable> clazz,
Function<? super Throwable, ? extends Throwable> mapper) {
return mono.onErrorMap(clazz::isInstance, mapper);
}

@AfterTemplate
Mono<T> after(
Mono<T> mono,
Class<? extends Throwable> clazz,
Function<? super Throwable, ? extends Throwable> mapper) {
return mono.onErrorMap(clazz, mapper);
}
}

/**
* Prefer {@link Flux#onErrorMap(Class, Function)} over {@link Flux#onErrorMap(Predicate,
* Function)} where possible.
*/
static final class FluxOnErrorMap<T> {
@BeforeTemplate
Flux<T> before(
Flux<T> flux,
Class<? extends Throwable> clazz,
Function<? super Throwable, ? extends Throwable> mapper) {
return flux.onErrorMap(clazz::isInstance, mapper);
}

@AfterTemplate
Flux<T> after(
Flux<T> flux,
Class<? extends Throwable> clazz,
Function<? super Throwable, ? extends Throwable> mapper) {
return flux.onErrorMap(clazz, mapper);
}
}

/**
* Prefer {@link Mono#onErrorResume(Class, Function)} over {@link Mono#onErrorResume(Predicate,
* Function)} where possible.
*/
static final class MonoOnErrorResume<T> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add similar rules for a few other overload pairs that accept a Class and a Predicate. And likewise for Flux :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Phew 😆
That's quite a lot that you added :D
I was wondering indeed if there was a set of those methods that we should cover 😬
Guess that was the case 😅
Thanks ;) I could've done it though ;)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciating the fallback Function though, I was trying to find a nice way to do it, this is definitely cleaner.

@BeforeTemplate
Mono<T> before(
Mono<T> mono,
Class<? extends Throwable> clazz,
Function<? super Throwable, ? extends Mono<? extends T>> fallback) {
return mono.onErrorResume(clazz::isInstance, fallback);
}

@AfterTemplate
Mono<T> after(
Mono<T> mono,
Class<? extends Throwable> clazz,
Function<? super Throwable, ? extends Mono<? extends T>> fallback) {
return mono.onErrorResume(clazz, fallback);
}
}

/**
* Prefer {@link Flux#onErrorResume(Class, Function)} over {@link Flux#onErrorResume(Predicate,
* Function)} where possible.
*/
static final class FluxOnErrorResume<T> {
@BeforeTemplate
Flux<T> before(
Flux<T> flux,
Class<? extends Throwable> clazz,
Function<? super Throwable, ? extends Publisher<? extends T>> fallback) {
return flux.onErrorResume(clazz::isInstance, fallback);
}

@AfterTemplate
Flux<T> after(
Flux<T> flux,
Class<? extends Throwable> clazz,
Function<? super Throwable, ? extends Publisher<? extends T>> fallback) {
return flux.onErrorResume(clazz, fallback);
}
}

/**
* Prefer {@link Mono#onErrorReturn(Class, Object)} over {@link Mono#onErrorReturn(Predicate,
* Object)} where possible.
*/
static final class MonoOnErrorReturn<T> {
@BeforeTemplate
Mono<T> before(Mono<T> mono, Class<? extends Throwable> clazz, T fallbackValue) {
return mono.onErrorReturn(clazz::isInstance, fallbackValue);
}

@AfterTemplate
Mono<T> after(Mono<T> mono, Class<? extends Throwable> clazz, T fallbackValue) {
return mono.onErrorReturn(clazz, fallbackValue);
}
}

/**
* Prefer {@link Flux#onErrorReturn(Class, Object)} over {@link Flux#onErrorReturn(Predicate,
* Object)} where possible.
*/
static final class FluxOnErrorReturn<T> {
@BeforeTemplate
Flux<T> before(Flux<T> flux, Class<? extends Throwable> clazz, T fallbackValue) {
return flux.onErrorReturn(clazz::isInstance, fallbackValue);
}

@AfterTemplate
Flux<T> after(Flux<T> flux, Class<? extends Throwable> clazz, T fallbackValue) {
return flux.onErrorReturn(clazz, fallbackValue);
}
}

/** Prefer {@link reactor.util.context.Context#empty()}} over more verbose alternatives. */
// XXX: Consider introducing an `IsEmpty` matcher that identifies a wide range of guaranteed-empty
// `Collection` and `Map` expressions.
Expand Down
65 changes: 65 additions & 0 deletions ...ontrib/src/test/resources/tech/picnic/errorprone/refasterrules/ReactorRulesTestInput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,14 @@ ImmutableSet<Flux<String>> testConcatMapIterableIdentityWithPrefetch() {
Flux.just(ImmutableList.of("bar")).concatMap(Flux::fromIterable, 2));
}

Mono<Integer> testMonoDoOnError() {
return Mono.just(1).doOnError(IllegalArgumentException.class::isInstance, e -> {});
}

Flux<Integer> testFluxDoOnError() {
return Flux.just(1).doOnError(IllegalArgumentException.class::isInstance, e -> {});
}

Mono<Integer> testMonoOnErrorComplete() {
return Mono.just(1).onErrorResume(e -> Mono.empty());
}
Expand All @@ -218,6 +226,63 @@ ImmutableSet<Flux<Integer>> testFluxOnErrorComplete() {
Flux.just(2).onErrorResume(e -> Flux.empty()));
}

ImmutableSet<Mono<Integer>> testMonoOnErrorCompleteClass() {
return ImmutableSet.of(
Mono.just(1).onErrorComplete(IllegalArgumentException.class::isInstance),
Mono.just(2).onErrorResume(IllegalStateException.class, e -> Mono.empty()));
}

ImmutableSet<Flux<Integer>> testFluxOnErrorCompleteClass() {
return ImmutableSet.of(
Flux.just(1).onErrorComplete(IllegalArgumentException.class::isInstance),
Flux.just(2).onErrorResume(IllegalStateException.class, e -> Mono.empty()),
Flux.just(3).onErrorResume(AssertionError.class, e -> Flux.empty()));
}

Mono<Integer> testMonoOnErrorCompletePredicate() {
return Mono.just(1).onErrorResume(e -> e.getCause() == null, e -> Mono.empty());
}

ImmutableSet<Flux<Integer>> testFluxOnErrorCompletePredicate() {
return ImmutableSet.of(
Flux.just(1).onErrorResume(e -> e.getCause() == null, e -> Mono.empty()),
Flux.just(2).onErrorResume(e -> e.getCause() != null, e -> Flux.empty()));
}

Mono<Integer> testMonoOnErrorContinue() {
return Mono.just(1).onErrorContinue(IllegalArgumentException.class::isInstance, (e, v) -> {});
}

Flux<Integer> testFluxOnErrorContinue() {
return Flux.just(1).onErrorContinue(IllegalArgumentException.class::isInstance, (e, v) -> {});
}

Mono<Integer> testMonoOnErrorMap() {
return Mono.just(1).onErrorMap(IllegalArgumentException.class::isInstance, e -> e);
}

Flux<Integer> testFluxOnErrorMap() {
return Flux.just(1).onErrorMap(IllegalArgumentException.class::isInstance, e -> e);
}

Mono<Integer> testMonoOnErrorResume() {
return Mono.just(1)
.onErrorResume(IllegalArgumentException.class::isInstance, e -> Mono.just(2));
}

Flux<Integer> testFluxOnErrorResume() {
return Flux.just(1)
.onErrorResume(IllegalArgumentException.class::isInstance, e -> Flux.just(2));
}

Mono<Integer> testMonoOnErrorReturn() {
return Mono.just(1).onErrorReturn(IllegalArgumentException.class::isInstance, 2);
}

Flux<Integer> testFluxOnErrorReturn() {
return Flux.just(1).onErrorReturn(IllegalArgumentException.class::isInstance, 2);
}

ImmutableSet<Context> testContextEmpty() {
return ImmutableSet.of(Context.of(new HashMap<>()), Context.of(ImmutableMap.of()));
}
Expand Down
Loading