Debugger now dies verbosely ...

dkasak wrote on Tue Aug  9 05:36:10 MEST 2005:

I've upgraded to EPIC-0.3.11 ( from the stable 0.3.0, which also doesn't
work for me ... see previous post ).

Now the debugger dies quite verbosely:

---

Name not found[415]

Name not found[416]

String not found[418]

String not found[418]

!!!!! Parse Error!!!!
!ENTRY org.epic.debug.perldebugger.perl **Error** 0 Aug 09, 2005 13:32:21.375
!MESSAGE Error Parsing Debugger Variables
!STACK 0
java.lang.Exception: *******************************
*******************************
+++++++Error Parsing Vars++++++
*******************************
*******************************
Subroutine main::dumpValue redefined at /usr/lib/eclipse-3.1/plugins/org.epic.debug_0.3.1//dumpvar_epic.pm
line 46.
 at /usr/lib/eclipse-3.1/plugins/org.epic.debug_0.3.1//dumpvar_epic.pm line
46
        require dumpvar_epic.pm called at (eval 16)[/usr/lib/perl5/5.8.7/perl5db.pl:628]
line 2
        eval '($@, $!, $^E, $,, $/, $\\, $^W) = @saved;package main; $^D
= $^D | $DB::db_stop;
;{eval { require PadWalker; PadWalker->VERSION(0.08) }or print $DB::OUT
("PadWalker module not found - please install\\n");do \'dumpvar_epic.pm\'
unless defined &main::dumpvar_epic;defined &main::dumpvar_epic or print
$DB::OUT "dumpvar_epic.pl not available.\\n";my $h = eval { PadWalker::peek_my(2)
};my @vars = split (\' \',\'\');$@ and $@ =~ s/ at .*//, print $DB::OUT
($@);my $savout = select($DB::OUT);dumpvar_epic::dumplex($_,$h->{$_},defined
$option{dumpDepth} ? $option{dumpDepth} : -1,@vars) for sort keys %$h;print
"E";select($savout);};;

;' called at /usr/lib/perl5/5.8.7/perl5db.pl line 628
        DB::eval called at /usr/lib/perl5/5.8.7/perl5db.pl line 3410
        DB::DB called at /home/dan/workspace/axis/demos/PDF-ReportWriter/demo.pl
line 27
E-------------------------------
-------------------------------
+++++++Error Parsing Vars++++++
-------------------------------
-------------------------------

        at org.epic.debug.varparser.TokenVarParser.logParsError(TokenVarParser.java:256)
        at org.epic.debug.varparser.TokenVarParser.parseVars(TokenVarParser.java:65)
        at org.epic.debug.varparser.TokenVarParser.parseVars(TokenVarParser.java:40)
        at org.epic.debug.PerlDB.setVarList(PerlDB.java:1227)
        at org.epic.debug.PerlDB.access$4(PerlDB.java:1217)
        at org.epic.debug.PerlDB$VarUpdateJob.run(PerlDB.java:1646)
        at org.eclipse.core.internal.jobs.Worker.run(Worker.java:76)

---

Any help would be greatly appreciated.

I'm using Linux PPC if it makes any difference.
I also have PadWalker installed.

jploski wrote on Tue Aug  9 14:15:12 MEST 2005:

The piece of code which outputs the message you quoted is an example of
what would qualify under "request for enhancement" #1114170 (inappropriate
exception handling):

        try{
        readVars();
        }catch(Exception e){mHasErrors = true;}
        if (mHasErrors) {
            System.out.println("!!!!! Parse Error!!!!");
            logParsError(fText);
        } else {
            System.out.println("!!!!! Parse OK!!!!");
        }

With code like this, you (the user) have to compensate for the original
author's convenience, lack of time and/or ignorance. Because it is morally
wrong (?) to complain about free code, I suggest that you check out the
latest version from CVS and debug this part (with Eclipse's built-in Java
debugger) to see what is really going on: what kind of exception is being
thrown and why.

stephan_ruehl wrote on Wed Aug 10 15:42:02 MEST 2005:

@Jan Ploski
If you would have taken the time to read the code correctly before commenting
on it you would have noticed that this is no issue with the Java code but
with the Perl code used and that no exception is raised. Furthermore the
issue is logged and reported to the user. So at least for this instance
you miss the point ....

Stephan

jploski wrote on Sat Aug 13 15:16:30 MEST 2005:

Stephan,

I stand corrected: the implementation is even worse than I assumed. Instead
of using Java exceptions, the code relies on System.out.println and a boolean
flag to report errors.

The amount, kind and mode of providing information about the error to the
user is inadequate. It does not provide any clue about how to fix the underlying
problem without stepping through the implementation's source code (both
Java and Perl).

I am emphasizing the point not to criticize any individual, but rather to
raise awareness of a topic which I feel is largely neglected in software
projects. Perhaps even more frequently in open source projects, which lack
commercial punishment for publishing poor (or just "unpolished") products
and may use the free availability of source code as an excuse for sloppiness.

-JPL

stephan_ruehl wrote on Tue Aug 16 17:04:10 MEST 2005:

JPL,


it is of course important to address general aspects of software quality.
But if there already is an agreement on the general topic it is not very
helpful restating the point again and again. On the other hand there is
a certain trade of between functionality and quality if you are operating
with limited resources. Under this circumstances "good enough for most users"
is more adequate than aiming for perfection.
To find this balance it is necessary and helpful to address specific topics
within the implementation. This requires a thorough analysis of the problem
and an evaluation of the implementation to see if the issue is handled adequately.
Concerning the issue stated by Dan about the "debugger dieing verbosely"
i will give you a brief sketch of the situation and how it is handled in
the code (just in case you haven't had the time to look at it yourself closely
enough).
First of all we need to distinguish two issues (both are intermixed within
the problem that Dan describes):
1. variables are not parsed/displayed correctly 
2. debugger seems to die

Starting with the first problem:
1. variables are not parsed/displayed correctly
Issue:
EPIC uses a Perl script (dumpvar_epic.pm)to get the variables defined within
the script being debugged (and their current values). A parser implemented
in Java then reads in the output generated by dumpvar_epic.pm and passes
this information to Eclipse for displaying. Issues might arise if  a) the
output generated is syntactically incorrect b) the parser is not implemented
correctly. In either cases variables might not be displayed correctly/completely,
but debugging can proceed nevertheless. Therefore the user should be notified
that data shown might be incorrect. As a "user" he has no further options
as to report the issue. Therefore the error message should give enough information
to allow for making a sensible error report.
Implementation:
If the parser encounters a problem it throws an exception and tries to restart
parsing. This usually leads to quite a number of exceptions (>>100) until
the parser can find a point to restart. Detailed information about that
is printed to the console (e.g. "Name not found[415]"). This information
is mainly intended for debugging the parser and is much to detailed to be
reported to the user. Nevertheless a flag is set to indicate that there
has been a parsing error.
After parsing has finished this flag is checked. If there had been parsing
errors this is reported to the user. Reporting is done in two ways:
a) writing the error to the log file (using the standard Eclipse mechanism)
b) a dialog box including the same information is displayed

The the error message includes the output of dumpvar_epic.pm which is essential
for reproducing and correcting the issue.
So in this case we are just discussing the user has been informed that the
variables displayed might not be correct and that he can proceed with debugging
(the message could be more verbosely, point grated). Additionally the user
is provided with enough information to file an error report (the information
was enough to find the source of the problem just by having a superficial
look at the error description). 
So what is the major aspect to be improve within the correct implementation
?

Coming to the second topic:
2. debugger seems to die
The debugger does not die. The impression is caused by the fact that vital
information (threads, stack frame, variables ...) are not displayed and
therefore stepping commands cannot be executed. This is caused by some changes
introduced to Eclipse 3.1 (displaying of threads, stack frame, variables
etc. uses some mechanisms not used in Eclipse 3.0). As no exceptions are
passed to EPIC this issue cannot be detected/reported during runtime (The
next version of EPIC will run with ECLIPSE 3.1).




So if your previous stated already considered these aspects, please would
you be so kind as to provide me with more detailed improvement suggestions.
If this is not the case, please be so kind as to make a more thorough analysis
of the issue discussed and the code related to this issue before stating
your opinion in such an indicting way.

Regards

 Stephan

jploski wrote on Tue Aug 16 22:14:06 MEST 2005:

Stephan,

I would call the implemented error handling perfect if the user (Dan) could
proceed in one of the following ways:

1. Figure out how to work around the problem on his own, based on feedback
given by the plugin. Let's say, within half an hour from seeing the error
message (and that's plenty of time).
2. Submit a bug report containing enough information for an immediate diagnosis
of the fault by anyone with access to EPIC's source code (not just by people
intimately familiar with the internals).

The error report should clearly point out which of the above two alternatives
is preferable (I am thinking about a message like "I am a bug, report me"
or "You can try this and that" here). What happened instead was "strange"
behavior and "debugger dying".

I have seen too many hopelessly tricky bugs to believe myself that the above
goals are easy to achieve in practice. However, they are worth pursuing,
more so in community-driven projects that expect and welcome external contributions.
Fortunately, there are some simple measures that can improve matters. My
basic proposition is that one should not spend effort on fixing _individual_
bugs, but rather on speeding up the process of bug fixing in general. The
primary means appears to be disciplined runtime error handling, which I
shall discuss below in some detail.

There seem to be two problems with error handling in the present case:

1. The error is not detected early enough. It is detected in the parser,
while the reason is quite probably in the Perl script which produces the
parser's input.
2. The user does not get the proper feedback on what state the computation
was left in and how to proceed. You gave both answers in your detailed reply
(some variables may appear incorrectly; the user should report a bug and
provide logged information to developers). However, it is the failing code
which should have given these hints in the first place.

Regarding error detection: speeding up error detection requires knowing
the expected input and output of each routine in the Perl script used to
parse variables. You have to check program states frequently and collect
information about deviations of real values from expected ones.

However, I could not find any kind of documentation for this script. Maybe
I overlooked it, but neither the expected input nor output of this script
is described anywhere. Where is the grammar? If the syntax is trivial enough
not to require BNF, where is a one-paragraph description? I am not going
to ask about automated tests.

Another unspecified thing is the functionality of the routines contained
in both the script and the Java code. By specification I mean a description
of what input state is expected and what output state is promised by each
routine. Based on experience, even an informal description helps a lot:
one written in natural language, but precise enough for distinguishing correct
and incorrect behaviors without having to read through (recursively) the
code of all invoked subroutines. Javadoc was invented for a reason... In
addition to the informal description, runtime assertions can help immensely
with recognizing error occurrences early and avoiding uncontrolled propagation
of errors.

You rightly assume that I did not spend enough time on reverse-engineering
the required specifications from the implementation. Note that without such
disciplined specifications, one cannot exactly talk about correctness or
proper error handling. I find it amusing that you refer to a "correct implementation"
in this context and also while discussing a bug report (reminds me of the
old "it's not a bug, it's a feature" mantra). You (possibly) have the advantage
of having the spec in your mind after thorough analysis, but others (like
myself and Dan) do not have this advantage and should not need to spend
a large amount of effort to gain it.

One might argue that the source code _is_ the detailed documentation, and
that studying it carefully (given enough time, of which there is supposedly
plenty in open source projects) is quite sufficient for resolving bugs.
This is simply not true. The source code provides an exact and very low-level
description of how each piece of code BEHAVES, but it says nothing about
how it SHOULD BEHAVE, WHY it behavs the way it does or WHO is to blame if
it does not meet expectations (or praise if it does).

My advice is simple:
1. Start by specifying the intended behavior (input and output states) for
each routine. Pay an equal amount attention to computation states (mostly
specs) and state transitions (mostly code).
2. Check for state deviations frequently
3. Signal an exception as soon as the first deviation is detected

As you can see, I have no suggestions of the type "fix this piece of code
in that method to make it better". If you were expecting such suggestions,
sorry.

In short, it is very difficult to talk about proper error handling and recovery
if even the first step from the above list is missing. (Error reporting
in parsers/compilers is a broad topic itself, but I do not think we actually
have this issue on our hands here.)

Regarding error messages: the recommended way of logging errors in Eclipse
plugins is not by writing to System.out.println, but by logging via the
plugin's ILog interface (the logged messages then appear in the view Error
Log).

The more important issue, however, appears to be not how logging should
occur, but what to log. It is reasonable to assume that the original recipient
of your error messages will be the user. Therefore, the messages should
be addressed at her, explain the reason of the error, describe the amount
of damage caused and suggest corrective actions.

It should be made clear which part of the output contains diagnostic messages
intended for developers (and thus does not have to be understood). Eclipse
supports hierarchical logging of multiple IStatus objects to achieve this
goal.

Furthermore, the error log entry should allow easy tracking of the exact
source code line where the error was originally detected, as well as reconstruction
of how it was propagated through the system. In other words, it should include
a detailed stack trace (or multiple traces for nested and concurrent exceptions).
Note that the stack trace logged in our discussed case does not point at
the spot at which the error was detected.

An important rule of exception propagation is not throwing away potentially
valuable information. Instead, additional information should be collected
during propagation. By printing out messages such as "Name not found[415]"
early on and then relying on a boolean flag you are ripping information
out of the context in which it was collected and can be meaningfully interpreted.
This is a painfully common mistake. Instead, I would suggest gathering the
information in an exception object, possibly at multiple stages of its propagation,
and logging it all at _one_ place.

Another issue that jumps to my mind (and indeed, the one which triggered
my first post in this thread) is demonstrated by the following piece of
code:

try { readVars(); } catch (Exception e) { mHasErrors = true; }

Although not directly related to Dan's problem, as you correctly pointed
out, this code is a prime example of discarding useful diagnostic information.
It is also a case of a blanket catch clause: one cannot really do anything
about java.lang.Exception other than forget it, propagate it (your invoker
won't know any better what to do either) or log it. Catch clauses (like
error messages) should be as specific as possible, unless they are used
as a last resort for coarse fault tolerance in transactional environments.

I hope that these suggestions, although of general nature (for reasons explained
above), will help improve EPIC's code and save some troubleshooting time
for both users and developers.

I cannot comment in depth on the "debugger seems to die" issue. Having read
the compatibility notes for Eclipse 3.1, it would surprise me if they had
broken the debugger API. On the other hand, if you are using 3.0-only internal
classes, it would be easy to detect 3.1 and inform the user about the problem
in meaningfully. Which brings us back to the topic of error messages...

As for the cathedral/bazaar argument: I second the opinion that it is important
to focus on functionality with the greatest value to users and release early/often
(definitely before "perfection" is achieved), but at the same time I maintain
that one should be very explicit about limitations of the released product.
Practicing disciplined error handling is one way of achieving such explicitness.

Best regards -
JPL

stephan_ruehl wrote on Wed Aug 10 15:32:29 MEST 2005:

The problem you are facing originates from the implementation of the module
dumpvar_epic.pm which  EPIC uses to retrieve variables and their values.
You most likely defined a routine called dumpValue(). This collides with
the definition in dumpvar_epic.pm which also defines a routine main::dumpValue().
This issue will be resolved in the next version of EPIC.
For a quick fix:
a) rename the routine mentioned
b) recompile EPIC from the CVS repository
c) if it is really urgent contact me (and I will provide you with a fixed
version)

Regards

 Stephan