On Mon, 4 Sep 2023 08:25:43 GMT, Vyom Tewari <[email protected]> wrote:
>> 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);
>>
>> Hello 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?
>
> I don't know the history but it is very old code and just to be on safer side
> i did the same changes as per getInputStream0(). Looking from out side
> wrapping the 'RuntimeException' into another 'RuntimeException' not make
> much sense.
I suspect the wrapping is here to make sure the caller of getInputStream()
appears in the call stack.
If you have code that does this:
try {
conn.getInputStream(); <<< rememberedException set here (1)
} catch (Exception e) { }
...
// retry
conn.getInputStream(); >>> rememberedException thrown here (2)
then the exception will be thrown at (2) but the stack trace will show it
thrown at (1), which will be confusing when debugging. If you wrap it you will
see both (1) and (2) in the stack trace. Specifically, you will see an
exception thrown at (2) that wraps an exception thrown at (1).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15483#discussion_r1315115188
