What to do when contract is violated

[abersnaze]

I was looking at a discussion in the RxJS related to what happens when onNext throws an exception. I'll try to summarize with a simple example. Lets say the map operator is implemented like this:

void onNext(T t) {
  try {
    R r = transformer.call(t); // could throw
    o.onNext(r); // shouldn't throw but could
  } catch(Throwable e) {
    o.onError(e);
  }
}

And the not so good implementation of a downstream subscriber is:

void onNext(Number r) {
  Number s = 1000/r; // could throw a divide by zero or null pointer exception
  o.onNext(s); // shouldn't throw but could
}
void onError(Throwable e) {
  // handle getting bad divisor from producer
  // but also has to handle failure from the onNext above.
}

• Some are arguing that the exceptions from an Observer's onNext should not be sent to the same Observer's onError because it means that the onError has to be able to handle two different types of errors: errors from the producer and errors from itself.
• Others are arguing that the only code to blame for the confusing situation of having to deal with different sources of the errors is the onNext implementation in the same class as the onError and can be easily solved by wrapping the body of the onNext with a try/catch.

The first question to be answered: is this a problem that should be fix? Yes or No please. We'll get into if it is even possible and the how later.

[akarnokd]

No.

[benlesh]

I wouldn't fix this for RxJava 1. I would definitely fix this for RxJava 2. FWIW, RxJava is the only version of Rx that does this.

To be clear, this is not to say that Observers/Subscribers that are part of some operator's internals can't behave this way. ... but I'd caution that it's probably extremely rare that an operator would want to hand an error back to it's own observer's error handler, but rather it would probably want to send it on down the line to the next observer in the chain. My 2 cents.

Perhaps the better answer here is composing Observers that you use to consume at the tail end. Observers, after all, are just as composable as Observables. source.subscribe(observer1.then(observer2)) or something like that. (but probably not then, I don't know)

cc/ @headinthebox @benjchristensen

[abersnaze]

This is also relevant to the discussion.
https://github.com/reactive-streams/reactive-streams-jvm/blob/v1.0.0/README.md#user-content-2.13

Calling onSubscribe, onNext, onError or onComplete MUST return normally except when any provided parameter is null in which case it MUST throw a java.lang.NullPointerException to the caller, for all other situations the only legal way for a Subscriber to signal failure is by cancelling its Subscription. In the case that this rule is violated, any associated Subscription to the Subscriber MUST be considered as cancelled, and the caller MUST raise this error condition in a fashion that is adequate for the runtime environment.

[akarnokd]

RS Subscriber methods should not throw and since RxJava 2.x doesn't let in nulls in, there is no reason for throwing anything. Operators don't throw and route exceptions from lambdas to the onError rail.

Operators still may fail in corner cases such as stack overflow or OOME which currently isn't handled (ie. no throwIfFatal) because of §2.13 allows a few options that need decisions. In this case, usually the closest catch around a lambda invocation or the top Thread method will get it.

It really depends on how well the users of the library is trusted with their Subscribers. There is a subscribeSafe method which wraps the incoming subscriber and does a lot of checks. But the default subscription mode is direct without safeguards for performance.

Currently, there are around 200 unit test that are non-RS conformant and are thus disabled; these include a bunch of null users and onNext throwers.

In addition, there is a policy that no onErrors should get lost, especially those coming after the terminal state of certain operators, so these are routed to RxJavaPlugins.onError (at least this was the intent, Rsc is more eager on this front but both could use another review round).

Bottom line is that I don't think this is an issue for us.

[stealthcode]

I think the goal of differentiating errors (user errors from library errors) lends towards the goal of ease of use and contribution. For instance, the solution may be a write up of what standard behavior should be conformed to if you write a subscriber to be manually lifted (or an Operator). Or writing basic tests that ensure the functionality of conforming operators.

These goals are worthwhile as they make it easier for others to contribute. It's nice that we may not have high exposure to these problems (at the moment) but if we continue to gain more and more external contribution it would be a good idea to have some framework (educational or computationally enforced) that provided assurance that operators and subscribers meet some fundamental standard.

[abersnaze]

ping @benjchristensen @headinthebox

[akarnokd]

Any more on this or can it be closed?

[akarnokd]

I'm closing this issue due to inactivity. If you have further input on the issue, don't hesitate to reopen this issue or post a new one.