On Tue, 14 Oct 2025 07:31:45 GMT, Volkan Yazici <[email protected]> wrote:
>> Ensure `HttpClient::sendAsync` translates every operational failure to an >> `IOException` as per specification. >> >> `tier1-2` passes with the proposed changes. >> >> **Context:** The parent issue, [JDK-8364733], reports that >> `HttpClient::sendAsync` leaks exceptions which do not extend from >> `IOException`, and this violates the method's specification. The >> [JDK-8367067] (#26876) sub-task improved issues around exceptions thrown by >> request body publishers – which triggered the first encounter with this >> problem. This PR (and its associated sub-task) is aimed to end this saga. >> >> [JDK-8364733]: https://bugs.openjdk.org/browse/JDK-8364733 >> [JDK-8367067]: https://bugs.openjdk.org/browse/JDK-8367067 > > src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java > line 1147: > >> 1145: var translatedException = unwrappedException >> instanceof Error >> 1146: ? unwrappedException >> 1147: : Utils.toIOException(exception); > > `toIOException()`, which is _only_ used in > `Http3PushManager::cancelPendingPushPromise`, performs unwrapping of > `CompletionException` and `ExecutionException` too, though it doesn't exclude > `Error`. I considered following approaches: > > 1. Don't use `toIOException` and hard-code all the translation logic to here > 2. Update `toIOException()` to not wrap `Error` – This implies I need to > change method's return type from `IOException` to `Throwable`, and this > creates problems at the `cancelPendingPushPromise()` call site, since it uses > `Http3PushPromiseStream#cancel(IOException)` (note the required `IOException` > argument!), which overrides `ExchangeImpl#cancel(IOException)`. In short, > this is a dead end. > 3. Specialize on `Error` here, but still use `toIOException()` for the rest. > > I've implemented the 3rd option. But as time passes, I'm getting inclined > towards hard-coding all the translation logic (1st option) *and* move > `Utils::toIOException` to `Http3PushManager` as a `private` method. I'd > appreciate your thoughts on the matter. Keep it in Utils. `ExchangeImpl#cancel(IOException)` is protocol agnostic which lets me think that maybe there are other places that do wrapping - or may need to do wrapping in the future. 3rd option LGTM ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27787#discussion_r2429420173
