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

OperatorAny - prevent multiple terminal events #4245

Conversation

davidmoten
Copy link
Collaborator

As per discussion in #4242, if an operator maps an onNext emission to an onError emission downstream then it needs be defensive about an onCompleted being sent from upstream even if upstream has been unsubscribed.

Includes a unit test that failed on the original code.

@codecov-io
Copy link

codecov-io commented Jul 27, 2016

Current coverage is 84.21% (diff: 100%)

Merging #4245 into 1.x will decrease coverage by <.01%

@@                1.x      #4245   diff @@
==========================================
  Files           266        266          
  Lines         17426      17431     +5   
  Methods           0          0          
  Messages          0          0          
  Branches       2649       2651     +2   
==========================================
+ Hits          14676      14679     +3   
+ Misses         1893       1890     -3   
- Partials        857        862     +5   

Powered by Codecov. Last update b72beff...ed8caed

if (!done) {
done = true;
child.onError(e);
}
Copy link
Member

Choose a reason for hiding this comment

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

Call RxJavaHooks.onError so no exception is lost.

@davidmoten davidmoten force-pushed the operator-any-prevent-multiple-terminal-events branch from b95cc5d to e732adf Compare July 27, 2016 21:11
@davidmoten
Copy link
Collaborator Author

  • Updated with RxJavaHooks.onError
  • checked done at start of onNext
  • added two unit tests for (onNext, onNext) and one for (onNext, onError) cases

@akarnokd akarnokd merged commit 689b22d into ReactiveX:1.x Jul 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants