fix(subscribe): ignore syncError when deprecated#3749
Merged
benlesh merged 2 commits intoReactiveX:masterfrom May 31, 2018
Merged
fix(subscribe): ignore syncError when deprecated#3749benlesh merged 2 commits intoReactiveX:masterfrom
benlesh merged 2 commits intoReactiveX:masterfrom
Conversation
Unless using the deprecated syncError handling, the syncErrorThrowable flag should be ignored.
benlesh
approved these changes
May 31, 2018
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
This PR adds a failing test and fixes the problem with the implementation of
subscribethat sees thecatchErroroperator bypassed in some circumstances.This will fix the behaviour when
useDeprecatedSynchronousErrorHandlingisfalse- i.e. the default version 6 behaviour.Elsewhere, the
syncErrorThrowableflag has no meaning unlessuseDeprecatedSynchronousErrorHandlingistrue.So, when
useDeprecatedSynchronousErrorHandlingisfalse, it makes no sense to considersyncErrorThrowablewhen deciding between a call to_subscribeor_trySubscribe.Whether or not - as discussed in the related issue - a getter should be added to
InnerSubscriberto attempt to fix the deprecated behaviour is another (more complicated) matter. And one that need not be relevant to this PR. As noted in this comment, even with the getter added, the deprecated behaviour is still broken.Related issue (if exists): #3740