analysis.lami: Replace OnDemandAnalysisException with CoreException
authorAlexandre Montplaisir <alexmonthy@efficios.com>
Wed, 4 May 2016 21:23:12 +0000 (17:23 -0400)
committerAlexandre Montplaisir <alexmonthy@efficios.com>
Fri, 20 May 2016 18:32:21 +0000 (14:32 -0400)
We may want to pass additional information to the exceptions thrown
when a LAMI analysis does not complete succesfully, like what
type of error should be displayed.

CoreException and IStatus are made exactly for that, and they hook
nicely into ErrorMessage to display, for example, a command-line
stderr output as additional Details in the dialog.

That way, a message like "no results returned" won't look like
a scary error.

Bug: 493941

Change-Id: Id3e6711ac410c3d993b4928a350dc1bdbcf89f5a
Signed-off-by: Alexandre Montplaisir <alexmonthy@efficios.com>
Reviewed-on: https://git.eclipse.org/r/72242
Reviewed-by: Hudson CI
Reviewed-by: Bernd Hufmann <bernd.hufmann@ericsson.com>
Tested-by: Bernd Hufmann <bernd.hufmann@ericsson.com>
Reviewed-by: Matthew Khouzam <matthew.khouzam@ericsson.com>
analysis/org.eclipse.tracecompass.analysis.lami.core.tests/src/org/eclipse/tracecompass/analysis/lami/core/tests/LamiAnalysisStub.java
analysis/org.eclipse.tracecompass.analysis.lami.core.tests/src/org/eclipse/tracecompass/analysis/lami/core/tests/LamiJsonParserTest.java
analysis/org.eclipse.tracecompass.analysis.lami.core/src/org/eclipse/tracecompass/internal/provisional/analysis/lami/core/module/LamiAnalysis.java
analysis/org.eclipse.tracecompass.analysis.lami.core/src/org/eclipse/tracecompass/internal/provisional/analysis/lami/core/module/Messages.java
analysis/org.eclipse.tracecompass.analysis.lami.core/src/org/eclipse/tracecompass/internal/provisional/analysis/lami/core/module/messages.properties
analysis/org.eclipse.tracecompass.analysis.lami.ui/src/org/eclipse/tracecompass/internal/provisional/analysis/lami/ui/handler/Messages.java
analysis/org.eclipse.tracecompass.analysis.lami.ui/src/org/eclipse/tracecompass/internal/provisional/analysis/lami/ui/handler/RunAnalysisHandler.java
analysis/org.eclipse.tracecompass.analysis.lami.ui/src/org/eclipse/tracecompass/internal/provisional/analysis/lami/ui/handler/messages.properties
tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/analysis/ondemand/IOnDemandAnalysis.java
tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/analysis/ondemand/OnDemandAnalysisException.java [deleted file]

index 18aa5dcaef26449d292a381a6e9a7e074dacc403..490b0bab777f81f790840cc738286691ae160328 100644 (file)
@@ -20,12 +20,12 @@ import java.util.Collections;
 import java.util.List;
 import java.util.stream.Collectors;
 
+import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
 import org.eclipse.tracecompass.internal.provisional.analysis.lami.core.module.LamiAnalysis;
 import org.eclipse.tracecompass.internal.provisional.analysis.lami.core.module.LamiChartModel;
-import org.eclipse.tracecompass.tmf.core.analysis.ondemand.OnDemandAnalysisException;
 import org.eclipse.tracecompass.tmf.core.trace.ITmfTrace;
 
 import com.google.common.collect.ImmutableMultimap;
@@ -65,8 +65,7 @@ public class LamiAnalysisStub extends LamiAnalysis {
 
     @Override
     protected String getResultsFromCommand(List<String> command, IProgressMonitor monitor)
-            throws OnDemandAnalysisException {
-
+            throws CoreException {
         return readLamiFile(fResultFilename);
     }
 
index 6f6acc5cab58bb00ad7ede774d07f0d264db8876..0f50cd5e281db83e0a43f75f8ba3e51185425cc2 100644 (file)
@@ -19,6 +19,7 @@ import static org.junit.Assert.assertTrue;
 import java.util.List;
 import java.util.Map;
 
+import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.NullProgressMonitor;
 import org.eclipse.tracecompass.internal.provisional.analysis.lami.core.aspect.LamiTableEntryAspect;
 import org.eclipse.tracecompass.internal.provisional.analysis.lami.core.module.LamiResultTable;
@@ -30,7 +31,6 @@ import org.eclipse.tracecompass.internal.provisional.analysis.lami.core.types.La
 import org.eclipse.tracecompass.internal.provisional.analysis.lami.core.types.LamiSize;
 import org.eclipse.tracecompass.internal.provisional.analysis.lami.core.types.LamiSystemCall;
 import org.eclipse.tracecompass.internal.provisional.analysis.lami.core.types.LamiTimeRange;
-import org.eclipse.tracecompass.tmf.core.analysis.ondemand.OnDemandAnalysisException;
 import org.eclipse.tracecompass.tmf.core.timestamp.ITmfTimestamp;
 import org.eclipse.tracecompass.tmf.core.timestamp.TmfTimeRange;
 import org.eclipse.tracecompass.tmf.core.timestamp.TmfTimestamp;
@@ -124,10 +124,10 @@ public class LamiJsonParserTest {
     /**
      * Test the results parsing.
      *
-     * @throws OnDemandAnalysisException when execute() fails.
+     * @throws CoreException when execute() fails.
      */
     @Test
-    public void testResults() throws OnDemandAnalysisException {
+    public void testResults() throws CoreException {
         LamiAnalysisStub analysis = new LamiAnalysisStub("test-metadata.json", "test-results.json");
 
         List<LamiResultTable> resultTables = analysis.execute(fTrace, null, "", new NullProgressMonitor());
@@ -213,10 +213,10 @@ public class LamiJsonParserTest {
     /**
      * Test the error parsing of the results.
      *
-     * @throws OnDemandAnalysisException when execute() fails.
+     * @throws CoreException when execute() fails.
      */
-    @Test (expected = OnDemandAnalysisException.class)
-    public void testResultsError() throws OnDemandAnalysisException {
+    @Test (expected = CoreException.class)
+    public void testResultsError() throws CoreException {
         LamiAnalysisStub analysis = new LamiAnalysisStub("test-metadata.json", "test-error.json");
 
         analysis.execute(fTrace, null, "", new NullProgressMonitor());
index 5cd62122c629a7dced5b3b8f1cc395d8698f1775..200c4e15fc1981d86f633660beb112aca938e7af 100644 (file)
@@ -30,7 +30,11 @@ import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.MultiStatus;
+import org.eclipse.core.runtime.Status;
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
 import org.eclipse.tracecompass.internal.analysis.lami.core.Activator;
@@ -54,7 +58,6 @@ import org.eclipse.tracecompass.internal.provisional.analysis.lami.core.types.La
 import org.eclipse.tracecompass.internal.provisional.analysis.lami.core.types.LamiData.DataType;
 import org.eclipse.tracecompass.internal.provisional.analysis.lami.core.types.LamiTimeRange;
 import org.eclipse.tracecompass.tmf.core.analysis.ondemand.IOnDemandAnalysis;
-import org.eclipse.tracecompass.tmf.core.analysis.ondemand.OnDemandAnalysisException;
 import org.eclipse.tracecompass.tmf.core.timestamp.TmfTimeRange;
 import org.eclipse.tracecompass.tmf.core.trace.ITmfTrace;
 import org.json.JSONArray;
@@ -487,12 +490,12 @@ public class LamiAnalysis implements IOnDemandAnalysis {
      * @param monitor
      *            The progress monitor used to report progress
      * @return The script's output, formatted into {@link LamiTableEntry}'s.
-     * @throws OnDemandAnalysisException
+     * @throws CoreException
      *             If execution did not terminate normally
      */
     @Override
     public List<LamiResultTable> execute(ITmfTrace trace, @Nullable TmfTimeRange timeRange,
-            String extraParams, IProgressMonitor monitor) throws OnDemandAnalysisException {
+            String extraParams, IProgressMonitor monitor) throws CoreException {
         /* Should have been called already, but in case it was not */
         initialize();
 
@@ -511,7 +514,8 @@ public class LamiAnalysis implements IOnDemandAnalysis {
         String output = getResultsFromCommand(command, monitor);
 
         if (output.isEmpty()) {
-            throw new OnDemandAnalysisException(Messages.LamiAnalysis_NoResults);
+            IStatus status = new Status(IStatus.INFO, Activator.instance().getPluginId(), Messages.LamiAnalysis_NoResults);
+            throw new CoreException(status);
         }
 
         /*
@@ -585,7 +589,8 @@ public class LamiAnalysis implements IOnDemandAnalysis {
                  * No results were reported. This may be normal, but warn the
                  * user why a report won't be created.
                  */
-                throw new OnDemandAnalysisException(Messages.LamiAnalysis_NoResults);
+                IStatus status = new Status(IStatus.INFO, Activator.instance().getPluginId(), Messages.LamiAnalysis_NoResults);
+                throw new CoreException(status);
             }
 
             for (int i = 0; i < results.length(); i++) {
@@ -652,7 +657,8 @@ public class LamiAnalysis implements IOnDemandAnalysis {
             }
 
         } catch (JSONException e) {
-            throw new OnDemandAnalysisException(e.getMessage());
+            IStatus status = new Status(IStatus.ERROR, Activator.instance().getPluginId(), e.getMessage(), e);
+            throw new CoreException(status);
         }
 
         return resultsBuilder.build();
@@ -707,13 +713,13 @@ public class LamiAnalysis implements IOnDemandAnalysis {
      * @param monitor
      *            The progress monitor
      * @return The analysis results
-     * @throws OnDemandAnalysisException
+     * @throws CoreException
      *             If the command ended abnormally, and normal results were not
      *             returned
      */
     @VisibleForTesting
     protected String getResultsFromCommand(List<String> command, IProgressMonitor monitor)
-            throws OnDemandAnalysisException {
+            throws CoreException {
 
         final int scale = 1000;
         double workedSoFar = 0.0;
@@ -792,7 +798,8 @@ public class LamiAnalysis implements IOnDemandAnalysis {
 
             if (monitor.isCanceled()) {
                 /* We were interrupted by the canceller thread. */
-                throw new OnDemandAnalysisException(null);
+                IStatus status = new Status(IStatus.CANCEL, Activator.instance().getPluginId(), null);
+                throw new CoreException(status);
             }
 
             if (ret != 0) {
@@ -801,8 +808,21 @@ public class LamiAnalysis implements IOnDemandAnalysis {
                  * gather the stderr and report it to the user.
                  */
                 BufferedReader br = new BufferedReader(new InputStreamReader(p.getErrorStream()));
-                String stdErrOutput = br.lines().collect(Collectors.joining("\n")); //$NON-NLS-1$
-                throw new OnDemandAnalysisException(stdErrOutput);
+                List<String> stdErrOutput = br.lines().collect(Collectors.toList());
+
+                MultiStatus status = new MultiStatus(Activator.instance().getPluginId(),
+                        IStatus.ERROR, Messages.LamiAnalysis_ErrorDuringExecution, null);
+                for (String str : stdErrOutput) {
+                    status.add(new Status(IStatus.ERROR, Activator.instance().getPluginId(), str));
+                }
+                if (stdErrOutput.isEmpty()) {
+                    /*
+                     * At least say "no output", so an error message actually
+                     * shows up.
+                     */
+                    status.add(new Status(IStatus.ERROR, Activator.instance().getPluginId(), Messages.LamiAnalysis_ErrorNoOutput));
+                }
+                throw new CoreException(status);
             }
 
             /* External script ended successfully, all is fine! */
@@ -810,7 +830,8 @@ public class LamiAnalysis implements IOnDemandAnalysis {
             return checkNotNull(resultsStr);
 
         } catch (IOException | InterruptedException e) {
-            throw new OnDemandAnalysisException(Messages.LamiAnalysis_ExecutionInterrupted);
+            IStatus status = new Status(IStatus.ERROR, Activator.instance().getPluginId(), Messages.LamiAnalysis_ExecutionInterrupted, e);
+            throw new CoreException(status);
 
         } finally {
             if (cancellerRunnable != null) {
index 1ca275ae416d814fcb844545e4f7af52104c7a44..8909b02c02616b163bf9c649d1777a9d6a02044e 100644 (file)
@@ -27,6 +27,8 @@ public class Messages extends NLS {
 
     public static String LamiAnalysis_MainTaskName;
     public static String LamiAnalysis_NoResults;
+    public static String LamiAnalysis_ErrorDuringExecution;
+    public static String LamiAnalysis_ErrorNoOutput;
     public static String LamiAnalysis_ExecutionInterrupted;
 
     public static String LamiAnalysis_ExtendedTableNamePrefix;
index c91b0a749138407f57dd2627e6da523bc5cbbad8..2b04f445a7d33b1339845f06f08e1e0eae10a015 100644 (file)
@@ -11,6 +11,8 @@ LamiAnalysis_DefaultDynamicTableName = Dynamic Table
 
 LamiAnalysis_MainTaskName = Invoking external analysis script
 LamiAnalysis_NoResults = No results were returned.
+LamiAnalysis_ErrorDuringExecution = Error during execution of the script.
+LamiAnalysis_ErrorNoOutput = (No output)
 LamiAnalysis_ExecutionInterrupted = Execution was interrupted.
 
 LamiAnalysis_ExtendedTableNamePrefix = Extended
index d1811eb7c38329a2cf6f9fceb07b95731d5992ef..58456de24942e43c7d09d810e18071e866a9320e 100644 (file)
@@ -30,8 +30,12 @@ public class Messages extends NLS {
     public static String ParameterDialog_ExternalParametersDescription;
     public static String ParameterDialog_StringValidatorMessage;
     public static String ParameterDialog_ReportNameSuffix;
-    public static String ParameterDialog_Error;
-    public static String ParameterDialog_ErrorMessage;
+
+    public static String ErrorDialog_Info;
+    public static String ErrorDialog_InfoMessage;
+    public static String ErrorDialog_Error;
+    public static String ErrorDialog_ErrorMessage;
+
     public static String AddAnalysisDialog_Name;
     public static String AddAnalysisDialog_Command;
     public static String AddAnalysisDialog_NameEmptyErrorMessage;
index e99c7cd605a53320b41da75e5f06f88cfef42dda..14d19edc4bc3222f71f68c23ebc2846a8ccb2476 100644 (file)
@@ -16,14 +16,15 @@ import java.util.List;
 import org.eclipse.core.commands.AbstractHandler;
 import org.eclipse.core.commands.ExecutionEvent;
 import org.eclipse.core.commands.ExecutionException;
+import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.NullProgressMonitor;
 import org.eclipse.core.runtime.Status;
 import org.eclipse.core.runtime.jobs.Job;
 import org.eclipse.jdt.annotation.Nullable;
+import org.eclipse.jface.dialogs.ErrorDialog;
 import org.eclipse.jface.dialogs.IInputValidator;
-import org.eclipse.jface.dialogs.MessageDialog;
 import org.eclipse.jface.viewers.ISelection;
 import org.eclipse.jface.viewers.IStructuredSelection;
 import org.eclipse.jface.window.Window;
@@ -35,7 +36,6 @@ import org.eclipse.tracecompass.internal.provisional.analysis.lami.core.module.L
 import org.eclipse.tracecompass.internal.provisional.analysis.lami.ui.views.LamiReportViewFactory;
 import org.eclipse.tracecompass.tmf.core.analysis.ondemand.IOnDemandAnalysis;
 import org.eclipse.tracecompass.tmf.core.analysis.ondemand.IOnDemandAnalysisReport;
-import org.eclipse.tracecompass.tmf.core.analysis.ondemand.OnDemandAnalysisException;
 import org.eclipse.tracecompass.tmf.core.timestamp.TmfTimeRange;
 import org.eclipse.tracecompass.tmf.core.trace.ITmfTrace;
 import org.eclipse.tracecompass.tmf.core.trace.TmfTraceManager;
@@ -137,22 +137,37 @@ public class RunAnalysisHandler extends AbstractHandler {
                     });
                     return Status.OK_STATUS;
 
-                } catch (OnDemandAnalysisException e) {
-                    String errMsg = e.getMessage();
-
-                    if (errMsg != null) {
-                        /* The analysis execution yielded an error */
-                        Display.getDefault().asyncExec(() -> {
-                            MessageDialog.openError(shell,
-                                    /* Dialog title */
-                                    Messages.ParameterDialog_Error,
-                                    /* Dialog message */
-                                    Messages.ParameterDialog_ErrorMessage + ":\n\n" + //$NON-NLS-1$
-                                            errMsg);
-                        });
+                } catch (CoreException e) {
+                    /*
+                     * The analysis execution did not complete normally, we will
+                     * report it to the user.
+                     */
+                    IStatus status = e.getStatus();
+
+                    /* Don't display a dialog if it was simply cancelled by the user */
+                    if (status.matches(IStatus.CANCEL)) {
+                        return status;
+                    }
+
+                    String dialogTitle;
+                    String dialogMessage;
+                    if (status.matches(IStatus.ERROR)) {
+                        dialogTitle = Messages.ErrorDialog_Error;
+                        dialogMessage = Messages.ErrorDialog_ErrorMessage;
+                    } else {
+                        dialogTitle = Messages.ErrorDialog_Info;
+                        dialogMessage = Messages.ErrorDialog_InfoMessage;
                     }
 
-                    return Status.CANCEL_STATUS;
+                    Display.getDefault().asyncExec(() -> {
+                        ErrorDialog.openError(shell, dialogTitle, dialogMessage, status);
+                    });
+
+                    /*
+                     * We showed our own error message, no need for the Job to
+                     * show another one.
+                     */
+                    return Status.OK_STATUS;
                 }
             }
         };
index 2af7ae71951c7c5a189a4007f9e63afeb97e449b..94cdb8b6fea2952deb903de70388f51649e8ce17 100644 (file)
@@ -14,8 +14,11 @@ ParameterDialog_ExternalParameters = External Analysis Parameters
 ParameterDialog_ExternalParametersDescription = Extra parameters to pass to the external analysis. Leave empty for none.
 ParameterDialog_StringValidatorMessage = Allowed characters are letters, numbers, ',' and '-'
 ParameterDialog_ReportNameSuffix = Report
-ParameterDialog_Error = Error running external script
-ParameterDialog_ErrorMessage = The script terminated abnormally
+
+ErrorDialog_Info = External script completed
+ErrorDialog_InfoMessage = The script finished executing.
+ErrorDialog_Error = Error running external script
+ErrorDialog_ErrorMessage = The script terminated abnormally.
 
 AddAnalysisDialog_Title = Add External Analysis
 AddAnalysisDialog_Name = Name
index 2d21f1d4e1c591caeb3ed5cc22b269d24f3601d9..24643f49386e99253885a75170e4debd3e32586c 100644 (file)
@@ -9,6 +9,7 @@
 
 package org.eclipse.tracecompass.tmf.core.analysis.ondemand;
 
+import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.tracecompass.tmf.core.timestamp.TmfTimeRange;
 import org.eclipse.tracecompass.tmf.core.trace.ITmfTrace;
@@ -93,10 +94,10 @@ public interface IOnDemandAnalysis {
      *            for a default monitor.
      * @return The results of this analysis. Exact object type is
      *         analysis-dependent, a more specific return type is encouraged.
-     * @throws OnDemandAnalysisException
+     * @throws CoreException
      *             If something went wrong with the execution, and expected
      *             results will not be returned
      */
     Object execute(ITmfTrace trace, TmfTimeRange range, String extraParams,
-            IProgressMonitor monitor) throws OnDemandAnalysisException;
+            IProgressMonitor monitor) throws CoreException;
 }
diff --git a/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/analysis/ondemand/OnDemandAnalysisException.java b/tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/analysis/ondemand/OnDemandAnalysisException.java
deleted file mode 100644 (file)
index 5abd3bb..0000000
+++ /dev/null
@@ -1,43 +0,0 @@
-/*******************************************************************************
- * Copyright (c) 2016 EfficiOS Inc., Alexandre Montplaisir
- *
- * All rights reserved. This program and the accompanying materials
- * are made available under the terms of the Eclipse Public License v1.0
- * which accompanies this distribution, and is available at
- * http://www.eclipse.org/legal/epl-v10.html
- *******************************************************************************/
-
-package org.eclipse.tracecompass.tmf.core.analysis.ondemand;
-
-import org.eclipse.jdt.annotation.Nullable;
-
-/**
- * Exceptions resulting from the execution of on-demand analyses.
- *
- * The suggested behavior is to display the reported messages to the end user,
- * so they know why execution did not end normally.
- *
- * @author Alexandre Montplaisir
- * @since 2.0
- */
-public class OnDemandAnalysisException extends Exception {
-
-    private static final long serialVersionUID = 7296987172562152876L;
-
-    /**
-     * Build a new exception. If the message is not null, it should be reported
-     * to the user.
-     *
-     * @param message
-     *            The message to display, if any
-     */
-    public OnDemandAnalysisException(@Nullable String message) {
-        super(message);
-    }
-
-    @Override
-    public @Nullable String getMessage() {
-        return super.getMessage();
-    }
-
-}
This page took 0.034033 seconds and 5 git commands to generate.