Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.

Close

Proposal to report parsing errors

Developers
Mahesh DC
2006-12-18
2013-05-15
  • Mahesh DC
    Mahesh DC
    2006-12-18

    This is a proposal to implement reporting errors while parsing a tdi file.
    It is intended to also handle other conditions like file not found, no read
    access etc.,

    Currently, the Parser is ran as a workbench job from the TraceEditor class.
    The workbench job calls run() method of the Parser class, in which the
    actual parsing method parseFile() is called.  parseFile() is an internal
    method which does not return anything.

    The proposal below, only describes how to catch exceptions at the parseFile()
    level and report it back to the user. The handling of exceptions deep within
    the parseFile() sub-methods, is not discussed here. But, it should be fairly
    simple to implement them, and the suggested method is to create an Exception
    at the deepest level (eg., an error in the tdi file is found) and pass it all
    the way to the parseFile() method.

    - Change Parser.parseFile() method to throw java.lang.Exception
       - catch all parsing errors inside this method and throw it

    - Change Parser.run() so that the exception from the parseFile() is caught,
      and a org.eclipse.core.runtime.Status object is created as follows:

      IStatus status = new Status(IStatus.ERROR, PLUGIN_ID, IStatus.ERROR,
                                          e.getMessage(), e);

      This status object is returned, but only after calling monitor.done().

    - In the TraceEditor.init(), the return value of Parser.run() is checked.
      If it is not Status.OK_STATUS, a InvocationTargetException() is thrown
      as follows:

      //Notice the embedding of status.getException()
      throw new InvocationTargetException(status.getException());

    - This gets caught in the Exception handler for InvocationTargetException,
      in the same method, i.e., TraceEditor.init()
       - The user is reported of the error, using the MessageDialog()
         and e.getCause().getMessage()
       - After the user has pressed OK on the MessageDialog,
         a PartInitException(IStatus) is thrown, to prevent calling of
         TraceEditor.createPartControl(). Instead of a incomplete TraceViewer,
         the user will see the default view created by Eclipse as a result of a
         PartInitException viz., an Error display with a message similar to
         "Unable to create this part due to an internal error. Reason for the
         failure:" followed by status.getException().getMessage(). Also a
         "Details >>" button is shown, which when pressed, displays the stack
         trace of the exception.

     
    • Martijn Rutten
      Martijn Rutten
      2006-12-18

      The proposal looks fine to me.
      However, please notice that currently the parser does not run as a real 'job'
      Maybe we should also change this and run the parser as a separate job, and wait for completion of the job before continuing in setting up the editor UI.

       
    • Martijn Rutten
      Martijn Rutten
      2006-12-19

      In addition to Mahhesh' proposal:

      - Remove 'extends Job' from the Parser class: it already runs as a job via the IRunnableWithProgress created in the TraceEditor
      - Change the parameters of the window.run() call to true to run the parser on a separate thread, and to allow the user to cancel parsing.
      - While parsing, for each new line, check monitor.isCanceled() return IStatus.CANCEL if canceled
        and handle this in the trace editor by throwing an InterruptedException which in return closes the open editor. Something like this (but hopefully a little less dirty):

      try {
          window.run(true, true, new IRunnableWithProgress() {
              public void run(final IProgressMonitor monitor) throws InvocationTargetException, InterruptedException {
                  IStatus status = parser.run(monitor);
                  if (!status.isOK()) {
                      throw new InterruptedException();
                  }
              }
          });
      } catch (InterruptedException e) {
          site.getPage().closeEditor(this, false);
      }

       
    • Mahesh DC
      Mahesh DC
      2006-12-20

      I agree to removing the Job sub-classing by the Parser. Here are a few further changes, that I propose

      - Rename Parser.run(IProgressMonitor) to Parser.doParse(IProgressMonitor), as run() is no more required
           = We still need to pass IProgressMonitor and even pass it further down to parseFile() method, to check for user cancelling using IProgressMonitor.isCanceled(), for each line parsed.
      - Instead of using IStatus to return from Parser.doParse, I am depending on parsing errors and user-cancellation, using only exceptions
      - Since IRunnableWithProgress.run() and IWorkbenchWindow.run() require handling of InterruptedException and InvocationTargetException, it becomes a bit messy to do it inside IRunnableWithProgress(anonymous) class. So, I propose to change doParse to throw both InterruptedException and InvocationTargetException.

      I am pasting the code of both Parser.doParse and IWorkbenchWindow.run() methods

      public final void doParse(final IProgressMonitor monitor) throws InterruptedException, InvocationTargetException {
          monitor.beginTask("Parsing trace...", IProgressMonitor.UNKNOWN);
         
          try {

              parseFile(monitor);
              compensateStartTime(calculateStartTime());
              model.setEndTime();
              closeLines();
              model.computeMaxValues();

          } catch (InterruptedException e) {
              //Is thrown by parseFile() method, when IProgressMonitor.isCanceled() returns true
              throw e;    
          } catch (Exception e) {
              //Any other exceptions like FileNotFoundException, is caught here and embedded into InvocationTargetException
              throw new InvocationTargetException(e);
          } finally {
              //To stop the progress monitor
              monitor.done();
          }
      }

      //Change in TraceEditor.init()
      IWorkbenchWindow window = this.getSite().getWorkbenchWindow();
      try {
          window.run(true, true, new IRunnableWithProgress() {

              public void run(final IProgressMonitor monitor) throws InvocationTargetException, InterruptedException {
                  parser.doParse(monitor);
              }

          });
      } catch (InvocationTargetException e) {

          // A parse exception
          MessageDialog.openError(this.getSite().getShell(), "Error", "Init failed: " + e.getCause().getMessage());
          throw new PartInitException(e.getCause().getMessage(), e.getCause()); //Message to the user

      } catch (InterruptedException e) {

          // User pressed cancel, close the editor
          super.getSite().getPage().closeEditor(this, false);
          return;

      }

      Further to this, parseFile(IProgressMonitor) is changed to throw java.lang.Exception. Any type of exception that happens further down, is reported by this method to doParse(IProgressMonitor) method.

      If this is okay, I will send a patch that includes these changes and -ve time error handling (that originally started this discussion). Rest of the error-handling can be taken up during the parser test-bench updatation task.

       
    • Martijn Rutten
      Martijn Rutten
      2006-12-22

      Okay, make my day ;-)

       
    • Mahesh DC
      Mahesh DC
      2007-01-17