RFR: 8343791: Socket.connect API should document whether the socket will be closed when hostname resolution fails or another error occurs [v23]

[Volkan Yazıcı]

On Fri, 29 Nov 2024 11:38:58 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Volkan Yazıcı has updated the pull request incrementally with three additional commits since the last revision:
>> 
>>  - Use more fine-grained suppression
>>  - Further simplify tests
>>  - Extend `translateToSocketException()` with `AlreadyConnectedException`
>
> src/java.base/share/classes/sun/nio/ch/Net.java line 171:
> 
>> 169:             nx = newSocketException("Already bound");
>> 170:         else if (x instanceof AlreadyConnectedException)
>> 171:             nx = newSocketException("already connected");
> 
> Probably better to use "Already connected" here, that will mean it has the same case as the exception message in SocketAdaptor.connect.

I deviated from the first-letter-in-upper-case convention deliberately to match the message in `Socket`. Are you sure you want me to replace `already connected` with `Already connected` in `Net`?

> test/jdk/java/net/Socket/ConnectFailTest.java line 72:
> 
>> 70: 
>> 71:     /**
>> 72:      * Verifies socket is closed when {@code unboundSocket.connect()} fails.
> 
> Did you mean to say "unboundSocket.connect"? I assume to mean to that it tests that an unbound Socket is closed when connect fails.

The current Javadoc is as follows:


Verifies socket is closed when {@code unboundSocket.connect()} fails.


Doesn't this express what you want me to say? How shall I rephrase it?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/22160#discussion_r1863527844
PR Review Comment: https://git.openjdk.org/jdk/pull/22160#discussion_r1863525183