Skip to content

Fix Observer#OnSubscribe contracts#1

Merged
ZacSweers merged 1 commit intomasterfrom
z/fixedOnSubscribe
Dec 23, 2016
Merged

Fix Observer#OnSubscribe contracts#1
ZacSweers merged 1 commit intomasterfrom
z/fixedOnSubscribe

Conversation

@ZacSweers
Copy link
Collaborator

Prior, we weren't guaranteed to send an onSubscribe() callback to the wrapped observer if there was a lifecycle issue (pre-post lifecycle). This ensures that by doing the check in dispose()/cancel() and triggering it to be safe, while still preventing a full completion upstream (especially problematic for scalar flowables, which could otherwise completely complete in onSubscribe())

@ZacSweers ZacSweers added this to the 0.1.0 milestone Dec 23, 2016
@ZacSweers ZacSweers changed the title Fix RecordingObserver#OnSubscribe contracts Fix Observer#OnSubscribe contracts Dec 23, 2016
Prior, we weren't guaranteed to send an `onSubscribe()` callback to the wrapped observer if there was a lifecycle issue (pre-post lifecycle). This ensures that by doing the check in `dispose()`/`cancel()` and triggering it to be safe, while still preventing a full completion upstream (especially problematic for scalar flowables, which could otherwise completely complete in onSubscribe())
Copy link
Contributor

@tonycosentini tonycosentini left a comment

Choose a reason for hiding this comment

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

Looks good, although I'm not particularly qualified to understand the subtle details here :)

It would be cool to have a way to generate these at some point - if there's one more type it'll be kind of tedious.

@ZacSweers ZacSweers merged commit 8a13bd0 into master Dec 23, 2016
ZacSweers added a commit that referenced this pull request Feb 16, 2017
Prior, we weren't guaranteed to send an `onSubscribe()` callback to the wrapped observer if there was a lifecycle issue (pre-post lifecycle). This ensures that by doing the check in `dispose()`/`cancel()` and triggering it to be safe, while still preventing a full completion upstream (especially problematic for scalar flowables, which could otherwise completely complete in onSubscribe())
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