Introduce assorted Reactor error handling Refaster rules #318
Conversation
| static final class MonoOnErrorResume<T> { | ||
| @BeforeTemplate | ||
| Mono<T> before(Mono<T> mono, Class<? extends Throwable> clazz, Mono<T> other) { | ||
| return mono.onErrorResume(clazz::isInstance, e -> other); |
There was a problem hiding this comment.
Instead of e -> other we should use Function<? super Throwable, ? extends Mono<? extends T>> fallback here, as that way we also support non-lambda function arguments.
| @@ -676,6 +676,19 @@ Flux<T> after(Flux<T> flux) { | |||
| } | |||
| } | |||
|
|
|||
| /** Drop redundant {@link Class#isInstance} in {@link Mono#onErrorResume(Function)}. */ | |||
There was a problem hiding this comment.
Mono#onErrorResume(Function) isn't the overload that's invoked 🙃
| @@ -676,6 +676,19 @@ Flux<T> after(Flux<T> flux) { | |||
| } | |||
| } | |||
|
|
|||
| /** Drop redundant {@link Class#isInstance} in {@link Mono#onErrorResume(Function)}. */ | |||
| static final class MonoOnErrorResume<T> { | |||
There was a problem hiding this comment.
We should add similar rules for a few other overload pairs that accept a Class and a Predicate. And likewise for Flux :)
There was a problem hiding this comment.
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 ;)
There was a problem hiding this comment.
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, Consumer<? super Throwable> onError) { | ||
| return mono.doOnError(clazz::isInstance, onError); |
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
I'll work on this on a separate PR 👌
Stephan202
force-pushed
the
bdiederichs/drop-redundant-is-instance
branch
from
1d0e338 to
8e061ba
Compare
Ptijohn
force-pushed
the
bdiederichs/drop-redundant-is-instance
branch
from
8e061ba to
809beb2
Compare
rickie
force-pushed
the
bdiederichs/drop-redundant-is-instance
branch
from
809beb2 to
c73bd48
Compare
There was a problem hiding this comment.
Changes LGTM!
Nice @Stephan202 that you added even more cases :).
|
Will merge once 🟢! |
rickie
changed the title
::isInstance in onErrorResume
No description provided.