David, Lance, and Rémi, Thanks for the prompt reviews, -Joe Rémi Forax wrote:

Agree, looks good.

Rémi

On 08/08/2011 03:41 PM, Lance Andersen - Oracle wrote:

...

Looks fine to me Joe

Best Lance On Aug 8, 2011, at 1:24 AM, Joe Darcy wrote:

...

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; }

Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 Lance.Andersen@oracle.com

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; }

I have picked my first Exception in this project. And unfortunatly i picked InternalError which seems to be used quite often. :-) I tried to change all initCause calls to the new Constructor and found many places where there is code like this: throw new InternalError(e.toString()); or throw new InternalError(e.getMessage()); or throw new InternalError("Lorem ipsum:"+e); These ones are candidates for cleanup too, but i want to discuss if someone sees a good argument to not to change to throw new InternalError(e.getMessage(),e); or throw new InternalError("Lorem ipsum",e); Also i don't completely understand why Throwable(Throwable cause) uses cause.toString() to fill the message instead of cause.getMessage(). Can someone explain to me why? Couldn't it be if (cause == null) { detailMessage = null; }else { String message = cause.getMessage(); detailMessage = (message==null ? null : cause.toString()); } instead? - Sebastian Am 08.08.2011 21:18, schrieb Sebastian Sickelmann:

Thanks for encouraging me.

I opened a bug [1] at bugs.openjdk.java.net. Hope that is a right first step to getting started. I wanted to investigate the calls to initCause in jdk-source first to pick the most valueable Exceptions first. I thinks there are many exceptions out of scope of core-libs. How to handle these ones. I think i will start with exceptions from core-libs. Is there a pattern that says me exceptions belongs to which openjdk-project?

- Sebastian

[1] https://bugs.openjdk.java.net/show_bug.cgi?id=100201

Am 08.08.2011 19:04, schrieb 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; }