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

2.x: Reactive Streams Rule 3.9 violations #5274

Closed
artem-zinnatullin opened this issue Apr 8, 2017 · 2 comments
Closed

2.x: Reactive Streams Rule 3.9 violations #5274

artem-zinnatullin opened this issue Apr 8, 2017 · 2 comments

Comments

@artem-zinnatullin
Copy link
Contributor

Rule 3.9 of Reactive Streams:

While theSubscription is not cancelled, Subscription.request(long n) MUST signal onError with a java.lang.IllegalArgumentException if the argument is <= 0. The cause message MUST include a reference to this rule and/or quote the full rule.

Current implementation of RxJava v2 seems to violate this rule in two places:

  1. Signal with IllegalArgumentException is delivered to the RxJavaPlugins.onError() and not the Subscriber.onError().

Tests:

@Test
public void negativeRequestShouldSignalOnError() {
    TestSubscriber<Integer> ts = TestSubscriber.create(0);
    Flowable.just(1).subscribe(ts);
    
    ts.request(-1);
    
    ts.assertError(IllegalArgumentException.class);
}

@Test
public void zeroRequestShouldSignalOnError() {
    TestSubscriber<Integer> ts = TestSubscriber.create(0);
    Flowable.just(1).subscribe(ts);

    ts.request(0);

    ts.assertError(IllegalArgumentException.class);
}
  1. Cause message does not include reference to the rule nor quotes it.

Test:

@Test
public void illegalRequestShouldReferenceRuleOrQuoteIt() {
    TestSubscriber<Integer> ts = TestSubscriber.create(0);
    Flowable.just(1).subscribe(ts);

    ts.request(-1);

    String cause = ts.errors().get(0).getMessage();
    
    assertTrue(cause.contains("3.9") || cause.contains("While the Subscription is not cancelled, Subscription.request(long n) MUST signal onError with a java.lang.IllegalArgumentException if the argument is <= 0. The cause message MUST include a reference to this rule and/or quote the full rule. The intent of this rule is to prevent faulty implementations to proceed operation without any exceptions being raised. Requesting a negative or 0 number of elements, since requests are additive, most likely to be the result of an erroneous calculation on the behalf of the Subscriber."));
}

I can open two separate PRs to fix these violations if issue will be approved.

@akarnokd
Copy link
Member

akarnokd commented Apr 8, 2017

@artem-zinnatullin
Copy link
Contributor Author

Aha, I see, so when we use RxJava v2 specific Subscriber types — 3.9 is relaxed, but if Reactive Streams Subscriber subscribes — these rules are not violated.

👍 what a slowpoke am I…

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

No branches or pull requests

2 participants