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

[Alan Bateman]

On Tue, 3 Jan 2023 10:47:11 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:
> 
>   re-throw SocketException, remove one enhancement because it is meant to be internal

src/java.base/share/classes/java/net/Socket.java line 537:

> 535:             throw e;
> 536:         } catch (IOException e) {
> 537:             throw new SocketException(e.getMessage());

I can't immediately think of real cases where a non-SocketException is thrown when creating a SocketImpl but for consistency with the equivalent in ServerSocket then you could add the cause here too.

src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java line 413:

> 411:             throw e;
> 412:         } catch (IOException ioe) {
> 413:             throw new SocketException(ioe.getMessage(), ioe);

I'm in two minds on whether this is a good idea or not because the stack trace of the cause is mostly the same as the SocketException, e.g. right now we get a clear stack trace like this:

Exception in thread "main" java.net.SocketException: Broken pipe
	at java.base/sun.nio.ch.NioSocketImpl.implWrite(NioSocketImpl.java:413)
	at java.base/sun.nio.ch.NioSocketImpl.write(NioSocketImpl.java:433)
	at java.base/sun.nio.ch.NioSocketImpl$2.write(NioSocketImpl.java:812)
	at java.base/java.net.Socket$SocketOutputStream.write(Socket.java:1192)
	at java.base/java.io.OutputStream.write(OutputStream.java:124)


but with the change we get a cause with almost the same stack trace:

Exception in thread "main" java.net.SocketException: Broken pipe
	at java.base/sun.nio.ch.NioSocketImpl.implWrite(NioSocketImpl.java:413)
	at java.base/sun.nio.ch.NioSocketImpl.write(NioSocketImpl.java:433)
	at java.base/sun.nio.ch.NioSocketImpl$2.write(NioSocketImpl.java:812)
	at java.base/java.net.Socket$SocketOutputStream.write(Socket.java:1192)
	at java.base/java.io.OutputStream.write(OutputStream.java:124)
Caused by: java.io.IOException: Broken pipe
	at java.base/sun.nio.ch.SocketDispatcher.write0(Native Method)
	at java.base/sun.nio.ch.SocketDispatcher.write(SocketDispatcher.java:62)
	at java.base/sun.nio.ch.NioSocketImpl.tryWrite(NioSocketImpl.java:388)
	at java.base/sun.nio.ch.NioSocketImpl.implWrite(NioSocketImpl.java:404)
	at java.base/sun.nio.ch.NioSocketImpl.write(NioSocketImpl.java:433)
	at java.base/sun.nio.ch.NioSocketImpl$2.write(NioSocketImpl.java:812)
	at java.base/java.net.Socket$SocketOutputStream.write(Socket.java:1192)
	at java.base/java.io.OutputStream.write(OutputStream.java:124)


The reason for the translation from IOException to SocketException is so that the SocketImpl added via JEP 353 throws the same specific exceptions as the legacy implementation. We were concerned there may be older code assuming that read/write throw SocketException. So one idea here is to avoid the confusing cause is to use the stack trace from the IOException, as in:

var e = new SocketException(ioe.getMessage());
e.setStackTrace(ioe.getStackTrace());
throw e;

which gives us a clear stack trace like we have now:

Exception in thread "main" java.net.SocketException: Broken pipe
	at java.base/sun.nio.ch.SocketDispatcher.write0(Native Method)
	at java.base/sun.nio.ch.SocketDispatcher.write(SocketDispatcher.java:62)
	at java.base/sun.nio.ch.NioSocketImpl.tryWrite(NioSocketImpl.java:388)
	at java.base/sun.nio.ch.NioSocketImpl.implWrite(NioSocketImpl.java:404)
	at java.base/sun.nio.ch.NioSocketImpl.write(NioSocketImpl.java:435)
	at java.base/sun.nio.ch.NioSocketImpl$2.write(NioSocketImpl.java:814)
	at java.base/java.net.Socket$SocketOutputStream.write(Socket.java:1192)
	at java.base/java.io.OutputStream.write(OutputStream.java:124)

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

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