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<? extends java.nio.ByteBuffer> chunks)
and text(Iterable<? extends CharSequence> 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 validate the UTF8 locally before sending?
Or rely on the remote endpoint to close the connection with a 1002
Protocol violation or 1007 Invalid Data Consistency?
- If you will support half-closed connections, then the .close() methods
should be on WebSocket.Outgoing
- If a single message results in multiple frames on the protocol,
possibly fragmented by an extension, how does a fault half-way through the
message get communicated to the CompleteableFuture<> ?
- If chunked/streaming send/write/outgoing is eventually supported, an
exception occurs in 1 chunk or frame, that doesn't mean the message is
invalid and the websocket connection is bad, it is quite likely to be
recoverable by the client application. Even backpressure indications on
write should be able to be handled by the client application.
- The binary() and text() methods should have options to allow metadata
to tag along for the extension to work with.
eg: I want to have permessage-deflate enabled and negotiated, but i'm
smart enough to know when what i'm about to send isn't worth compressing
(it could be too small, or be already compressed).
having the ability to say .. text(msg) be compressed, and binary(myPng)
not be compressed.
WebSocket.Incoming.Chunks
- Don't like the .beginData() and .endData() as there's no information
about the type of message.
If the MUX extension gets revived (again) then this interface is
invalid.
Note: the mux extension is currently abandoned, as it was thought that
HTTP/2 could provide this feature
instead. But the lack of WebSocket over HTTP/2 spec is having people
rethink mux.
- Consider using the signature .onChunk(WebSocket ws, T chunk, boolean
final)
This handles the "reset" between messages cleanly, and contains the
type information as well.
- Why is there no WebSocket.Outgoing chunking equivalent?
This will be important for those wanting to deal with streaming
behaviors over websocket.
WebSocket.Incoming
- Where is .onPing() and .onPong() ?
- Will you allow half-closed .onClose() behavior?
meaning, if the local endpoint receives a onClose(), is it the
responsibility of the user to issue the
WebSocket.close()? or does the implementation do this automatically?
- WebSocket.onClose() signature is invalid.
Not all exceptions cause a close.
the signature should not contain a Throwable, that should be part of an
WebSocket level onError(Throwable) event.
- WebSocket.onClose() should contain the close reason code.
- WebSocket.text() should java apidoc indicating that it is *always*
UTF-8 encoded.
WebSocket
- WebSocket.close() handling ...
There are 3 close techniques to enable.
1) .close(int code, String reasonPhrase)
2) .close(int code)
3) .close()
In case #1 the close can fail due to protocol reasons because the
reasonPhrase is too large (or an invalid
close reason code is used)
If the reasonPhrase is too big, do we trim it? or throw an error to the
user?
Trimming the reasonPhrase has to be on valid UTF8 boundaries.
In case #2 the close can fail for an invalid status code.
In case #3 the close is sent without a payload, this should not cause a
failure or an exception or an error.
At this point the implementation should do its best to close (or
disconnect if the opposite side has closed as well).
- WebSocket should implement java.io.Closeable
- Where would the onError notifications for Incoming issues arrive?
- A thought, WebSocket.close() is a non-half-closed approach, with
WebSocket.Outgoing.close() being for half-closed behavior.
- big thumbs up for .suspendReceiving() and .resumeReceiving() concept
(but the location/method names seem awkward)
wouldn't having them on WebSocket.Incoming make more sense? (again this
is partly my hangup on half-closed support, and partly my desire to see the
the API be easier)
WebSocket.Incoming.suspend()
WebSocket.Incoming.resume()
Benefit here is that the sequence could be ...
Receive Frame
Parse Frame
Call method WebSocket.Incoming.on*()
while in user code, they call this.suspend() to halt reception of
more
also, a isSuspended() is probably warranted here.
General Questions
- Will the API eventually (jdk10?) grow into something to support server
side WebSocket?
(aka java.net.ServerWebSocket ?)
If so, then some of the API choices now could use small tweaks to make
them reusable later.
- Is this WebSocket API layer in stone?
or based on some sort of user/developer feedback?
or encouraged based on guidelines for java, the JDK, or classlib?
can alternate APIs be proposed still?
- Joakim Erdfelt
Eclipse/Jetty WebSocket Implementor
On Mon, Aug 31, 2015 at 7:30 AM, Pavel Rappo <pavel.ra...@oracle.com> wrote:
> 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
>
