Collapses 2.13 and 2.14 into one and changes the wording of the rule to ... #117
Conversation
| | <a name="1.7">7</a> | Once a terminal state has been signaled (`onError`, `onComplete`) it is REQUIRED that no further signals occur. Situational scenario MAY apply [see 2.13] | ||
| | <a name="1.8">8</a> | `Subscription`'s which have been canceled SHOULD NOT receive subsequent `onError` or `onComplete` signals, but implementations will not be able to strictly guarantee this in all cases due to the intrinsic race condition between actions taken concurrently by `Publisher` and `Subscriber` | | ||
| | <a name="1.9">9</a> | `Publisher.subscribe` SHOULD NOT throw a non-fatal `Throwable`. The only legal way to signal failure (or reject a `Subscription`) is via the `onError` method. Non-fatal `Throwable` excludes any non-recoverable exception by the application (e.g. OutOfMemory) | | ||
| | <a name="1.7">7</a> | Once a terminal state has been signaled (`onError`, `onComplete`) it is REQUIRED that no further signals occur |
There was a problem hiding this comment.
I suspect that this rule is now not needed and can be completely covered by 1.6
There was a problem hiding this comment.
True, but 1.7 is very simple to understand I think, if it were me I'd keep it.
There was a problem hiding this comment.
What about:
| 6 | If a Publisher signals either onError or onComplete on a Subscriber, that Subscriber’s Subscription MUST be considered canceled and it is REQUIRED that no further signals to it occur.
|
A 1-week-later-ping to @reactive-streams/contributors |
|
SECOND PING @reactive-streams/contributors |
| @@ -117,8 +117,7 @@ public interface Subscriber<T> { | |||
| | <a name="2.10">10</a> | A `Subscriber` MUST be prepared to receive an `onError` signal with or without a preceding `Subscription.request(long n)` call | | |||
| | <a name="2.11">11</a> | A `Subscriber` MUST make sure that all calls on its `onXXX` methods happen-before [1] the processing of the respective signals. I.e. the Subscriber must take care of properly publishing the signal to its processing logic | | |||
| | <a name="2.12">12</a> | `Subscriber.onSubscribe` MUST NOT be called more than once (based on object equality) | | |||
| | <a name="2.13">13</a> | A failing `onComplete` invocation (e.g. throwing an exception) is a specification violation and MUST signal `onError` with `java.lang.IllegalStateException`. The cause message MUST include a reference to this rule and/or quote the full rule | | |||
| | <a name="2.14">14</a> | A failing `onError` invocation (e.g. throwing an exception) is a violation of the specification. In this case the `Publisher` MUST consider a possible `Subscription` for this `Subscriber` as canceled. The `Publisher` MUST raise this error condition in a fashion that is adequate for the runtime environment (e.g. by throwing an exception, notifying a supervisor, logging, etc.). | | |||
There was a problem hiding this comment.
Remove the spec214_failingOnErrorInvocation methods from the TCK as well as part of this PR then?
There was a problem hiding this comment.
That's a good point. Will do after I get some more feedback on the PR.
There was a problem hiding this comment.
@ktoso I'm creating an Issue to update the TCK for the new 2.13
|
23 days later, ping @reactive-streams/contributors |
|
👍 |
|
This all looks OK to me, with one further suggestion. Consider transforming "Subscriptions to just something like: Upon cancellation, a subscriber MUST eventually stop receiving onError You could add something about promptness of shutting down signals being a quality of implmenetation issues, but why bother. |
|
After @DougLea suggestion: 👍 |
|
The plural nature of "onError or onComplete signals" doesn't seem quite right, especially ignoring If we're going to genericize this statement then I think it's more accurate to say:
I suggest this because |
|
@benjchristensen I like "Upon cancellation, a subscriber MUST eventually stop receiving signals." |
|
👍 |
|
When adding it I realized it harmonizes better with 1.3, 1.4 and 1.5 if it is worded like this: | 8 | If a I think the above is a keeper, but wdyt? |
|
The rewording is good. I'm fine with either. |
viktorklang
force-pushed
the
wip-106-√
branch
from
d1aff11 to
a7c9a23
Compare
…to be positive. Rewords 1.8 so that it is simpler and positively worded. Reformulates 1.9 in the same wording as 2.13.
viktorklang
force-pushed
the
wip-106-√
branch
from
a7c9a23 to
c95c8a2
Compare
|
@reactive-streams/contributors Please give a 👍 or object to these changes before the 24th of October GMT+0 |
|
@reactive-streams/contributors Only 👍's and no objections. Merging |
Collapses 2.13 and 2.14 into one and changes the wording of the rule to ...
...be positive.
Also includes minor reformatting.
As discussed in #106, with positive rule focus suggested by @DougLea
Review by @reactive-streams/contributors