-
Notifications
You must be signed in to change notification settings - Fork 184
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.
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
Single
/Completable
subscribe
overloads to handle failures
#3112
Conversation
Motivation: Currently, users can consume the result or execute a `Runnable` for successful termination of a `Single`/`Completable`. Failures will be logged at debug level. Users can only use `whenOnError` operator to intercept the error, but it may result in duplicated logging or users simply forget to handle the error case. Modifications: - Add `Single.subscribe(Consumer<? super T>, Consumer<? super Throwable>)`; - Add `Completable.subscribe(Runnable, Consumer<? super Throwable>)`; - Modify `SimpleSingleSubscriber/SimpleCompletableSubscriber` to handle error when `errorConsumer` is provided. Result: Users can easily handle error cases when they subscribe to `Single` or `Completable`.
* @return {@link Cancellable} used to invoke {@link Cancellable#cancel()} on the parameter of | ||
* {@link Subscriber#onSubscribe(Cancellable)} for this {@link Single}. | ||
*/ | ||
public final Cancellable subscribe(Consumer<? super T> resultConsumer, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 consumers vs bi-consumer: I picked the way that exists in both rxjava (for Single and Completable) and reactor (for Mono) to be as close as possible to other implementations. We can add BiConsumer
later if there is an ask.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally prefer the bifunction version, but I agree that following precedent is probably more important.
* {@link Subscriber#onSubscribe(Cancellable)} for this {@link Single}. | ||
*/ | ||
public final Cancellable subscribe(Consumer<? super T> resultConsumer, | ||
Consumer<? super Throwable> errorConsumer) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to enforce that this helper doesn't receive a null errorConsumer
? If not, we might as well funnel the subscribe(Consumer<? super T>)
variant through this method.
The same question applies to Completable
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, changed ctors to use requireNonNull
when we expect errorConsumer
Use new `Single.subscribe(...)` overload introduced in [apple#3112](apple#3112) that takes `errorConsumer` to avoid using extra operator `whenOnError`. It will also help to avoid logging those exceptions by `SimpleSingleSubscriber` (hard to relate to a real request) and instead they will only bubble up on the caller path.
…3114) Use new `Single.subscribe(...)` overload introduced in [#3112](#3112) that takes `errorConsumer` to avoid using extra operator `whenOnError`. It will also help to avoid logging those exceptions by `SimpleSingleSubscriber` (hard to relate to a real request) and instead they will only bubble up on the caller path.
…e#3112) Motivation: Currently, users can consume the result or execute a `Runnable` for successful termination of a `Single`/`Completable`. Failures will be logged at debug level. Users can only use `whenOnError` operator to intercept the error, but it may result in duplicated logging or users simply forget to handle the error case. Modifications: - Add `Single.subscribe(Consumer<? super T>, Consumer<? super Throwable>)`; - Add `Completable.subscribe(Runnable, Consumer<? super Throwable>)`; - Modify `SimpleSingleSubscriber/SimpleCompletableSubscriber` to handle error when `errorConsumer` is provided. Result: Users can easily handle error cases when they subscribe to `Single` or `Completable`.
…pple#3114) Use new `Single.subscribe(...)` overload introduced in [apple#3112](apple#3112) that takes `errorConsumer` to avoid using extra operator `whenOnError`. It will also help to avoid logging those exceptions by `SimpleSingleSubscriber` (hard to relate to a real request) and instead they will only bubble up on the caller path.
Motivation:
Currently, users can consume the result or execute a
Runnable
for successful termination of aSingle
/Completable
. Failures will be logged at debug level. Users can only usewhenOnError
operator to intercept the error, but it may result in duplicated logging or users simply forget to handle the error case.Modifications:
Single.subscribe(Consumer<? super T>, Consumer<? super Throwable>)
;Completable.subscribe(Runnable, Consumer<? super Throwable>)
;SimpleSingleSubscriber/SimpleCompletableSubscriber
to handle error whenerrorConsumer
is provided.Result:
Users can easily handle error cases when they subscribe to
Single
orCompletable
.