search

jglobus / JGlobus

jGlobus is a collection of Java client libraries for Globus® Toolkit security, GRAM, and GridFTP.
http://www.globus.org/toolkit/jglobus/
Apache License 2.0
24 stars 44 forks source link

Update GlobusGSSException to avoid boring messages. #81

Closed paulmillar closed 11 years ago

paulmillar commented 11 years ago

Quite often, a GlobusGSSException is used to wrap an existing Exception where is adds no additional meaning. Often this is due to catching Exception but the signature of the method allows only GSSException to be thrown, which is a bad code-smell.

The result of this problem is Exception messages with many chained "Failure unspecified at GSS-API level" messages. Here is an example:

24 Apr 2013 07:31:17 (SRM-vedrfolnir) [] org.globus.common.ChainedIOException: Authentication failed [Caused by: Failure unspecified at GSS-API level [Caused by: Failure unspecified at GSS-API level [Caused by: sun.security.pkcs11.wrapper.PKCS11Exception: CKR_DOMAIN_PARAMS_INVALID]]]

As a work-around, this patch skips over these semantically-null messages: if the message would be "Failure unspecified at GSS-API level" and an exception has been logged as the cause (printed as "Caused by...") then only the message from the cause is printed.

With this change, the above exception becomes:

pr 2013 07:37:21 (SRM-vedrfolnir) [] org.globus.common.ChainedIOException: Authentication failed [Caused by: sun.security.pkcs11.wrapper.PKCS11Exception: CKR_DOMAIN_PARAMS_INVALID]

I consider this patch a quick-fix work-around as the real solution would be to modify code so it refrains from wrapping exceptions in situations where it isn't needed and no additional context is provided. This will probably be part of a complete rewrite of GlobusGSSException (perhaps dropping the class altogether) and how it is being used.

Please also merge this to the v2.0 branch for the next 2.0.x release.

paulmillar commented 11 years ago

I've updated the patch to cover some other cases.

The SSLEngine class can throw an SSLException that has no (meaningful) content in the message, but has an exception that triggered the SSLException with useful information.

Patch now covers this case, too.

bbockelm commented 11 years ago

Hi Paul,

To help with the merge, can you re-submit the pull against the 2.0.x branch? I'll then pull it through master by hand.

Brian

paulmillar commented 11 years ago

Sure

paulmillar commented 11 years ago

Adding a new branch fix_GlobusGSSException_v2_0 targeted against the v2.0 branch. I'll submit a pull-request for this now.

bbockelm commented 11 years ago

I'm closing this; I took the v2.0 version and cherry-picked the patch forward.