RFR: JDK-8299475: Enhance SocketException by cause where it is missing in net and nio area [v4]

[Alan Bateman]

On Tue, 3 Jan 2023 13:57:20 GMT, Matthias Baesken <mbaesken at openjdk.org> wrote:

>> We have a couple of places where a SocketException is thrown but the cause is omitted. It would be beneficial for example in error analysis not to throw away the cause (causing exception) but to add it to the created SocketException.
>
> Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Adjust Socket createImpl again

The changes in [8af8a34d] to Socket, ServerSocket and SocksSocketImpl looks okay. I think the changes to NioSocketImpl will lead to confusing output as the cause will be almost identify to the SocketException. I assume your goal with the change is to get the deepest implementation frames into the stack trace. They would be there if read/write threw IOException but we decided in JEP 353 to throw the more specific SocketException to maintain historical behavior. So maybe we could try this instead - this will throw the SocketException with the stack trace from the IOException so it will include the deepest frames:


diff --git a/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java b/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java
index b2531135b03..f96d8380f29 100644
--- a/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java
+++ b/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java
@@ -312,7 +312,8 @@ public final class NioSocketImpl extends SocketImpl implements PlatformSocketImp
             connectionReset = true;
             throw new SocketException("Connection reset");
         } catch (IOException ioe) {
-            throw new SocketException(ioe.getMessage());
+            // throw SocketException to maintain compatibility
+            throw asSocketException(ioe);
         } finally {
             endRead(n > 0);
         }
@@ -410,7 +411,8 @@ public final class NioSocketImpl extends SocketImpl implements PlatformSocketImp
         } catch (InterruptedIOException e) {
             throw e;
         } catch (IOException ioe) {
-            throw new SocketException(ioe.getMessage());
+            // throw SocketException to maintain compatibility
+            throw asSocketException(ioe);
         } finally {
             endWrite(n > 0);
         }
@@ -1081,10 +1083,8 @@ public final class NioSocketImpl extends SocketImpl implements PlatformSocketImp
                 default:
                     throw new SocketException("Unknown option " + opt);
                 }
-            } catch (SocketException e) {
-                throw e;
             } catch (IllegalArgumentException | IOException e) {
-                throw new SocketException(e.getMessage());
+                throw asSocketException(e);
             }
         }
     }
@@ -1133,10 +1133,8 @@ public final class NioSocketImpl extends SocketImpl implements PlatformSocketImp
                 default:
                     throw new SocketException("Unknown option " + opt);
                 }
-            } catch (SocketException e) {
-                throw e;
             } catch (IllegalArgumentException | IOException e) {
-                throw new SocketException(e.getMessage());
+                throw asSocketException(e);
             }
         }
     }
@@ -1249,6 +1247,19 @@ public final class NioSocketImpl extends SocketImpl implements PlatformSocketImp
         return remainingNanos;
     }
 
+    /**
+     * Creates a SocketException from the given exception.
+     */
+    private static SocketException asSocketException(Exception e) {
+        if (e instanceof SocketException se) {
+            return se;
+        } else {
+            var se = new SocketException(e.getMessage());
+            se.setStackTrace(e.getStackTrace());
+            return se;
+        }
+    }
+
     /**
      * Returns the socket protocol family.
      */

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

PR: https://git.openjdk.org/jdk/pull/11813