hg: code-tools/jtharness: 7902304: Fix 'throws' clause with subclassing exceptions

[Jonathan Gibbons]

OK, sounds like you've thought it through, and it is more than just a 
false positive found by the IDE.

My primary general concern is to try and give better, more meaningful 
messages to to the user, and generally "file not found" is more 
informative than "Some IO problem happened."  That's all.

It may not apply here, but I also note that (as a general rule) it is 
good not to directly pass exceptions up too may levels, since you may 
lose context when doing so ... what may be meaningful low down may be 
less so at higher levels.

-- Jon


On 9/5/18 9:49 AM, dmitry.bessonov at oracle.com wrote:
> As I can see that the only case when FileNotFoundException could be 
> thrown is creating a FileInputStream from "workdir/jtData" file and 
> the following happens:
> " - if the file does not exist, is a directory rather than a regular 
> file, or for some other reason cannot be opened for reading"
> but later in the lines which were removed the original exception is 
> dropped and instead an unclear and very generic message is provided:
>
> wd.noTestSuiteFile=Cannot determine the test suite for the work 
> directory {0}.
>
> While after the changes below:
> 1. exception instance is passed to the BadDirectoryFault wrapper
> 2. the message is
> "wd.badTestSuiteFile=Problem reading test suite data for work 
> directory {0}.\n{1}"
>
> which generally looks like an improvement.
>
> I can revert to the legacy state... at least providing 
> FileNotFoundException instance to the wrapper to reduce the confusion.
>
>
> dmitry
>
>
> On 05/09/2018 16:15, Jonathan Gibbons wrote:
>> This one I disagree with. I would recommend reverting it.
>>
>> Yes, FileNotFoundException is a subtype of IOException, but it
>> is reasonable to handle it differently, as shown in the lines you
>> have deleted:
>>
>> - } // try
>> - catch (FileNotFoundException e) {
>> - throw new BadDirectoryFault(i18n, "wd.noTestSuiteFile", canonDir);
>> } catch (IOException e) {
>> throw new BadDirectoryFault(i18n, "wd.badTestSuiteFile", canonDir, e);
>> } catch (TestSuite.Fault e) {
>>
>>
>> Notice the error message is different.
>>
>> -- Jon
>>
>> On 9/5/18 5:51 AM, dmitry.bessonov at oracle.com wrote:
>>> Changeset: 1aca482dba53
>>> Author:    dbessono
>>> Date:      2018-09-05 13:43 +0100
>>> URL:http://hg.openjdk.java.net/code-tools/jtharness/rev/1aca482dba53
>>>
>>> 7902304: Fix 'throws' clause with subclassing exceptions
>>>
>>> ! src/com/sun/javatest/WorkDirectory.java
>>>
>>