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

[Alan Bateman]

On Thu, 28 Nov 2024 15:44:23 GMT, Volkan Yazıcı <duke at openjdk.org> wrote:

>> It should be called twice with a new created Socket.  Socket.connect is specified to throw IOException if already connected, we shouldn't be throwing (or testing for) AlreadyConnectedException here. It may be that Net.translateToSocketException is missing that mapping (we should check that).
>
> I need to verify the thrown exception in the test:
> 
> * If the socket passed created using `Socket::new`, a `SocketException("already connected")` is thrown
> * If the socket passed created using `SocketChannel.open().socket()`, an `AlreadyConnectedException` is thrown
> 
> AFAICT, the verifier needs to be passed along with the socket, and this implies either separate tests, which is what the current code does, or a separate test method source as in
> 
> 
> static List<Arguments> connectedSocketFactoriesAndReconnectFailureVerifiers() {
>     return List.of(
>             Arguments.of(
>                     (Function<SocketAddress, Socket>) ConnectFailTest::createConnectedSocket,
>                     (Consumer<Executable>) executable -> {
>                         SocketException exception = assertThrows(SocketException.class, executable);
>                         assertEquals("already connected", exception.getMessage());
>                     }),
>             Arguments.of(
>                     (Function<SocketAddress, Socket>) ConnectFailTest::createConnectedNioSocket,
>                     (Consumer<Executable>) executable -> assertThrows(AlreadyConnectedException.class, executable))
>     );
> }
> 
> 
> Am I mistaken?

I checked the socket adaptor and there is needed a bug here. SocketChannel throws AlreadyConnectedException and the socket adaptor should map that to SocketException("already connected"). It doesn't do that so we should fix it.

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

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