{"action":"created","issue":{"url":"https://api.github.com/repos/reactive-streams/reactive-streams-jvm/issues/271","labels_url":"https://api.github.com/repos/reactive-streams/reactive-streams-jvm/issues/271/labels{/name}","comments_url":"https://api.github.com/repos/reactive-streams/reactive-streams-jvm/issues/271/comments","events_url":"https://api.github.com/repos/reactive-streams/reactive-streams-jvm/issues/271/events","html_url":"https://github.com/reactive-streams/reactive-streams-jvm/issues/271","id":85334848,"number":271,"title":"onError(Throwable) vs cancel()","user":{"login":"gregw","id":70829,"avatar_url":"https://avatars.githubusercontent.com/u/70829?v=3","gravatar_id":"","url":"https://api.github.com/users/gregw","html_url":"https://github.com/gregw","followers_url":"https://api.github.com/users/gregw/followers","following_url":"https://api.github.com/users/gregw/following{/other_user}","gists_url":"https://api.github.com/users/gregw/gists{/gist_id}","starred_url":"https://api.github.com/users/gregw/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gregw/subscriptions","organizations_url":"https://api.github.com/users/gregw/orgs","repos_url":"https://api.github.com/users/gregw/repos","events_url":"https://api.github.com/users/gregw/events{/privacy}","received_events_url":"https://api.github.com/users/gregw/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":5,"created_at":"2015-06-04T23:52:04Z","updated_at":"2015-06-09T01:53:12Z","closed_at":null,"body":"\r\n@viktorklang Bringing this discussion back here from http://cs.oswego.edu/pipermail/concurrency-interest/2015-June/014275.html\r\n\r\nI understand your comments about the expense of bidirectional communication and general acknowledgements. However, I also think that leaving the establishment of a back flow for such things as recycling outside the standard is leaving a bit of a hole for interoperability. But let me raise that separately.\r\n\r\nFor now, I want to understand more about the error handling, specifically in a chain of Processors.    For any particular processor, it is possible that something can go wrong and when it does both ends of the RS need to be informed.    Currently the API allows for errors to be propagated by calling onError(Throwable) down stream and cancel() upstream.\r\n\r\nThis is sufficient for shutting down the stream, but my question is why is the exception passed down not up?    Essentially the stream can be shutdown without passing the exception in either direction and the throwable is not needed:\r\n\r\n```java\r\nSubscriber{\r\n  ...\r\n  void cancel();\r\n}\r\n\r\nSubscription{\r\n  ...\r\n  void onError();\r\n}\r\n```\r\n\r\nSo given that as a conceptual starting point, can the usability of the API be improved by telling one end or the other the details of a problem?   \r\n\r\nI'm struggling to think of an exception that the ultimate subscriber can use to improve it's implementation.   All it really needs to know is that the stream is not completing normally.  \r\n\r\nOn the other hand, I can think of several examples where passing the exception upstream to the origin publisher will help the usability of the API.   If a publisher is delivering an unsuitable object (it is incomplete, illegal state, can't be serialized, contains types that have no json convertor, don't match DB schema etc.), then telling that publisher what is the problem will be valuable in helping them correct their Publisher.   Sure the exception can be logged by either the Processor or the ultimate Subscriber, but that might be on a remote system to the Publisher and they may not see the log.\r\n\r\nSo if a Publisher is sending an unsuitable item, all they will see is a cancel call in return and no explanation why.\r\n\r\nSo if the exception is to be propagated, sending it upstream seams to me more appropriate than sending it downstream.   But not opposed to sending it both ways:\r\n\r\n```java\r\nSubscriber{\r\n  ...\r\n  void onError(Thowable);\r\n}\r\n\r\nSubscription{\r\n  ...\r\n  void onError(Throwable);\r\n}\r\n```\r\n\r\nWith the spec having suitable protections about looping errors.\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/reactive-streams/reactive-streams-jvm/issues/comments/110195793","html_url":"https://github.com/reactive-streams/reactive-streams-jvm/issues/271#issuecomment-110195793","issue_url":"https://api.github.com/repos/reactive-streams/reactive-streams-jvm/issues/271","id":110195793,"user":{"login":"gregw","id":70829,"avatar_url":"https://avatars.githubusercontent.com/u/70829?v=3","gravatar_id":"","url":"https://api.github.com/users/gregw","html_url":"https://github.com/gregw","followers_url":"https://api.github.com/users/gregw/followers","following_url":"https://api.github.com/users/gregw/following{/other_user}","gists_url":"https://api.github.com/users/gregw/gists{/gist_id}","starred_url":"https://api.github.com/users/gregw/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gregw/subscriptions","organizations_url":"https://api.github.com/users/gregw/orgs","repos_url":"https://api.github.com/users/gregw/repos","events_url":"https://api.github.com/users/gregw/events{/privacy}","received_events_url":"https://api.github.com/users/gregw/received_events","type":"User","site_admin":false},"created_at":"2015-06-09T01:53:12Z","updated_at":"2015-06-09T01:53:12Z","body":"@viktorklang,\r\n\r\nso I'm definitely talking about failures here and not errors.   I get it you're saying that errors should be handled in some form of ack/nack back flow.... so let's talk about that another time once I've got failure handling nailed down.\r\n\r\nThe first failure type I'm concerned with is spec violations.  You say that an implementation much check for compliance and I totally agree.  My question is what should I do once non compliance is detected.  I would very much like to have code like this:\r\n```java\r\n@Override void onNext(Object item)\r\n{\r\n    if (requests.decrementAndGet()<0)\r\n        throw new IllegalStateException(\"unrequested call to onNext\");\r\n    ...\r\n}\r\n```\r\n\r\nBut that is against the spec, as I can't throw.   So how do I inform the possibly remote publisher that it has screwed up?  The only spec compliant mechanism I have is to cancel the Subscription, which is hardly a good way of signalling such a fundamental failure!      I think it would be fine for the spec to allow an implementation to throw an ISE if it detects non spec compliant behaviour.\r\n\r\nOn the other type of failure,  I acknowledge your observation about the truths of the subscriber vs the truths of the publisher.   It is true that they are not the same and that which view point you take will definitely influence how failures should be handled.   So let's consider another example....\r\n\r\nImagine an asynchronous webapplication that is using RS based database and IO.   For a given request from the client, it needs to perform a database query that may return many elements, each of which will have some business logic applied, perhaps another layer of filtering, then converted to JSON, then encoded as bytes, then compressed and finally sent as a HTTP2 data frame.    Let's implement each of these steps as an asynchronous RS processor so from origin publisher to ultimate subscriber we would setup something like:\r\n\r\nDBQueryPublisher->MyBusinessLogic->BusinessFilter->JSONConvertor->StringEncoder->Compressor->HTTP2Stream\r\n\r\nNow let's say the objects passing through this RS are polymorphic up until the JSONConvertor stage, when it uses some extensible JSONConverter to turn the object into string(s).\r\n\r\nIf a particular object comes through the RS that does not have a JSON Conversion available for that type, then I see that as a Failure.  It's not a normal error, as it is something the developer forgot to provide or didn't imagine could happen. The app is broken.\r\n\r\nSo who can we tell about it?  Currently the API would allow a  onError(new NoSuchJSONConvertor()) to be sent downstream.  But what has that got to do with the StringEncoder, Compressor or even the HTTP2Stream?  They are all just going to treat it as an arbitrary failure and ignore the passed exception.\r\n\r\nBut I also see that there is little that the DBQueryPublisher could do with a noSuchJSONConvertor exception.  As you say, it's truth is that the item it published was a valid response to the query it was given.  All it cares about is if the RS is cancelled or not.   \r\n\r\nYet I don't see that as generally the case, as in this example it is the truth of the MyBusinessLogic and/or BusinessFilter processors that it has assumed that all the objects it receives from is query subscription are fit to be published on its downstream subscribers.     This is a wrong assumption and constitutes a failure... but a failure they will not be informed about as all they see is a cancel() coming upstream, which is indistinguishable from the very common case of the client browser just suspended or went out of wifi range.\r\n\r\nI think it is very important for the BusinessLogic/BusinessFilter to be able to distinguish between the case of the network connection closed (common occurrence and is not a failure and is well captured by cancel semantics) vs a failure caused by an incorrect assumption that the objects they are publishing are suitable for the RS they are publishing to.\r\n\r\nThus in this case, I see that the NoSuchJSONConvertor failure should be able to propagate upstream to the BusinessLogic/Business/Filter at least.   It is then a business decision if that failure is fatal (and they cancel the RS) or if they log and ignore it and continue publishing the other elements they receive. \r\n\r\nI do not think this is a case where setting up a backchannel makes sense.  The failure occurs because the developer didn't think there would be any objects that could not be converted, so they wouldn't think to provide a back channel for failures they don't think could occur.\r\n\r\nSo I now think that there is onError(Throwable) and cancel() semantics needed both upstream and downstream.\r\n\r\n\r\ncheers\r\n\r\n\r\n\r\n \r\n\r\n"}}