JDK 8 code review request for 6380161 (reflect) Exception from newInstance() not chained to cause.

[Joe Darcy]

Hello.

On 8/8/2011 7:24 AM, Sebastian Sickelmann wrote:
> Looks good to me.
>
> But i have one question:
>
>     Why there are two ways to plumb the causes of an exception?

The history of this situation is covered in older versions of the 
javadoc of Throwable:
http://download.oracle.com/javase/6/docs/api/java/lang/Throwable.html

>     In many exceptions-classes you can use a constuctor-level argument 
> to specify the cause, and in some classes you must use the initCause 
> method.
>
>     Is it: "When you often need a cause at creation-time than we got a 
> specializes constructor." ?
>
>     I found 500+ lines jdk/src/share/classes/**/*.java where i found 
> line like
>
>            XYZException iae = new XYZException();
>             iae.initCause(e);
>             throw iae;
>
>     I am searching a minor project for my second Contribution to 
> OpenJDK to learn more about the developement / review process.
>     Is it worthwhile to look at this issue and maybe refactor some of 
> the often used Exceptions to accept a cause at construction-time?

I would say such a project would be helpful; for context, this topic 
came up a few months ago on core-libs in the context of another code review:

     
http://mail.openjdk.java.net/pipermail/core-libs-dev/2011-June/007005.html
     
http://mail.openjdk.java.net/pipermail/core-libs-dev/2011-June/007008.html

Cheers,

-Joe

>
> - Sebastian
>
> Am 08.08.2011 07:24, schrieb Joe Darcy:
>> Hello.
>>
>> Please review this small fix, developed after I went through my open 
>> bug list; patch below, webrev at:
>>
>>    6380161 (reflect) Exception from newInstance() not chained to cause.
>>    http://cr.openjdk.java.net/~darcy/6380161.0/
>>
>> Several call to initCause are added to plumb up caused-by exception 
>> chains to exception types without constructors taking a cause.  In 
>> addition, multi-catch is added in on location.
>>
>> I looked for similar issues in Constructor, Method, and Executable, 
>> but there weren't any.
>>
>> Thanks,
>>
>> -Joe
>>
>> --- old/src/share/classes/java/lang/Class.java    2011-08-07 
>> 22:21:58.000000000 -0700
>> +++ new/src/share/classes/java/lang/Class.java    2011-08-07 
>> 22:21:58.000000000 -0700
>> @@ -349,7 +349,8 @@
>>                         });
>>                 cachedConstructor = c;
>>             } catch (NoSuchMethodException e) {
>> -                throw new InstantiationException(getName());
>> +                throw (InstantiationException)
>> +                    new InstantiationException(getName()).initCause(e);
>>             }
>>         }
>>         Constructor<T> tmpConstructor = cachedConstructor;
>> @@ -973,7 +974,8 @@
>>                 descriptor      = (String)   enclosingInfo[2];
>>                 assert((name != null && descriptor != null) || name 
>> == descriptor);
>>             } catch (ClassCastException cce) {
>> -                throw new InternalError("Invalid type in enclosing 
>> method information");
>> +                throw (InternalError)
>> +                    new InternalError("Invalid type in enclosing 
>> method information").initCause(cce);
>>             }
>>         }
>>
>> @@ -1239,7 +1241,8 @@
>>         try {
>>             return 
>> getName().substring(enclosingClass.getName().length());
>>         } catch (IndexOutOfBoundsException ex) {
>> -            throw new InternalError("Malformed class name");
>> +            throw (InternalError)
>> +                new InternalError("Malformed class 
>> name").initCause(ex);
>>         }
>>     }
>>
>> @@ -2954,9 +2957,8 @@
>>             }
>>             // These can happen when users concoct enum-like classes
>>             // that don't comply with the enum spec.
>> -            catch (InvocationTargetException ex) { return null; }
>> -            catch (NoSuchMethodException ex) { return null; }
>> -            catch (IllegalAccessException ex) { return null; }
>> +            catch (InvocationTargetException | NoSuchMethodException |
>> +                   IllegalAccessException ex) { return null; }
>>         }
>>         return enumConstants;
>>     }
>>
>