[jelly] XMLParser exception patch

[Kevin Ross]

I’ve run into a few issues where I make mistakes in my script, namely bad jexl expressions in attributes.  Exception stacks where somewhat incomprehensible, and no line number was present, when I know the fine Jelly code has it somewhere ;)   I’ve removed a few calls to log.warn, where useful messages where there, but not in the exception message (now added).  Also, I have removed one call to e.printStackTrace().  These were definitely removed to reduce logging, enable the exception to have the full context, and allow the caller to log or throw as they wish.   On the issue of visibility, I have found a crazy qwerk in the SAXException.toString() when a cause (exception) is added to it.  It simply forgets it’s message by calling (internally) getException().toString() (equivalent to jdk1.4 getCause().toString()).  To properly see the message from the SAXException, including SAXParseException, you must explicitly call SAXException.getMessage() + SAXException.toString().  This is probably why these other log statements were placed there.   I have not done this yet, but I additionally propose we add a JellyParseException (extends SAXParseException) that will override this behavior and toString() properly, and change this in XMLParser.  I would be glad to do so if the commiters are on board.   Kevin Ross   PS- in the patch-diff, I left the old lines commented.  Is it conventional to do so, or just make my changes?

Index: XMLParser.java
===================================================================
RCS file: 
/home/cvspublic/jakarta-commons/jelly/src/java/org/apache/commons/jelly/parser/XMLParser.java,v
retrieving revision 1.45
diff -u -r1.45 XMLParser.java
--- XMLParser.java      24 Jan 2003 10:04:33 -0000      1.45
+++ XMLParser.java      20 Mar 2003 17:31:54 -0000
@@ -1084,10 +1084,8 @@
             return null;
         }
         catch (Exception e) {
-            log.warn(
-                "Could not create taglib or URI: " + namespaceURI + " tag name: " + 
localName,
-                e);
-            throw createSAXException(e);
+            //log.warn("Could not create taglib or URI: " + namespaceURI + " tag 
name: " + localName,e);
+            throw createSAXException("Could not create taglib or URI: " + 
namespaceURI + " tag name: " + localName, e);
         }
     }
                 
@@ -1125,13 +1123,8 @@
             return script;
         }
         catch (Exception e) {
-            log.warn(
-                "Could not create static tag for URI: "
-                    + namespaceURI
-                    + " tag name: "
-                    + localName,
-                e);
-            throw createSAXException(e);
+            //log.warn("Could not create static tag for URI: " + namespaceURI + " tag 
name: " + localName, e);
+            throw createSAXException("Could not create static tag for URI: " + 
namespaceURI + " tag name: " + localName, e);
         }
     }
 
@@ -1204,6 +1197,22 @@
         return new JexlExpressionFactory();
     }
 
+       protected String getLocationMessage(){
+               
+        if (locator != null) {
+            return
+                "Error at ("
+                    + locator.getLineNumber()
+                    + ", "
+                    + locator.getColumnNumber()
+                    + ")";
+        }
+        else{
+        
+               return "Error at (unknown location)";           
+        }
+       }
+       
     /**
      * Create a SAX exception which also understands about the location in
      * the file where the exception occurs
@@ -1211,29 +1220,23 @@
      * @return the new exception
      */
     protected SAXException createSAXException(String message, Exception e) {
-        log.warn("Underlying exception: " + e);
-        e.printStackTrace();
+        //log.warn("Underlying exception: " + e);
+        //e.printStackTrace();
+        String newMessage = getLocationMessage() + ": " + message;
         if (locator != null) {
-            String error =
-                "Error at ("
-                    + locator.getLineNumber()
-                    + ", "
-                    + locator.getColumnNumber()
-                    + "): "
-                    + message;
             if (e != null) {
-                return new SAXParseException(error, locator, e);
+                return new SAXParseException(newMessage, locator, e);
             }
             else {
-                return new SAXParseException(error, locator);
+                return new SAXParseException(newMessage, locator);
             }
         }
-        log.error("No Locator!");
+        //log.error("No Locator!");
         if (e != null) {
-            return new SAXException(message, e);
+            return new SAXException(newMessage, e);
         }
         else {
-            return new SAXException(message);
+            return new SAXException(newMessage);
         }
     }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]