geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rick McGuire <rick...@gmail.com>
Subject Re: svn commit: r547679 - in /geronimo/server/trunk/modules: geronimo-axis-builder/src/main/java/org/apache/geronimo/axis/builder/ geronimo-axis2/src/main/java/org/apache/geronimo/axis2/pojo/ geronimo-cli/src/main/java/org/apache/geronimo/cli/deployer/ ger...
Date Mon, 18 Jun 2007 09:15:10 GMT
Yeah, some of the ones I changed were pretty much came down to a coin 
toss as to whether they should be changed or not.  I mostly used a 
judgement call on whether additional stack track information would be at 
all useful for diagnosing a problem if the exception did occur.  I do 
know that I was constantly running into problems trying to diagnose bugs 
where I needed as many as 3-4 runs to backtrack through all of the 
levels of swallowed exceptions to get any information about the root 
cause of the problem, so I leaned a litle more toward making the change 
than not. 

Rick

David Jencks wrote:
> I think there's a balance between showing everything possible and 
> showing what's useful.  After a quick scan, I notice that for most of 
> these I wrote the original form of the exception to try to clearly 
> indicate the exact problem, without showing lots of stack trace that 
> won't really help anyone.  For instance, with a malformed URL 
> exception, I think its important to know what the string that isn't a 
> URL is, and where it came from in great detail.  I don't see how 
> showing the stack trace from "new URL("this isn't a url")" is going to 
> add any useful info, although this kind of detail often covers lots of 
> pages of logging.
>
> What do others think?
>
> thanks
> david jencks
>
> On Jun 15, 2007, at 6:34 AM, rickmcguire@apache.org wrote:
>
>> Author: rickmcguire
>> Date: Fri Jun 15 06:34:32 2007
>> New Revision: 547679
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=547679
>> Log:
>> GERONIMO-3246 Cleanup exception handling so stack traces for first 
>> failures are not discarded.
>>
>>
>> Modified:
>>     
>> geronimo/server/trunk/modules/geronimo-axis-builder/src/main/java/org/apache/geronimo/axis/builder/AxisBuilder.java

>>
>>     
>> geronimo/server/trunk/modules/geronimo-axis-builder/src/main/java/org/apache/geronimo/axis/builder/HeavyweightTypeInfoBuilder.java

>>
>>     
>> geronimo/server/trunk/modules/geronimo-axis2/src/main/java/org/apache/geronimo/axis2/pojo/POJOWebServiceContainerFactoryGBean.java

>>
>>     
>> geronimo/server/trunk/modules/geronimo-cli/src/main/java/org/apache/geronimo/cli/deployer/DeployerCLParser.java

>>
>>     
>> geronimo/server/trunk/modules/geronimo-client-builder/src/main/java/org/apache/geronimo/client/builder/AppClientModuleBuilder.java

>>
>>     
>> geronimo/server/trunk/modules/geronimo-clustering-wadi/src/main/java/org/apache/geronimo/clustering/wadi/BasicWADISessionManager.java

>>
>>     
>> geronimo/server/trunk/modules/geronimo-clustering-wadi/src/main/java/org/apache/geronimo/clustering/wadi/WADISessionAdaptor.java

>>
>>
>> Modified: 
>> geronimo/server/trunk/modules/geronimo-axis-builder/src/main/java/org/apache/geronimo/axis/builder/AxisBuilder.java

>>
>> URL: 
>> http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-axis-builder/src/main/java/org/apache/geronimo/axis/builder/AxisBuilder.java?view=diff&rev=547679&r1=547678&r2=547679

>>
>> ============================================================================== 
>>
>> --- 
>> geronimo/server/trunk/modules/geronimo-axis-builder/src/main/java/org/apache/geronimo/axis/builder/AxisBuilder.java

>> (original)
>> +++ 
>> geronimo/server/trunk/modules/geronimo-axis-builder/src/main/java/org/apache/geronimo/axis/builder/AxisBuilder.java

>> Fri Jun 15 06:34:32 2007
>> @@ -323,7 +323,7 @@
>>          try {
>>              location = new URL(locationURIString);
>>          } catch (MalformedURLException e) {
>> -            throw new DeploymentException("Could not construct web 
>> service location URL from " + locationURIString);
>> +            throw new DeploymentException("Could not construct web 
>> service location URL from " + locationURIString, e);
>>          }
>>          return location;
>>      }
>> @@ -369,7 +369,7 @@
>>          try {
>>              location = new URL(locationURIString);
>>          } catch (MalformedURLException e) {
>> -            throw new DeploymentException("Could not construct web 
>> service location URL from " + locationURIString);
>> +            throw new DeploymentException("Could not construct web 
>> service location URL from " + locationURIString, e);
>>          }
>>          return location;
>>      }
>>
>> Modified: 
>> geronimo/server/trunk/modules/geronimo-axis-builder/src/main/java/org/apache/geronimo/axis/builder/HeavyweightTypeInfoBuilder.java

>>
>> URL: 
>> http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-axis-builder/src/main/java/org/apache/geronimo/axis/builder/HeavyweightTypeInfoBuilder.java?view=diff&rev=547679&r1=547678&r2=547679

>>
>> ============================================================================== 
>>
>> --- 
>> geronimo/server/trunk/modules/geronimo-axis-builder/src/main/java/org/apache/geronimo/axis/builder/HeavyweightTypeInfoBuilder.java

>> (original)
>> +++ 
>> geronimo/server/trunk/modules/geronimo-axis-builder/src/main/java/org/apache/geronimo/axis/builder/HeavyweightTypeInfoBuilder.java

>> Fri Jun 15 06:34:32 2007
>> @@ -428,7 +428,7 @@
>>                          Field field = javaClass.getField(fieldName);
>>                          javaType = field.getType();
>>                      } catch (NoSuchFieldException e) {
>> -                        throw new DeploymentException("field name " 
>> + fieldName + " not found in " + properties);
>> +                        throw new DeploymentException("field name " 
>> + fieldName + " not found in " + properties, e);
>>                      }
>>                  }
>>                  QName xmlName = new QName("", 
>> variableMapping.getXmlElementName().getStringValue().trim());
>>
>> Modified: 
>> geronimo/server/trunk/modules/geronimo-axis2/src/main/java/org/apache/geronimo/axis2/pojo/POJOWebServiceContainerFactoryGBean.java

>>
>> URL: 
>> http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-axis2/src/main/java/org/apache/geronimo/axis2/pojo/POJOWebServiceContainerFactoryGBean.java?view=diff&rev=547679&r1=547678&r2=547679

>>
>> ============================================================================== 
>>
>> --- 
>> geronimo/server/trunk/modules/geronimo-axis2/src/main/java/org/apache/geronimo/axis2/pojo/POJOWebServiceContainerFactoryGBean.java

>> (original)
>> +++ 
>> geronimo/server/trunk/modules/geronimo-axis2/src/main/java/org/apache/geronimo/axis2/pojo/POJOWebServiceContainerFactoryGBean.java

>> Fri Jun 15 06:34:32 2007
>> @@ -80,7 +80,7 @@
>>          try {
>>              container.init();
>>          } catch (Exception e) {
>> -            throw new RuntimeException(e);
>> +            throw new RuntimeException("Failure initializing web 
>> service containter", e);
>>          }
>>          return container;
>>      }
>>
>> Modified: 
>> geronimo/server/trunk/modules/geronimo-cli/src/main/java/org/apache/geronimo/cli/deployer/DeployerCLParser.java

>>
>> URL: 
>> http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-cli/src/main/java/org/apache/geronimo/cli/deployer/DeployerCLParser.java?view=diff&rev=547679&r1=547678&r2=547679

>>
>> ============================================================================== 
>>
>> --- 
>> geronimo/server/trunk/modules/geronimo-cli/src/main/java/org/apache/geronimo/cli/deployer/DeployerCLParser.java

>> (original)
>> +++ 
>> geronimo/server/trunk/modules/geronimo-cli/src/main/java/org/apache/geronimo/cli/deployer/DeployerCLParser.java

>> Fri Jun 15 06:34:32 2007
>> @@ -252,7 +252,7 @@
>>          try {
>>              getPort();
>>          } catch (NumberFormatException e) {
>> -            throw new CLParserException("Port [" + 
>> commandLine.getOptionValue(ARGUMENT_PORT_SHORTFORM) + "] is not an 
>> integer.");
>> +            throw new CLParserException("Port [" + 
>> commandLine.getOptionValue(ARGUMENT_PORT_SHORTFORM) + "] is not an 
>> integer.", e);
>>          }
>>      }
>>
>>
>> Modified: 
>> geronimo/server/trunk/modules/geronimo-client-builder/src/main/java/org/apache/geronimo/client/builder/AppClientModuleBuilder.java

>>
>> URL: 
>> http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-client-builder/src/main/java/org/apache/geronimo/client/builder/AppClientModuleBuilder.java?view=diff&rev=547679&r1=547678&r2=547679

>>
>> ============================================================================== 
>>
>> --- 
>> geronimo/server/trunk/modules/geronimo-client-builder/src/main/java/org/apache/geronimo/client/builder/AppClientModuleBuilder.java

>> (original)
>> +++ 
>> geronimo/server/trunk/modules/geronimo-client-builder/src/main/java/org/apache/geronimo/client/builder/AppClientModuleBuilder.java

>> Fri Jun 15 06:34:32 2007
>> @@ -230,7 +230,7 @@
>>                  throw new DeploymentException("Manifest class path 
>> entry is not allowed in a standalone jar (JAVAEE 5 Section 8.2)");
>>              }
>>          } catch (IOException e) {
>> -            throw new DeploymentException("Could not get manifest 
>> from app client module: " + moduleFile.getName());
>> +            throw new DeploymentException("Could not get manifest 
>> from app client module: " + moduleFile.getName(), e);
>>          }
>>
>>          String specDD;
>> @@ -391,7 +391,7 @@
>>                  gerAppClient = createDefaultPlan(path, appClient, 
>> standAlone, environment);
>>              }
>>          } catch (XmlException e) {
>> -            throw new DeploymentException(e);
>> +            throw new DeploymentException("Unable to parse 
>> application plan", e);
>>          }
>>          return gerAppClient;
>>      }
>> @@ -477,7 +477,7 @@
>>          try {
>>              
>> earContext.addIncludeAsPackedJar(URI.create(module.getTargetPath()), 
>> moduleFile);
>>          } catch (IOException e) {
>> -            throw new DeploymentException("Unable to copy app client 
>> module jar into configuration: " + moduleFile.getName());
>> +            throw new DeploymentException("Unable to copy app client 
>> module jar into configuration: " + moduleFile.getName(), e);
>>          }
>>          AppClientModule appClientModule = (AppClientModule) module;
>>          appClientModule.setEarFile(earFile);
>> @@ -493,7 +493,7 @@
>>          try {
>>              appClientDir = 
>> targetConfigurationStore.createNewConfigurationDir(clientEnvironment.getConfigId());

>>
>>          } catch (ConfigurationAlreadyExistsException e) {
>> -            throw new DeploymentException(e);
>> +            throw new DeploymentException("Unable to create 
>> configuration directory for " + clientEnvironment.getConfigId(), e);
>>          }
>>
>>          // construct the app client deployment context... this is 
>> the same class used by the ear context
>> @@ -520,7 +520,7 @@
>>              try {
>>                  
>> appClientDeploymentContext.addIncludeAsPackedJar(URI.create(module.getTargetPath()),

>> moduleFile);
>>              } catch (IOException e) {
>> -                throw new DeploymentException("Unable to copy app 
>> client module jar into configuration: " + moduleFile.getName());
>> +                throw new DeploymentException("Unable to copy app 
>> client module jar into configuration: " + moduleFile.getName(), e);
>>              }
>>              ClassPathList libClasspath = (ClassPathList) 
>> earContext.getGeneralData().get(ClassPathList.class);
>>              if (libClasspath != null) {
>> @@ -613,7 +613,7 @@
>>                  try {
>>                      
>> appClientDeploymentContext.addIncludeAsPackedJar(moduleBase, 
>> moduleFile);
>>                  } catch (IOException e) {
>> -                    throw new DeploymentException("Unable to copy 
>> app client module jar into configuration: " + moduleFile.getName());
>> +                    throw new DeploymentException("Unable to copy 
>> app client module jar into configuration: " + moduleFile.getName(), e);
>>                  }
>>
>>                  // add manifest class path entries to the app client 
>> context
>> @@ -766,7 +766,7 @@
>>              mainClas = classLoader.loadClass(mainClass);
>>          }
>>          catch (ClassNotFoundException e) {
>> -            throw new DeploymentException("AppClientModuleBuilder: 
>> Could not load main class: " + mainClass);
>> +            throw new DeploymentException("AppClientModuleBuilder: 
>> Could not load main class: " + mainClass, e);
>>          }
>>          while (mainClas != null && mainClas != Object.class) {
>>              classes.add(mainClas);
>> @@ -781,7 +781,7 @@
>>                  clas = 
>> classLoader.loadClass(cls.getStringValue().trim());
>>              }
>>              catch (ClassNotFoundException e) {
>> -                throw new 
>> DeploymentException("AppClientModuleBuilder: Could not load 
>> callback-handler class: " + cls.getStringValue());
>> +                throw new 
>> DeploymentException("AppClientModuleBuilder: Could not load 
>> callback-handler class: " + cls.getStringValue(), e);
>>              }
>>              classes.add(clas);
>>          }
>> @@ -804,7 +804,7 @@
>>          try {
>>              manifest = jarFile.getManifest();
>>          } catch (IOException e) {
>> -            throw new DeploymentException("Could not read manifest: 
>> " + jarFileLocation);
>> +            throw new DeploymentException("Could not read manifest: 
>> " + jarFileLocation, e);
>>          }
>>
>>          if (manifest == null) {
>> @@ -822,7 +822,7 @@
>>              try {
>>                  pathUri = new URI(path);
>>              } catch (URISyntaxException e) {
>> -                throw new DeploymentException("Invalid manifest 
>> classpath entry: jarFile=" + jarFileLocation + ", path=" + path);
>> +                throw new DeploymentException("Invalid manifest 
>> classpath entry: jarFile=" + jarFileLocation + ", path=" + path, e);
>>              }
>>
>>              if (!pathUri.getPath().endsWith(".jar")) {
>>
>> Modified: 
>> geronimo/server/trunk/modules/geronimo-clustering-wadi/src/main/java/org/apache/geronimo/clustering/wadi/BasicWADISessionManager.java

>>
>> URL: 
>> http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-clustering-wadi/src/main/java/org/apache/geronimo/clustering/wadi/BasicWADISessionManager.java?view=diff&rev=547679&r1=547678&r2=547679

>>
>> ============================================================================== 
>>
>> --- 
>> geronimo/server/trunk/modules/geronimo-clustering-wadi/src/main/java/org/apache/geronimo/clustering/wadi/BasicWADISessionManager.java

>> (original)
>> +++ 
>> geronimo/server/trunk/modules/geronimo-clustering-wadi/src/main/java/org/apache/geronimo/clustering/wadi/BasicWADISessionManager.java

>> Fri Jun 15 06:34:32 2007
>> @@ -113,7 +113,7 @@
>>          try {
>>              session = manager.createWithName(sessionId);
>>          } catch 
>> (org.codehaus.wadi.core.manager.SessionAlreadyExistException e) {
>> -            throw new SessionAlreadyExistException(sessionId);
>> +            throw new SessionAlreadyExistException("Session " + 
>> sessionId + " already exists", e);
>>          }
>>          return new WADISessionAdaptor(session);
>>      }
>>
>> Modified: 
>> geronimo/server/trunk/modules/geronimo-clustering-wadi/src/main/java/org/apache/geronimo/clustering/wadi/WADISessionAdaptor.java

>>
>> URL: 
>> http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-clustering-wadi/src/main/java/org/apache/geronimo/clustering/wadi/WADISessionAdaptor.java?view=diff&rev=547679&r1=547678&r2=547679

>>
>> ============================================================================== 
>>
>> --- 
>> geronimo/server/trunk/modules/geronimo-clustering-wadi/src/main/java/org/apache/geronimo/clustering/wadi/WADISessionAdaptor.java

>> (original)
>> +++ 
>> geronimo/server/trunk/modules/geronimo-clustering-wadi/src/main/java/org/apache/geronimo/clustering/wadi/WADISessionAdaptor.java

>> Fri Jun 15 06:34:32 2007
>> @@ -39,7 +39,7 @@
>>          try {
>>              session.destroy();
>>          } catch (Exception e) {
>> -            throw new IllegalStateException("Cannot release session 
>> " + session);
>> +            throw new IllegalStateException("Cannot release session 
>> " + session, e);
>>          }
>>      }
>>
>>
>>
>
>


Mime
View raw message