On Wed, 30 Jul 2025 10:05:26 GMT, Jaikiran Pai <[email protected]> wrote:
>> The `IOE` thrown is wrapped by an `UncheckedIOE` in
>> `FileChannelIterator::next`, which overrides `Iterator::next` and that does
>> not allow a `throws` in the `next()` footprint. Would you mind elaborating
>> on your remark, please?
>
> Hello Volkan, the specification of `HttpClient.send(...)` (and sendAsync())
> is that it throws a checked `IOException`. So any other exceptions that we
> throw internally (like this one) need to be converted to an `IOException`
> when it reaches the application code.
>
> We have code in `HttpClientImpl.send(...)` which currently does instanceof
> checks against these exceptions and converts them to an `IOException`. I'm
> guessing your test is currently passing, which suggests to me that
> `sendAsync()` is propagating a `UncheckedIOException` to the application
> code. Would it be possible to tweak the test a bit to replace that call of
> `sendAsync()` with a `send()` (even if those tweaked changes cannot be pushed
> to this PR) and see what gets propagated? I suspect it would be `IOException`.
I've confirmed that updating the test to use `send()` results in `IOE` getting
received. I have not updated the PR in this direction, since it adds more
boilerplate, and I read your lines as, not a change, but a research request –
let me know if you meant otherwise.
> the specification of `HttpClient.send(...)` (and `sendAsync()`) is that it
> throws a checked `IOException`
`HttpClient::send` has the following Javadoc:
* @throws IOException if an I/O error occurs when sending or receiving, or
* the client has {@linkplain ##closing shut down}
* @throws InterruptedException ...
* @throws IllegalArgumentException ...
whereas `::sendAsync` Javadoc has no details regarding I/O failures. Do you
think the fact that `sendAsync()` does *not* have a
to-`IOException`-translation is an oversight that should be improved, or an
intentional slack for the implementation?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26155#discussion_r2243460718
