Hi Pavel,
I’m sorry for the late response. I was too busy this week.
> Maybe we could somehow melt what you call CloseReason and CloseCode into a
> single entity? Something like (different name is an option too):
>
> public final class CloseReason {
>
> static CloseReason normalClosure(CharSequence reason);
> static CloseReason goingAway(CharSequence reason);
> // ...
> static CloseReason unexpectedCondition(CharSequence reason);
>
> static CloseReason of(int code);
> static CloseReason of(int code, CharSequence reason);
> }
>
> In an application:
>
> ws.sendClose(CloseReason.normalClosure("Too tired"));
> ws.sendClose(CloseReason.of(2016, "Leap year"));
>
> This doesn't solve the first problem of not having code description, but seems
> to solve the second one.
I think it is an acceptable solution. At least I can live with it.
>
>> And I think there should be a way to configure a timeout for sending
>> messages.
>
> As I understand timeout expiration for sendXXX would be a fatal condition (in
> general.) Simply because by the time a returned CF has completed
> exceptionally,
> some bytes of a message might have been sent already. (That's generally not
> the
> case with read timeouts see java.net.Socket#setSoTimeout)
>
> What would you expect _additionally_ from the implementation (API guarantees)
> in
> a case where send completes exceptionally with a timeout in contrast with this
> snippet?
> ws.sendBinary(giganticByteBuffer, true)
> .orTimeout(30, TimeUnit.SECONDS)
> .whenComplete((v, e) -> {
> if (e instanceof TimeoutException) {
> try {
> ws.abort();
> } catch (IOException ignored) { }
> }
> });
>
I would expect to get pre-configured CompletableFutures from the
#sendXXX-methods.
So I don’t need to call the #orTimeout()-method on every CompletableFuture
returned by
the #sendXXX-methods. But I think the usage the #orTimeout-method is also OK.
BTW, someone should describe in the JavaDocs of CompletableFuture#orTimeout()
what would happen when this method is called multiple times on the same
CompletableFuture with different arguments.
Best regards,
Andrej Golovnin
> BTW, I'm thinking of changing sendXXX methods return type to CompletionStage
> (now it's CompletableFuture), this will cause, for example,
> CompletableFuture#get(long, java.util.concurrent.TimeUnit) not to be directly
> accessible.
>
>> One minor thing off-topic:
>> While changing my code I noticed that the class
>> ProxySelector$StaticProxySelector
>> and the JavaDocs of the ProxySelector#of-method can be improved a little bit:
>>
>> diff -r 589795e4cd38 src/java.base/share/classes/java/net/ProxySelector.java
>> --- a/src/java.base/share/classes/java/net/ProxySelector.java Wed Mar
>> 23 19:33:39 2016 -0700
>> +++ b/src/java.base/share/classes/java/net/ProxySelector.java Mon Mar
>> 28 19:30:24 2016 +0200
>> @@ -165,7 +165,8 @@
>>
>> /**
>> * Returns a ProxySelector which uses the given proxy address for all HTTP
>> - * and HTTPS requests. If proxy is {@code null} then proxying is
>> disabled.
>> + * and HTTPS requests. If the given proxy address is {@code null}
>> + * then proxying is disabled.
>> *
>> * @param proxyAddress
>> * The address of the proxy
>> @@ -178,9 +179,9 @@
>> return new StaticProxySelector(proxyAddress);
>> }
>>
>> - static class StaticProxySelector extends ProxySelector {
>> + private static final class StaticProxySelector extends ProxySelector {
>> private static final List<Proxy> NO_PROXY_LIST =
>> List.of(Proxy.NO_PROXY);
>> - final List<Proxy> list;
>> + private final List<Proxy> list;
>>
>> StaticProxySelector(InetSocketAddress address){
>> Proxy p;
>> @@ -198,9 +199,9 @@
>> }
>>
>> @Override
>> - public synchronized List<Proxy> select(URI uri) {
>> - String scheme = uri.getScheme().toLowerCase();
>> - if (scheme.equals("http") || scheme.equals("https")) {
>> + public List<Proxy> select(URI uri) {
>> + String scheme = uri.getScheme();
>> + if (scheme.equalsIgnoreCase("http") ||
>> scheme.equalsIgnoreCase("https")) {
>> return list;
>> } else {
>> return NO_PROXY_LIST;
>
> CC'ing Michael.
>
