RFR 8158023: SocketExceptions contain too little information sometimes

[Roger Riggs]

Hi Christoph,

Looking good, its unfortunate that the handling of mixed platform and
utf string require jni up calls to invoke java methods.

jni_util.c: line 216:

   You should not need to create an extra string; the string s is 
non-null and ready to use.

jni_util.h:

   line 117-119, The original comment was just as informative as the 
replacement.

The rest looks fine.

Roger

On 6/28/16 4:45 PM, Langer, Christoph wrote:
>
> Hi Paul,
>
> Ok, you kind of got me convinced and it is also a quite simple 
> modification. I changed from “java.net.SocketException: ioctl 
> SIOCGSIZIFCONF failed: Bad file number” to “java.net.SocketException: 
> Bad file number (ioctl SIOCGSIZIFCONF failed)” like you suggested.
>
> The update is in place: 
> http://cr.openjdk.java.net/~clanger/webrevs/8158023.3/ 
> <http://cr.openjdk.java.net/%7Eclanger/webrevs/8158023.3/>
>
> Now I finally need a review…
>
> Best regards
>
> Christoph
>
> *From:*Paul Benedict [mailto:pbenedict at apache.org]
> *Sent:* Montag, 27. Juni 2016 18:15
> *To:* Langer, Christoph <christoph.langer at sap.com>
> *Cc:* Kenji Kazumura <kzr at jp.fujitsu.com>; Chris Hegarty 
> <chris.hegarty at oracle.com>; nio-dev at openjdk.java.net; 
> core-libs-dev at openjdk.java.net; net-dev at openjdk.java.net
> *Subject:* Re: RFR 8158023: SocketExceptions contain too little 
> information sometimes
>
> Christoph, I didn't understand your explanation on your message 
> preference. Typically root cause information is printed last, not 
> first. Another reason not to change the ordering of the exception 
> message is that applications may be looking at existing strings. For 
> this example, if I may presume "Bad file number" is an existing 
> message, I would defer to the possibility applications may be exist 
> that test for that message condition.
>
>
> Cheers,
> Paul
>
> On Mon, Jun 27, 2016 at 2:42 AM, Langer, Christoph 
> <christoph.langer at sap.com <mailto:christoph.langer at sap.com>> wrote:
>
>     Hi,
>
>     eventually here is the - hopefully final - version of this fix:
>     http://cr.openjdk.java.net/~clanger/webrevs/8158023.3/
>     <http://cr.openjdk.java.net/%7Eclanger/webrevs/8158023.3/>
>
>     Now I leave JNU_ThrowByNameWithLastError untouched and I've also
>     added conversion to the new function
>     JNU_ThrowByNameWithMessageAndLastError. I've replaced
>     JNU_ThrowByNameWithLastError with
>     JNU_ThrowByNameWithMessageAndLastError in the java/net coding
>     where I think it is appropriate (mostly in occasions when a
>     SocketException is thrown kind of generically). @Paul: thanks for
>     your suggestion regarding the output format but I would still
>     prefer an output like "java.net.SocketException: ioctl
>     SIOCGSIZIFCONF failed: Bad file number" vs. "
>     java.net.SocketException: Bad file number (ioctl SIOCGSIZIFCONF
>     failed)" as I'm always handing down a message to the new throw
>     routine.
>
>     Hoping for a review :)
>
>     Best regards
>     Christoph
>
>     > -----Original Message-----
>     > From: Kenji Kazumura [mailto:kzr at jp.fujitsu.com]
>     > Sent: Mittwoch, 8. Juni 2016 02:51
>     > To: Langer, Christoph <christoph.langer at sap.com>
>     > Cc: net-dev at openjdk.java.net <mailto:net-dev at openjdk.java.net>;
>     nio-dev at openjdk.java.net <mailto:nio-dev at openjdk.java.net>; core-libs-
>     > dev at openjdk.java.net <mailto:dev at openjdk.java.net>
>     > Subject: Re: RFR 8158023: SocketExceptions contain too little
>     information
>     > sometimes
>     >
>     > Christoph,
>     >
>     > You should not remove conversion codes (platform string to Java
>     String)
>     > at JNU_ThrowByNameWithLastError,
>     > and you have to add conversion codes at
>     > JNU_ThrowByNameWithMessageAndLastError.
>     >
>     > It seems that you assume strerror returns only ascii characters,
>     but actually
>     > not.
>     > It depends on the locale of your environment where java programs
>     runs.
>     >
>     >
>     > -Kenji Kazumura
>     >
>     >
>     > In message
>     > <decc19cdab854bbeac7126cb8e236f1e at DEWDFE13DE11.global.corp.sap
>     <mailto:decc19cdab854bbeac7126cb8e236f1e at DEWDFE13DE11.global.corp.sap>>
>     >    RFR 8158023: SocketExceptions contain too little information
>     sometimes
>     >    "Langer, Christoph"
>     <<mailto:christoph.langer at sap.com>christoph.langer at sap.com> wrote:
>     >
>     > > Hi all,
>     > >
>     > > please review the following change:
>     > > Webrev:
>     <http://cr.openjdk.java.net/%7Eclanger/webrevs/8158023.1/>http://cr.openjdk.java.net/~clanger/webrevs/8158023.1/
>     > > Bug: https://bugs.openjdk.java.net/browse/JDK-8158023
>     > >
>     > > During error analysis I stumbled over a place where I
>     encountered a
>     > SocketException which was thrown along with some strerror
>     information as
>     > message. I found it hard to find the originating code spot with
>     that information.
>     > >
>     > > So I looked at the places where we throw exceptions, namely
>     JNU_Throw...
>     > and NET_Throw... functions and came up with the following
>     enhancement:
>     > > - NET_ThrowByNameWithLastError can go completely as it does
>     not provide
>     > any benefit over JNU_ThrowByNameWithLastError.
>     > > - JNU_ThrowByNameWithLastError can be cleaned up.
>     > >
>     > > - I added JNU_ThrowByNameWithMessageAndLastError to print out
>     a string
>     > like message + ": " + last error.
>     > >
>     > > - I went over all places where NET_ThrowByNameWithLastError is
>     used and
>     > replaced it appropriately.
>     > >
>     > > Do you think this change is desirable/possible?
>     > >
>     > > Though it's mainly a net topic, I'm posting it to nio-dev and
>     core-libs-dev as
>     > well as JNU_Throw... code affects all.
>     > >
>     > > Best regards
>     > > Christoph
>     > >
>