Code Review Request JDK-8170329 New SSLSocket testing template
Xuelei Fan
xuelei.fan at oracle.com
Tue Dec 6 03:53:14 UTC 2016
New webrev:
http://cr.openjdk.java.net/~xuelei/8170329/webrev.01/
On 12/5/2016 10:16 AM, Artem Smotrakov wrote:
>
> On 12/02/2016 09:48 PM, Xue-Lei Fan wrote:
>>
>>>>>>> - Exceptions are printed out in startServer/startClient methods, it
>>>>>>> doesn't look necessary to use suppressed exceptions and initCause()
>>>>>>> method. What was wrong with the code in runTest() method? The
>>>>>>> code in
>>>>>>> runTest() method looks shorter and cleaner to me.
>>>>>>>
>>>>>> The main issue of runTest() is that it does not throw the original
>>>>>> exception. Exception tacks have more information for debugging. I
>>>>>> want to keep the good side of Brad's previous hardness on this point.
>>>>> It is not clear what's the original exception should be because you
>>>>> have
>>>>> client and server. If you print all exceptions in
>>>>> startServer/startClient (right after they occurred), then you don't
>>>>> hide
>>>>> anything helpful for debugging.
>>>>>
>>>> The exception dump may have more information than the single exception
>>>> track, for example the nested cause stacks.
>>> Can't it be printed out in startServer/startClient accordingly?
>> Throw the right exception is important even for test case. Having the
>> Java handling the exception is more nature. Reading the debug log
>> dump is not as straightforward as read the exception at the end of
>> each run. Wrapping into RuntimeException is very confusing sometimes.
>>
>> Comparing the following two log:
>> java.net.XXException: this is an exception message.
>> cased by A
>> caused by B.
>> and
>> java.lang.RuntimeException: xxxx
>> caused by java.net.XXException: this is an exception message.
>> caused by A.
>> (cause by B may be swallowed)
>>
>> I don't like the later one, which introduce unnecessary exception
>> stacks and take more time for the debugging.
> I am not suggesting neither #2.
>
> I am suggesting not to use initCause because it modifies the original
> exception, and not to use addSupressed() because it's not clear where
> the suppressed exception came from. Instead, I am suggesting to print
> all original exceptions on that sides where they occurred. I am not
> suggesting to wrap anything into a runtime exception. A runtime
> exception can be used just to indicate a test failure.
I see your points. But most of the people would first read the
exception stacks at first. It's a natural behavior to check the failure
exception at first for developers and customers. Failure with the right
exception helps a lot for debugging. The use of supressed and caused
exception is also for this consideration.
Xuelei
More information about the security-dev
mailing list