WebSocket client API

2015-08-31 Thread Pavel Rappo
Hi,

I would appreciate if you help to review a WebSocket client API proposed for
JDK 9. This work is a part of JEP-110 [1] and has started its public path with
HTTP client review in March this year [2].

Proposed WebSocket API is relatively small and focuses on convenient exchange of
data in a fully asynchronous fashion. API consists of 6 types located in the
java.net package [3]:

1. WebSocket
2. WebSocket.Builder
3. WebSocket.Incoming
4. WebSocket.Incoming.Chunks
5. WebSocket.Outgoing
6. WebSocketException

Starting point is a class description for java.net.WebSocket. Along with
in-javadoc examples, several API test samples are provided in the webrev [4] and
named test/java/net/WebSocket/Example%.java. They are only for informational
purposes and won't be included in the final version of the API.

I would appreciate any feedback on this API. Thanks.

---
[1] http://openjdk.java.net/jeps/110
[2] http://mail.openjdk.java.net/pipermail/net-dev/2015-March/008932.html
[3] http://cr.openjdk.java.net/~prappo/8087113/javadoc.00/
[4] http://cr.openjdk.java.net/~prappo/8087113/webrev.00/

-Pavel


Re: WebSocket client API

2015-08-31 Thread Joakim Erdfelt
java.net.WebSocket

In general, I like the simpler approach that JEP110 is taking (over how
javax.websocket and JSR356 did it)
(MessageHandlers, Configurators, Decoders, and Encoders, oh my!)

Here's my first pass through the javadoc and JEP110 (with a focus on
WebSocket)

Upgrade:

  - There's 2 ways some of the websocket headers can be declared, and 1 way
for others.
  Origin   - easy enough, only in HttpClient.
  Sec-WebSocket-Protocol   - easy enough to declare as header in
HttpClient, but it also exists in WebSocket.Builder (with a nice
interface), but what happens if both are used?
  Sec-WebSocket-Extensions - this is tricky to declare correctly, the
extensions themselves should be the canonical source for a properly
formatted Extension (with parameters) for this header.
 I think it's a mistake to rely on only
String here.
 See the history of extension configuration
strings in the various PMCE specs.
 Extensions should be able to be
initialized as Objects and passed to the
 WebSocket.Builder.extensions() list here.
  - Make sure that adding Cookies to the upgrade is bullet-proof, its the
feature with the most questions on
stackoverflow (for all languages, even the Javascript WebSocket API)
  - Errors during upgrade are communicated only via the WebSocket.create()
and WebSocket.createAsync() exceptions.
Access to the HTTP Response (headers, and status codes) are very
important, even when using the
non HttpClient version via WebSocket.newBuilder(String uri)
  - Add new WebSocket.newBuilder(URI) method for formal URI use
  - The use of HttpClient is a nice touch, seems like it would allow reuse
of an existing physical HTTP/2
connection to establish a new WebSocket channel (once the Websocket
over HTTP/2 spec is formalized)


Extensions:

  - Extensions should be better formalized, real objects, or at least real
configuration objects.
  - New extension implementations should be able to be provided.
  - WebSocket.extensions() should have apidoc indicating that it is list of
extensions that were negotiated during the upgrade.
  - How can someone plug in a new extension implementation?
  - Will JDK9 ship with permessage-deflate enabled by default?
If so, will the java.util.zip.Deflater/Inflater support controlling the
LZ77 Sliding Window Size?
See:
https://tools.ietf.org/html/draft-ietf-hybi-permessage-compression-28#section-7.1.2
  - There should be possible to have extensions that are invisible and only
exist on the local endpoint
(such as debugging / identity / analysis extensions)
  - Extensions should be configurable for features that exist outside of
the extension negotiation parameters.
eg: permessage-deflate configured to only compress outgoing TEXT and
not BINARY messages as the BINARY messages are images or other
precompressed data.
permessage-deflate configured for a minimum message size before
compression kicks in for outgoing messages.


Async Send

  - What happens if a user spams sendAsync() too fast?
Is there some sort of immediate feedback about write back-pressure?
or is this information entirely within the CompletableFuture for the
user to handle?
if so, how?
Do subsequent sendAsync() event block?
or do all sendAsync() calls queue up?  If so, how big is the queue?
  - Is there a way to ask the if a write can proceed without blocking?
  - If there is queueing? can it be controlled?
eg:
 1) queue up X frames or bytes then write? (useful for lots of small
writes)
 2) queue only X frames or bytes, then toss warnings (useful for
backpressure)
 3) no queue in java, only rely on the OS write queue, if that's full
toss warning.
 4) allow queue to build up, but also allow cancel/remove of queued
items by user (for stream control).


WebSocket.Outgoing

  - What does binary(Iterable chunks)
and text(Iterable chunks) do?
Is there an expectation that each chunk is preserved as a websocket
frame?
Or is this treated similarly to how
GatheringByteChannel.write(ByteBuffer[]) functions?

I would advocate that the call represents a single WebSocket message,
and the individual chunks
can be merged/split/fragmented/mangled to suit the implementation
and/or extension behavior as well.
  - Where's the .ping() or .pong() ?
  - text() should have apidoc indicating that *only* UTF-8 is supported.
  - text() based on CharSequence is an interesting choice.  how will you
handle non-UTF8 encoded Strings?
Will you always (internally) use
CharSequence.toString().getBytes(StandardCharset.UTF_8) to get
the raw bytes before sending them to the remote endpoint?  (thus
potentially mangling / replacing
the bad code points with replacement chars)
You can't rely on the CharSequence.codePoints() or .chars() as the
encoding could be invalid.
Will you validat