On Tue, 3 Jan 2023 13:57:20 GMT, Matthias Baesken <mbaes...@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
