On Wed, 20 Nov 2024 18:12:39 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> Volkan Yazıcı has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Replace `UnknownHostException exception` with `var uhe`
>> - Fix `connect()` Javadoc on `UHE` results in socket close
>
> src/java.base/share/classes/java/net/Socket.java line 637:
>
>> 635: * is already connected or the socket is closed
>> 636: * @throws UnknownHostException if the endpoint is an unresolved
>> 637: * {@link InetSocketAddress}
>
> It looks like you deleted the throws SocketTimeoutException by mistake.
Restored in 7d96bffcb8a93f3730d30f6d0d2edb49d701f465. (I misread what you meant
[in this
comment](https://github.com/openjdk/jdk/pull/22160#discussion_r1849829296), and
hence removed.)
> src/java.base/share/classes/java/net/Socket.java line 1607:
>
>> 1605: } catch (IOException exception) {
>> 1606: if (exception != parentException) {
>> 1607: parentException.addSuppressed(exception);
>
> I don't think you need the identity check, close would be very broken if it
> threw the enclosing exception.
Removed in 385d6b24500ff76d8aa4395dc9c8bbbdadb68ec1. (That improvement was
suggested by @jaikiran.)
> src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java line 567:
>
>> 565: InetSocketAddress isa = (InetSocketAddress) remote;
>> 566: if (isa.isUnresolved()) {
>> 567: throw new UnknownHostException("Unresolved address: " +
>> isa.getHostName());
>
> The changes means this change isn't needed now. I'm really tempted to say we
> change the unsupported address type to throw UOE, and the unresolved case to
> throw IAE, but don't have the complicated this PR any more, plus the
> SocketImpl API is hugely under specified so might result in more discussion
> that is worth it.
Reverted in d3568d16cd227940d9254bc4212f332f89243bc2.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22160#discussion_r1851574153
PR Review Comment: https://git.openjdk.org/jdk/pull/22160#discussion_r1851573979
PR Review Comment: https://git.openjdk.org/jdk/pull/22160#discussion_r1851573877
