Skip to content

Handle exceptions thrown by the consumer passed to SdkPublisher.subscribe or filter. #2877

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

Merged
merged 2 commits into from
Dec 1, 2021

Conversation

millems
Copy link
Contributor

@millems millems commented Nov 30, 2021

Previously, the future returned by subscribe or filter would never be completed in this scenario. Now, the result future is completed exceptionally with the exception raised by the consumer.

@millems millems requested a review from a team as a code owner November 30, 2021 23:32
…ribe or filter.

Previously, the future returned by `subscribe` or `filter` would never be completed in this scenario. Now, the result future is completed exceptionally with the exception raised by the consumer.
@millems millems force-pushed the millem/paginator-hang branch from f1bc9c4 to 2ce1a6e Compare November 30, 2021 23:52
}
} catch (RuntimeException e) {
// Handle the predicate throwing an exception
subscription.cancel();
Copy link
Contributor

Choose a reason for hiding this comment

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

Query just for my curiosity : Earlier these exceptions were thrown higher in stack and now we are catching it , do we need to worry about any backward compatibility issue were Users have expecting this exceptions to be caught directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's against "the rules" to throw out of an onNext (except for null parameters), so we're out of compliance already here. It's possible someone is relying on the old behavior: https://xkcd.com/1172/

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks Matt.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 1, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@millems millems merged commit 219f755 into master Dec 1, 2021
@millems millems deleted the millem/paginator-hang branch December 1, 2021 23:44
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