Hello Rossen,
> On 22 Feb 2018, at 02:51, Rossen Stoyanchev <rstoyanc...@pivotal.io> wrote:
>
> hi,
>
> > Simone Bordet:
> > As the WebSocket APIs were moved to incubation and are now slated
> > for JDK 11, it may be worth considering again whether it's the
> > case to provide a Flow-based APIs.
>
> I second that. There were two main issues pointed out in this thread:
>
> > The first one is how to communicate errors back to the user's
> > code that publishes messages to WebSocket.
>
> Isn't it mostly low level, transport errors about which the application can't
> do much about, but clean up and close the connection? Arguably a cancellation
> is adequate, or do you have more specific errors and cases in mind?
> Alternatively, the JSR-356 WebSocket API provides a single onError callback
> on the Endpoint type. I haven't come across any limitations around that.
>
I can't see how Flow (RS) can be seamlessly married with this low-level
WebSocket API. Maybe you might help.
1. If there is no error reporting, then how would the user know if there's
something wrong with the data they send? I'm talking about incomplete/malformed
UTF-8, fragments intermixing, sending messages after a Close message has been
sent, etc. How should the user troubleshoot a situation where the WebSocket's
internal Subscriber cancels the subscription unexpectedly? We can't perform all
the checks during a message creation. Some messages could not be checked up
until they appear in some context. For example, a text fragment is a okay per
se. But if it appears in the midst of a fragmented binary message, it is not
okay. Should the API not allow fragmented (partial) messages then? Or should we
provide error reporting out-of-band in relation to Flow? If we allow only whole
messages, then we are not flexible enough to support streaming. In other words
the case of an arbitrarily large message that does not have to be assembled
prior to consumption.
2. How should the user close their side of the communication? There seems to be
a mismatch in the protocols. WebSocket is bidirectional and relies on Close
messages. Flow is unidirectional-ish and relies on cancellation through
onComplete/onError and cancel signals. If the user has no more messages to
publish, the user may wish to send a Close message with a status code and a
reason. If instead we rely on onComplete/onError being called then we loose the
ability to provide these extra pieces of information. What should we choose
here? To not allow a Close message to be sent or to require the user to signal
onComplete() after a Close message has been signalled through onNext(). Maybe
there should not be a representation of a Close message at all. Maybe we should
leave it as a protocol low-level detail. But then, how should the API deliver
the received status code and the reason of a Close message it receives? Should
it translate the received Close message to onComplete()/onError() depending on
this message's contents?
3. WebSocket protocol mandates a peer should send a Close message as soon as
practical after receiving one. In the current API it is implemented using a
CompletionStage returned from the WebSocket.Listener.onClose method. How should
we implement this using Flow?
Having said that, it's either (but not both):
a) We ditch low-level API (to Ping/Pong/Close and (possibly) incomplete
messages and start using Flow, or
b) We leave the API as it is and instead provide a Flow-adaptor suitable
for the anticipated most common use case. As for now let's not dwell into
details of who and when provides this adaptor.
>
> <snip>
>
> James Roper mentioned this already, but the Netty PooledByteBufAllocator
> demonstrates it's feasible to recycle buffers transparently without the need
> for a per-message confirmation. I don't think the argument holds that not
> having per message completion signals forces unnecessary allocations, garbage
> creation and copying.
>
Sure one may recycle buffers transparently. But only if the
buffer type supports it. io.netty.buffer.PooledByteBufAllocator provides
instances of io.netty.buffer.ByteBuf. Not java.nio.ByteBuffer. The latter one
does not have any release/free/dispose/etc. methods and semantics.
>
> There were a "dozen of smaller issues" mentioned. However there were no links
> to earlier discussions, so I'm not sure what those were.
>
If you want you can spend some time tracking the evolution of the API:
http://mail.openjdk.java.net/pipermail/net-dev/2015-September/009079.html
http://mail.openjdk.java.net/pipermail/net-dev/2015-October/009223.html
http://mail.openjdk.java.net/pipermail/net-dev/2015-October/009259.html
http://mail.openjdk.java.net/pipermail/net-dev/2016-March/009616.html
http://mail.openjdk.java.net/pipermail/net-dev/2016-April/009632.html
http://mail.openjdk.java.net/pipermail/net-dev/2016-April/009702.html
http://mail.openjdk.java.net/pipermail/net-dev/2016-May/009749.html
http://mail.openjdk.java.net/pipermail/net-dev/2016-May/009868.html
http://mail.openjdk.java.net/pipermail/net-dev/2016-June/009872.html
http://mail.openjdk.java.net/pipermail/net-dev/2016-June/009894.html
http://mail.openjdk.java.net/pipermail/net-dev/2016-June/009912.html
http://mail.openjdk.java.net/pipermail/net-dev/2016-June/009925.html
http://mail.openjdk.java.net/pipermail/net-dev/2016-June/009985.html
http://mail.openjdk.java.net/pipermail/net-dev/2016-July/010081.html
http://mail.openjdk.java.net/pipermail/net-dev/2016-August/010155.html
http://mail.openjdk.java.net/pipermail/net-dev/2017-July/010872.html
http://mail.openjdk.java.net/pipermail/net-dev/2017-December/011034.html
I'm sorry I don't have more relevant links to hand.
>
> The main purpose of Reactive Streams is to connect components from different
> libraries. The currently proposed WebSocket API cannot be easily integrated
> into a Reactive Streams pipeline. One would have to build a bridge to
> Reactive Streams first and I don't think that should be left as a separate
> exercise, since Flow is in the JDK and it's the reason why it was introduced,
> to provide a common contract for such scenarios.
>
I completely understand the value of RS (Flow) and personally think it has a lot
of scenarios it is applicable to. I'm not sure it is the case here though.
> <snip>
P.S. How may purely RS WebSocket APIs are there? If there are some, we can
probably
learn from their experience and make things a bit better. Otherwise I don't see
why we should create one more of them. Would you rather have something
low-level, you could then implement your high-level API that is most relevant to
your case?
Thanks,
-Pavel
