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

fix(retry): Ensure teardown happens before resubscription with synchr… #5621

Merged

Conversation

benlesh
Copy link
Member

@benlesh benlesh commented Aug 3, 2020

…onous observables

Related: #5620

  • Resolves an issue where all teardowns would not execute until the result observable was complete if the source was synchronous

BREAKING CHANGE: Removed an undocumented behavior where passing a negative count argument to retry would result in an observable that repeats forever.

@benlesh
Copy link
Member Author

benlesh commented Aug 3, 2020

In fact, I'd bet there's an issue like this with retryWhen, repeatWhen and catchError as well.

src/internal/operators/retry.ts Outdated Show resolved Hide resolved
src/internal/operators/retry.ts Outdated Show resolved Hide resolved
src/internal/operators/retry.ts Show resolved Hide resolved
…onous observables

Related: ReactiveX#5620

- Resolves an issue where all teardowns would not execute until the result observable was complete if the source was synchronous

BREAKING CHANGE: Removed an undocumented behavior where passing a negative count argument to `retry` would result in an observable that repeats forever.
BREAKING CHANGE: Leaked implementation detail `_unsubscribeAndRecycle` has been removed. Just use new `Subscription` objects
@benlesh benlesh force-pushed the fix/retry-should-sub-after-teardown branch from f506399 to 27d7834 Compare August 5, 2020 22:01
@benlesh
Copy link
Member Author

benlesh commented Aug 5, 2020

Addressed comments.

Also removed _unsubscribeAndRecycle! 🎉 🎈 🍰

@benlesh benlesh requested a review from cartant August 5, 2020 22:02
Copy link
Collaborator

@cartant cartant left a comment

Choose a reason for hiding this comment

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

LGTM, but API guardian needs to be updated, as _unsubscribeAndRecycle has been removed.

@benlesh benlesh merged commit 8f32ffe into ReactiveX:master Aug 6, 2020
@benlesh
Copy link
Member Author

benlesh commented Aug 6, 2020

🎉

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