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

OperatorReduce - prevent multiple terminal events #4246

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.

Also renamed test class to match tested class.

actual.onError(e);
if (!done) {
done = true;
actual.onError(e);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't here RxJavaHooks.onError missing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep, fixing

@codecov-io
Copy link

codecov-io commented Jul 27, 2016

Current coverage is 84.30% (diff: 100%)

Merging #4246 into 1.x will increase coverage by 0.08%

@@                1.x      #4246   diff @@
==========================================
  Files           266        266          
  Lines         17426      17434     +8   
  Methods           0          0          
  Messages          0          0          
  Branches       2649       2652     +3   
==========================================
+ Hits          14676      14697    +21   
+ Misses         1893       1873    -20   
- Partials        857        864     +7   

Powered by Codecov. Last update b72beff...586d9f7

@davidmoten davidmoten force-pushed the operator-reduce-prevent-multiple-terminal-events branch from 0865a71 to 83f2108 Compare July 27, 2016 21:41
@davidmoten
Copy link
Collaborator Author

  • Updated with RxJavaHooks.onError
  • checked done at start of onNext
  • added unit tests including coverage of calling .reduce on an empty stream

@akarnokd akarnokd merged commit 60dbbed 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

4 participants