Making log parsing errors visible in the GWT interface

[GoogleCodeExporter]

Log parsing with the current GWT interface is a one-shot deal -- either 
everything works, or nothing works and the user doesn't know why. Internally, 
however, the TraceParser class generates all sorts of useful exceptions that 
can help the user understand the error and correct their regular expressions. 
This information must be (1) caught in the GWT server, and (2) made visible to 
the user.

For example, if there is a problem with a regular expression, the interface 
should highlight the regular expressions that is problematic and say what the 
problem is -- such as "malformed regular expression", or "regular expressions 
missing TYPE named group", etc.

Original issue reported on code.google.com by bestchai on 14 Jun 2011 at 1:48

[GoogleCodeExporter]

Issue 82 has been merged into this issue.

Original comment by bestchai on 14 Jun 2011 at 2:06

[GoogleCodeExporter]

Original comment by bestchai on 14 Jun 2011 at 10:46

• Changed state: Accepted

[GoogleCodeExporter]

The "Remote Procedure Call failure while parsing log" message does not go away 
if the user corrects the problem and re-runs Synoptic model creation (even if 
the subsequent run does not have any RPC failure).

Original comment by michael.ernst@gmail.com on 14 Jun 2011 at 11:19

[GoogleCodeExporter]

Original comment by bestchai on 14 Jun 2011 at 11:51

• Added labels: Student-Starter-Task

[GoogleCodeExporter]

Original comment by bestchai on 28 Aug 2011 at 4:55

• Changed state: New

[GoogleCodeExporter]

Taking ownership.

Original comment by T.101.JV on 8 Sep 2011 at 8:49

[GoogleCodeExporter]

T.101.JV:

I've got some errors properly being surfaced to the user, but others are still 
getting lost. I'm also curious as to whether it would be okay for me to replace 
InternalSynopticException with ParseException. Maybe I will end up doing that 
to fix my current problem for surfacing the addRegex() exceptions. If I'm lucky 
I can catch and present the information from the Exception base class rather 
than the ParseException class, but I think I tried that and it didn't work.

Ah okay, so the ParseException gets wrapped into an InternalSynopticException 
which is a runtime exception. I can't catch that as an exception and surface 
the information. I'll come back tomorrow and work on the checked exception 
cases, but I'd like to defer to you, Ivan, on what to do for the unchecked 
exception cases.

Original comment by bestchai on 11 Sep 2011 at 9:06

[GoogleCodeExporter]

To my knowledge everything that implements Throwable (e.g., Exception)
can be caught in Java. Therefore, runtime exceptions can be caught (as
they extend Exception), as far as I know. Yes, we certainly want to
handle/catch and surface to the user all the Exceptions that might
occur. It would be nice to fail silently, but if the invariants cannot
be mined or the initial graph cannot be created then there's nothing
we can do but fail and tell the user.

Original comment by bestchai on 11 Sep 2011 at 9:06

[GoogleCodeExporter]

T.101.JV (revision 86d2d3464ba1):

My changes take error information that is being logged and places it into a 
ParseException that is already being thrown nearby. There are also places where 
InternalSynopticException is wrapping ParseException, so I've made reasonable 
modifications to InternalSynopticException to accomodate error surfacing. I'm 
also assuming that the relevant exceptions are being thrown only inside of 
SynopticService.parseLog when the parser is constructed or when 
TraceParser.parseTraceString is called.

Short of breaking each if statement that throws a ParseException, I can't think 
of a decent way to test that all of the error information is being surfaced. I 
also get the feeling that you, Ivan, could probably figure out if I am missing 
something with a quick look.

I'm more or less done with the first half of Issue 90 now. I'm going start 
working on highlighting malformed regular expressions. I feel like it's not 
going to be a trivial change, but I haven't looked into it that much yet. 

Original comment by bestchai on 11 Sep 2011 at 9:07

[GoogleCodeExporter]

Yes, for now just assume that the errors are parsing errors --
basically, if the parser things everything is fine then the partition
graph construction should never fail (if it does, we would want to
tell the user, but this should happen rarely, and we don't really need
to include much debugging info).

For testing, we do need test cases that have maximal code coverage --
that is, exercise all the if/else statements in the parsing code. But,
I would create a new issue for this and consider this future work
because it will take a while and I would like a better means of
reusing Synoptic tests in the gwt code (all those parsing errors are
already exercised in the synoptic unit tests, so re-writing this code
from scratch doesn't make sense).

For associating errors with regular expressions, I think you should
extend the parsing exception class to bundle information about which
reg exp, if any, caused the problem. Since regular expressions are
given as a list, it might be sufficient to simply specify the index of
the regular expression. I think that simply highlighting the
problematic reg exp is enough here -- no need to highlight parts of it
or anything with higher granularity. Also, the problem might be with
the log file (e.g., there is a line that does not match any of the
regular expressions), in this case associating the error with the text
and specifying the line number should be enough (though, we also need
to display line numbers somehow..).

Original comment by bestchai on 11 Sep 2011 at 9:07

[GoogleCodeExporter]

I've finished implementing this change. I'd like a code review please. After 
the review process, I'll integrate the change into default.

Original comment by T.101.JV on 18 Sep 2011 at 1:17

[GoogleCodeExporter]

Closing issue.

Original comment by T.101.JV on 21 Sep 2011 at 5:55

• Changed state: Fixed