Hi Simone, thanks for the detailed reply (as always)!
> On 13 Jul 2016, at 17:07, Simone Bordet <simone.bor...@gmail.com> wrote:
>
>> * @throws IllegalArgumentException
>> * if the {@code statusCode} has an illegal value, or
>> * {@code reason} doesn't have an UTF-8 representation not longer
>> * than {@code 123} bytes
>
> Too many negations on the reason.
Is it really too many? The only acceptable reason is the one whose UTF-8
representation's length is 123 bytes or less.
"123 bytes or less" is equivalent to "not longer than 123 bytes"
* {@code reason} doesn't have (an UTF-8 representation not longer
* than {@code 123} bytes)
> Consider "or {@code reason} has a UTF-8 representation longer than 123 bytes".
What if reason doesn't have an UTF-8 representation at all?
Maybe anyone could propose a more concise language here?
> What I don't like is that illegal status codes are signaled by
> throwing, rather than failing the CF.
> You did not explain *that* rationale :)
It has been addressed in this change:
http://mail.openjdk.java.net/pipermail/net-dev/2016-June/009912.html
You might have been on holidays (and/or too busy) around these dates, because I
haven't heard from you on this.
The rationale for IAE is that it's a programming error to stick an illegal value
in a method (given the fact the value is illegal is known a priori.)
So one should better read the javadoc before coding.
> Furthermore, it's not clear in what status is the WS being left after the
> throw.
> Is it closed with a different code ?
> Should the application try to close again ?
I think it should be pretty much the same as trying to remove an element with
index=5 from the list of 2 elements. Nothing should happen, except for the IAE
of course.
As always I'm open to arguments. If you feel this is not right, please explain.
> In general, I think you need to decide what to do with throwing exceptions.
> I like just one mechanism.
> It is always that one mechanism, everybody knows it, no clunky
> additional try/catch blocks.
What has been left after 8156693, is just IAE and NPE. We could re-consider it
later if there's a really good evidence why it should be done this way.
I've tried my best to understand both points of view [*] and to be honest... I
like the idea that everything should be relied thought CF/CS (I have had this in
one of my early API experiments), but there should be a very good argument to
persuade others.
> Consider the case of a too long reason.
> The current specification forces to perform the conversion to UTF-8
> bytes immediately because if it's too long, the implementation must
> throw.
Okay. I see your point. I must say, I heard that one before from Chris Hegarty
(probably off list). And I replied that it's a corner case. We probably
shouldn't take into account apps that bombard WebSocket with Close messages
after the first (and the only) one has been sent. This would be a crazy app!
And after all, reasons are expected to be really short strings. If there's a
long one we can do a quick check in the impl:
if (reason.length() > 123) {
throw new IAE();
} else {
// proper check
}
Simply because we know in UTF-8 a Unicode Code Point cannot be represented with
less than 1 byte. Anyhow, it's an implementation detail.
(Btw, the same is true for sendText. Although CharSequence(s) there might be
very long, it's strange to account for apps that might try to send any messages
after WebSocket has been closed. It's a programming error.)
> Consider also:
>
> ws.sendClose(1000, too_long).handle((ws, th) -> cleanup());
>
> versus:
>
> try {
> ws.sendClose(1000, too_long).handle((ws, th) -> cleanup());
> } catch (Throwable x) {
> // Uh, no idea if the implementation actually closed !
> // Let's *hope* this one goes fine :/
> ws.sendClose(1000, "short").handle((ws, th) -> cleanup());
> }
It indeed looks nice, but it could only happen when one tries to stick bad args
into sendClose. Why would you write something like this, given you are using a
correct implementation that clearly documents eligible argument's values?
Thanks,
-Pavel
--------------------------------------------------------------------------------
[*] http://cs.oswego.edu/pipermail/concurrency-interest/2016-May/015125.html
