Request for review: 7084245: Update usages of InternalError to use exception chaining

[Xuelei Fan]

I reviewed the security part. In general, it looks fine. But sometimes,
from my very personal view, the exception-chain might be over used. For
example, the following code:

  try {
      md = MessageDigest.getInstance("SHA");
  } catch (NoSuchAlgorithmException nsae) {
      throw new InternalError("internal error: SHA-1 not available.");
  }

The exception message and stack is simple and easy to understand. If
using exception-chain, the message chain looks like:
   "internal error: SHA-1 not available."
   -> "SHA ... not available""

I think the information is redundant, and expose unnecessary call stack.

Exceptions thrown by a method should be defined at an abstraction level
consistent with what the method does, not necessarily with the low-level
details of how it is implemented. Please also refer to item 61, "Throw
exceptions appropriate to the abstraction", in "Effective Java 2nd Editor".

It seems that the webrev offline now, I cannot access it.

Thanks,
Xuelei

On 8/29/2011 3:35 AM, Sebastian Sickelmann wrote:
> Hi, here is a webrev[1] for some cleanup that i want to integrated in
> tl-repositories.
> 
> Alan Bateman had scanned the changes and gave me some good input[3] for
> further discussion here:
> 
> The changes to java.util.concurrent should go through Doug Lea's
> upstream CVS. Alan told me that Chris Hegarty is on this topic already.
> The suggested changes for this is here[2].
> 
> I have changed some classes in awt / sun.java2d maybe someone of the
> 2d-dev maillinglist can look at these changes.
> I also changed some classes in java/securtiy maybe someone of
> security-dev maillinglist can look at these changes.
> 
> Let me know if there is a need to split/rebase the main-webrev[1] to
> review and/or push it individually.
> 
> Mostly the patch changes exception-chains. But there are some places
> where the patch changes behavoir:
> 
> - I removed some printstackTraces in
> sun.java2d.pipe.LoopPipe.getStrokesSpans and sun.misc.Launcher (Alan
> told me that kumar maybe want to have a look at it?).
> - I changed java.text.Format.clone not to return null. I think it will
> never happen. But if so throwing an InternalError seems to be better
> than returning null and let all the extended classes crash in there
> clone Method with a NullPointerException. And so catching an Exception
> in java.text.DecimalFormat.clone is unnecessary.
> 
> -- Sebastian
> 
> [1] http://oss-patches.24.eu/openjdk8/InternalError/part2/7084245_main_0/
> [2]
> http://oss-patches.24.eu/openjdk8/InternalError/part2/7084245_concurrent_0/
> [3]
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2011-August/007563.html
>