fhpd vs RuntimeExceptions

[Mark Wielaard <mark@klomp.org>]

[-- Attachment #1: Type: text/plain, Size: 1379 bytes --]

Hi,

While investigating some bugs I noticed that the fhpd sometimes swallows
the RuntimeExceptions from the core (and there I thought all was
fine...). This makes debugging the fhpd itself a little hard. I added a
possible exception cause to the Message class and while I was at it
added checks to make sure we don't present the user with an empty or
null message (which is very uninformative). Currently we always print
these possible exception causes when they are attached to a Message in
CLI.flushMessages(). But we could add a fhpd 'frysk_debug mode' so they
are only printed if an 'expert' is running frysk. For now I assume all
users are experts and want to see them, but maybe it is annoying.
Opinions?

2007-11-14  Mark Wielaard  <mwielaard@redhat.com>
    
     * CLI.java (addMessage): New variant that takes a possible
     exception cause.
     (doAttach): Use new addMessage().
     (execCommand): Likewise.
     (flushMessages): Add possible exception cause if present.
     Actually flush outWriter.
     * EvalCommands.java (eval): Add possible RuntimeException cause.
     * Message.java (Message): Add constructor that takes a possible
     exception cause.
     (getException): New method.
     * PlocationCommand.java (interpret): Add possible RuntimeException
     cause.

All tests pass before and after this patch (go hackers - all PASS!)

Cheers,

Mark

[-- Attachment #2: message_exception_cause.patch --]
[-- Type: text/x-patch, Size: 5430 bytes --]

diff --git a/frysk-core/frysk/hpd/CLI.java b/frysk-core/frysk/hpd/CLI.java
index 3c14b2e..d79e557 100644
--- a/frysk-core/frysk/hpd/CLI.java
+++ b/frysk-core/frysk/hpd/CLI.java
@@ -123,6 +123,7 @@ public class CLI {
 	    return topLevelCommand.complete(this, new Input(buffer), cursor,
 					    candidates);
 	} catch (RuntimeException e) {
+	    // XXX - FIXME - What if this is something fatal?
 	    return -1;
 	}
     }
@@ -142,7 +143,7 @@ public class CLI {
 	    outWriter.print("Attached to process ");
 	    outWriter.println(attached);
         } catch (InterruptedException ie) {
-            addMessage("Attach interrupted.", Message.TYPE_ERROR);
+            addMessage("Attach interrupted.", Message.TYPE_ERROR, ie);
             return;
         } finally {
             synchronized (this) {
@@ -253,19 +254,14 @@ public class CLI {
 		    topLevelCommand.interpret(this, command);
                 }
             }
-            catch (NullPointerException e) {
-                e.printStackTrace();
-                String msg = "";
-                if (e.getMessage() != null)
-                    msg = e.getMessage();
-
-                addMessage(msg, Message.TYPE_DBG_ERROR);
-            }
+            catch (InvalidCommandException ice) {
+                addMessage(ice.getMessage(), Message.TYPE_ERROR);
+	    }
             catch (RuntimeException e) {
-                String msg = "";
-                if (e.getMessage() != null)
-                    msg = e.getMessage();
-                addMessage(msg, Message.TYPE_ERROR);
+                String msg = e.getMessage();
+                if (msg == null || msg.equals(""))
+                    msg = e.toString();
+                addMessage(msg, Message.TYPE_DBG_ERROR, e);
             }
             flushMessages();
         }
@@ -280,6 +276,10 @@ public class CLI {
         addMessage(new Message(msg, type));
     }
 
+    void addMessage(String msg, int type, Throwable exc) {
+        addMessage(new Message(msg, type, exc));
+    }
+
     private void flushMessages() {
         for (Iterator iter = messages.iterator(); iter.hasNext();) {
             Message tempmsg = (Message) iter.next();
@@ -293,8 +293,12 @@ public class CLI {
             if (prefix != null)
                 outWriter.print(prefix);
             outWriter.println(tempmsg.getMessage());
+	    Throwable exc = tempmsg.getException();
+	    if (exc != null)
+		exc.printStackTrace(outWriter);
             iter.remove();
         }
+	outWriter.flush();
     }
 
     PTSet createSet(String set) {
diff --git a/frysk-core/frysk/hpd/EvalCommands.java b/frysk-core/frysk/hpd/EvalCommands.java
index 5c69032..d9679db 100644
--- a/frysk-core/frysk/hpd/EvalCommands.java
+++ b/frysk-core/frysk/hpd/EvalCommands.java
@@ -96,7 +96,10 @@ abstract class EvalCommands extends ParameterizedCommand {
 	    try {
 		result = cli.parseValue(task, expression, options.dumpTree);
 	    } catch (RuntimeException nnfe) {
-		cli.addMessage(nnfe.getMessage(), Message.TYPE_ERROR);
+		String msg = nnfe.getMessage();
+		if (msg == null || msg.equals(""))
+		    msg = nnfe.toString();
+		cli.addMessage(msg, Message.TYPE_ERROR, nnfe);
 		continue;
 	    }
 
diff --git a/frysk-core/frysk/hpd/Message.java b/frysk-core/frysk/hpd/Message.java
index 933305f..aeb4ed2 100644
--- a/frysk-core/frysk/hpd/Message.java
+++ b/frysk-core/frysk/hpd/Message.java
@@ -49,17 +49,39 @@ class Message
 	public static int TYPE_NORMAL = 3;
 	public static int TYPE_VERBOSE = 4;
 
-	String msg = null;
-	int type = 0;
+	private final String msg;
+	private final int type;
+        private final Throwable exc;
 
-	public Message (String msg, int type)
+        /**
+	 * Creates a new Message with the given message and type
+	 * and no exception.
+	 */
+        public Message (String msg, int type)
+        {
+	    this(msg, type, null);
+	}  
+        /**
+	 * Creates a new Message with the given message and type.
+	 * The message cannot be null or empty. The exception is optional
+	 * and may be null.
+	 */
+        public Message (String msg, int type, Throwable exc)
 	{
+	        if (msg == null)
+		    throw new NullPointerException("null msg");
+
+	        if (msg.equals(""))
+		    throw new IllegalArgumentException("empty msg");
+
 		this.msg = msg;
 
 		if (type < TYPE_DBG_ERROR || type > TYPE_VERBOSE)
 			throw new IllegalArgumentException("Debugger message created with illegal type.");
 		else
 			this.type = type;
+
+		this.exc = exc;
 	}
 
 	public String getMessage()
@@ -71,4 +93,9 @@ class Message
 	{
 		return type;
 	}
+
+        public Throwable getException()
+        {
+	        return exc;
+        }
 }
diff --git a/frysk-core/frysk/hpd/PlocationCommand.java b/frysk-core/frysk/hpd/PlocationCommand.java
index 225cc09..61ced31 100644
--- a/frysk-core/frysk/hpd/PlocationCommand.java
+++ b/frysk-core/frysk/hpd/PlocationCommand.java
@@ -77,7 +77,10 @@ class PlocationCommand extends ParameterizedCommand {
             try {
                 result = cli.parseValue(task, sInput);	  
             } catch (RuntimeException nnfe) {
-                cli.addMessage(nnfe.getMessage(), Message.TYPE_ERROR);
+		String msg = nnfe.getMessage();
+		if (msg == null || msg.equals(""))
+		    msg = nnfe.toString();
+                cli.addMessage(msg, Message.TYPE_ERROR, nnfe);
                 continue;
             }
 	    result.getLocation().toPrint(cli.outWriter);