Skip to content

FailingReactiveSocket violates spec#136

Merged
stevegury merged 1 commit intorsocket:masterfrom
NiteshKant:failure-socket
Jul 11, 2016
Merged

FailingReactiveSocket violates spec#136
stevegury merged 1 commit intorsocket:masterfrom
NiteshKant:failure-socket

Conversation

@NiteshKant
Copy link
Copy Markdown
Contributor

Problem

FailingReactiveSocket was invoking Subscriber.onError() without calling onSubscribe() which is invalid as per reactive-streams spec:

https://github.com/reactive-streams/reactive-streams-jvm#api-components

This means that onSubscribe is always signalled, followed by a possibly unbounded number of onNext signals (as requested by Subscriber) followed by an onError signal if there is a failure, or an onComplete signal when no more elements are available—all as long as the Subscription is not cancelled.

Modification

Modified to use Publishers.error() which follows the spec correctly.

Result

Better adherence of spec, happy users :)

#### Problem

`FailingReactiveSocket` was invoking `Subscriber.onError()` without calling `onSubscribe()` which is invalid as per reactive-streams spec:

https://github.com/reactive-streams/reactive-streams-jvm#api-components

```
This means that onSubscribe is always signalled, followed by a possibly unbounded number of onNext signals (as requested by Subscriber) followed by an onError signal if there is a failure, or an onComplete signal when no more elements are available—all as long as the Subscription is not cancelled.
```

#### Modification

Modified to use `Publishers.error()` which follows the spec correctly.

#### Result

Better adherence of spec, happy users :)
@stevegury
Copy link
Copy Markdown
Member

👍

@stevegury stevegury merged commit bb578d6 into rsocket:master Jul 11, 2016
@NiteshKant NiteshKant deleted the failure-socket branch July 12, 2016 22:40
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