On Mon, 4 Apr 2022 09:52:56 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> src/java.net.http/share/classes/jdk/internal/net/http/HttpClientBuilderImpl.java >> line 140: >> >>> 138: return this; >>> 139: } >>> 140: >> >> Either this should check that localAddr is an `InetSocketAddress` and that >> port = 0, and throw an UnsupportedOperationException in that case, or the >> build() method should specify what exception is thrown and when, if the >> local address passed to the HttpClient implementation is not supported - >> because some exception will be thrown at some point... IMO it would be >> better to do the check early - although we would probably redo it in the >> HttpClientImpl (but there it wouldn't fail unless something bad happened). > > I've now updated the PR to change this implementation to throw an > `UnsupportedOperationException` if the passed address isn't an > `InetSocketAddress` or if the port is not 0. This method will (continue to > allow) setting(/resetting to) a `null` address. > > In the constructor of `HttpClientImpl`, I have then used an `assert` where it > verifies the address to be an InetSocketAddress with non-zero port. Of > course, this will only trigger when assertions are enabled. I did not add a > runtime check here since, like you say, I couldn't think of a way where after > our checks in the builder's `localAddress` implementation, we would end up > with the condition failing here. If we do add the runtime check here, we have > to also decide what type of an exception we would throw from here. The > current API doc of `HttpClient.Builder.build()` states that it will (only) > throw an `UncheckedIOException`. Is that the one we would want to throw (with > a `null` cause) if we do want to do the runtime check in the constructor? Just adding a new comment to trigger skara bot emails. ------------- PR: https://git.openjdk.java.net/jdk/pull/6690
