On Sun, 3 Sep 2023 09:16:24 GMT, Vyom Tewari <[email protected]> wrote:
>> With the current implementation of HttpURLConnection if server rejects the
>> “Expect 100-continue” then there will be ‘java.net.ProtocolException’ will
>> be thrown from 'expect100Continue()' method.
>>
>> After the exception thrown, If we call any other method on the same instance
>> (ex getHeaderField(), or getHeaderFields()). They will internally call
>> getOuputStream() which invokes writeRequests(), which make the actual server
>> call.
>>
>> The code change will sets the existing variable ‘rememberedException’ when
>> there is exception and getOutputStream0() will re-throw
>> ‘rememberedException’ if the ‘rememberedException’ is not null.
>>
>> Note: getOutputStream0() also call’s ‘expect100Continue()’ if
>> ‘expectContinue’ is true.
>
> Vyom Tewari has updated the pull request incrementally with one additional
> commit since the last revision:
>
> modified the junit tests names
src/java.base/share/classes/sun/net/www/protocol/http/HttpURLConnection.java
line 1436:
> 1434: if (rememberedException != null) {
> 1435: if (rememberedException instanceof RuntimeException) {
> 1436: throw new RuntimeException(rememberedException);
Hell Vyom, this looks a bit odd that if it's a `RuntimeException` then we are
wrapping that `RuntimeException` into another `RuntimeException` before
throwing. Having said that, it appears that this same thing is done in
`getInputStream0()` method of this class. Do you know if that is intentional
and needed? If it's not needed, perhaps we can just rethrow this
`RuntimeException` without wrapping it into another?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15483#discussion_r1314534857
