RFR: 8314978: Multiple server call from connection failing with expect100 in getOutputStream [v2]

[Daniel Fuchs]

On Mon, 4 Sep 2023 08:25:43 GMT, Vyom Tewari <vtewari at openjdk.org> 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