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.

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

Merged
merged 2 commits into from
Nov 18, 2024

Conversation

idelpivnitskiy
Copy link
Member

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.

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,
Copy link
Member Author

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.

Copy link
Contributor

@bryce-anderson bryce-anderson left a 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) {
Copy link
Contributor

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.

Copy link
Member Author

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

@idelpivnitskiy idelpivnitskiy merged commit 7a05d7d into apple:main Nov 18, 2024
11 checks passed
@idelpivnitskiy idelpivnitskiy deleted the subscribe branch November 18, 2024 18:10
idelpivnitskiy added a commit to idelpivnitskiy/servicetalk that referenced this pull request Nov 18, 2024
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.
idelpivnitskiy added a commit that referenced this pull request Nov 18, 2024
…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.
mgodave pushed a commit to mgodave/servicetalk that referenced this pull request Nov 19, 2024
…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`.
mgodave pushed a commit to mgodave/servicetalk that referenced this pull request Nov 19, 2024
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants