From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22423 invoked by alias); 14 Nov 2007 14:45:33 -0000 Received: (qmail 22407 invoked by uid 22791); 14 Nov 2007 14:45:30 -0000 X-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00,DK_POLICY_SIGNSOME,SPF_HELO_PASS,SPF_PASS,TW_FH X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 14 Nov 2007 14:45:26 +0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.13.8/8.13.1) with ESMTP id lAEEjNNm021302; Wed, 14 Nov 2007 09:45:23 -0500 Received: from pobox.corp.redhat.com (pobox.corp.redhat.com [10.11.255.20]) by int-mx1.corp.redhat.com (8.13.1/8.13.1) with ESMTP id lAEEjNoc006452; Wed, 14 Nov 2007 09:45:23 -0500 Received: from [127.0.0.1] (sebastian-int.corp.redhat.com [172.16.52.221]) by pobox.corp.redhat.com (8.13.1/8.13.1) with ESMTP id lAEEjLvr007196; Wed, 14 Nov 2007 09:45:22 -0500 Message-ID: <473B09B8.1070104@redhat.com> Date: Wed, 14 Nov 2007 14:45:00 -0000 From: Andrew Cagney User-Agent: Thunderbird 1.5.0.12 (X11/20070530) MIME-Version: 1.0 To: Mark Wielaard CC: frysk@sourceware.org Subject: Re: fhpd vs RuntimeExceptions References: <1195050364.3027.24.camel@dijkstra.wildebeest.org> In-Reply-To: <1195050364.3027.24.camel@dijkstra.wildebeest.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact frysk-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: frysk-owner@sourceware.org X-SW-Source: 2007-q4/txt/msg00126.txt.bz2 Mark Wielaard wrote: > Hi, > > While investigating some bugs I noticed that the fhpd sometimes swallows > the RuntimeExceptions from the core (and there I thought all was > fine...). Just fyi, broadly the message stuff, at least for normal output, is going away. The reason is that the cli alternates between using addMessage and PrintWriter.print(...) for normal out; so I've been migrating stuff to just do PrintWriter.print. But this leaves us still needing a way to consistently report errors. > 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(). We were printing both the error and the stack, that looked terrible (the number of times an exception occurs for legitimated reasons is surprising); so they were turned off. Did this turn them back on? > 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 > > * 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 > > ------------------------------------------------------------------------ > > 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); >