RFR: 8369595: HttpClient: HttpHeaders.firstValueAsLong failures should be converted to ProtocolException [v5]

[Volkan Yazici]

On Wed, 10 Dec 2025 19:54:58 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> Doh! Very good catch. I've fixed this in 024e355f034. I've copied the `errorHandler` we pass to `ResponseSubscribers::getBodyAsync`. Note that I've slightly changed the `errorHandler` to have a `finally` block:
>> 
>> 
>> try {
>>     subscriber.onError(error);
>>     cf.completeExceptionally(error);
>> } finally {
>>     asyncReceiver.setRetryOnError(false);
>>     asyncReceiver.onReadError(error);
>> }
>> 
>> 
>> This is what the `executor.execute(() -> ...)` block does too, which I presume to guard against misbehaving `subscriber::onError` and `cf::completeExceptionally`.
>> 
>> @dfuch, I'm not resolving this conversation yet. Would you mind reviewing 024e355f034 and letting me know if it is okay, please?
>
> Looks OK - out of curiosity, is the try-finally simple hygienic or did you observe a case where it was required?

@djelinski also DM'ed me about the same questions yesterday. I unfortunately don't have a very satisfactory answer with complete certainty. In short, I've copied the existing convention in the area that I touched.

1. This convention – having `subscriber.onError(error)` and `cf.completeExceptionally(error)` in `try`, and `asyncReceiver.setRetryOnError(false)` and `asyncReceiver.onReadError(error)` in `finally` – was already present in the code before my changes.
2. Even though the handler passed to `ResponseSubscribers.getBodyAsync` did not have a `try`/`finally`, I did not opt for no-`try`/`finally` in `errorNotifier`, because AFAICT, `ResponseSubscribers.getBodyAsync` appears like an oversight, and all earlier lines always had `asyncReceiver.onReadError(error)` and `subscriber.onError(error)` in `finally`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28431#discussion_r2610351303