Skip to content

Commit

Permalink
improve error visibility
Browse files Browse the repository at this point in the history 
* `restoreId` was throwing a `ClassCastException`, which is not one that we were
handling in the try/catch. Instead of trying to catch all possible errors and
ignoring them, just add them to the feedback list.
* Add an explicit note to the user that continuing with the project may cause
data loss and attention is needed. It would probably be better to have a popup
dialog because of how serious this is, but I don't want to stick a dialog deep
in the stack here. I will probably revisit this and place a dialog elsewhere,
after passing up a signal that the project appears corrupted.
* Change many error feedback messages to be shown with the error styling
  • Loading branch information
@garfieldnate
garfieldnate committed Sep 6, 2024
1 parent 014c6ab commit c109256
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 30 deletions.
11 changes: 11 additions & 0 deletions src/main/java/edu/umich/soar/visualsoar/mainframe/MainFrame.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1429,19 +1429,30 @@ public void openProject(@NotNull File vsaFile, boolean readOnly) {
} catch (FileNotFoundException fnfe) {
JOptionPane.showMessageDialog(
this, fnfe.getMessage(), "File Not Found", JOptionPane.ERROR_MESSAGE);
getFeedbackManager().setStatusBarError("Failed to open " + vsaFile);
} catch (IOException ioe) {
getFeedbackManager().setStatusBarError("Failed to open " + vsaFile);
JOptionPane.showMessageDialog(
this, ioe.getMessage(), "I/O Exception", JOptionPane.ERROR_MESSAGE);
ioe.printStackTrace();
} catch (NumberFormatException nfe) {
// TODO: find where this is getting thrown and change it to a (possibly custom) checked
// exception
nfe.printStackTrace();
getFeedbackManager().setStatusBarError("Failed to open " + vsaFile);
JOptionPane.showMessageDialog(
this,
"Error Reading File, Data Incorrectly Formatted",
"Bad File",
JOptionPane.ERROR_MESSAGE);
} catch (Throwable e) {
e.printStackTrace();
getFeedbackManager().setStatusBarError("Failed to open " + vsaFile);
JOptionPane.showMessageDialog(
this,
"Error: Failed to read project due to unknown error",
"Bad File",
JOptionPane.ERROR_MESSAGE);
}
}

Expand Down
19 changes: 15 additions & 4 deletions src/main/java/edu/umich/soar/visualsoar/mainframe/feedback/FeedbackManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import javax.swing.*;
import java.awt.*;
import java.util.Collection;
import java.util.List;
import java.util.Vector;

Expand All @@ -30,8 +31,11 @@ private AtomicContext() {
public void close() {
if (!inAtomicFeedback) {
System.err.println(
"WARNING: AtomicFeedbackManager closed after MainFrame "
+ "had already exited atomic feedback mode");
"WARNING: "
+ getClass().getName()
+ " closed after "
+ FeedbackManager.this.getClass().getName()
+ " had already exited atomic feedback mode");
}
}
}
Expand All @@ -54,14 +58,21 @@ public AtomicContext beginAtomicContext() {
* @param v the vector list of feedback data; {@code null} either clears or doesn't modify the list.
*/
public void showFeedback(
@NotNull Vector<FeedbackListEntry> v) {
@NotNull Collection<? extends FeedbackListEntry> v) {
if (inAtomicFeedback) {
feedbackList.appendListData(v);
} else {
feedbackList.setListData(v);
feedbackList.setListData(new Vector<>(v));
}
}

/**
* @see #showFeedback(Collection)
*/
public void showFeedback(@NotNull FeedbackListEntry e) {
showFeedback(List.of(e));
}

public void clearFeedback() {
feedbackList.clearListData();
}
Expand Down
48 changes: 22 additions & 26 deletions src/main/java/edu/umich/soar/visualsoar/operatorwindow/OperatorWindow.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import edu.umich.soar.visualsoar.graph.SoarVertex;
import edu.umich.soar.visualsoar.mainframe.feedback.FeedbackEntryOpNode;
import edu.umich.soar.visualsoar.mainframe.feedback.FeedbackListEntry;
import edu.umich.soar.visualsoar.mainframe.feedback.FeedbackManager;
import edu.umich.soar.visualsoar.misc.Prefs;
import edu.umich.soar.visualsoar.misc.Template;
import edu.umich.soar.visualsoar.parser.ParseException;
Expand Down Expand Up @@ -1201,16 +1202,7 @@ private void openVersionFour(FileReader fr, String parentPath) throws IOExceptio
MainFrame.getMainFrame().getFeedbackManager().setStatusBarError("Unable to parse " + dataMapFile.getName());
}

try {
restoreStateIds();
}
catch(ArrayIndexOutOfBoundsException aioobe) {
//if the parse was unsuccessful don't say anything since there are
// already more informative error messages in the feedback window
if (success) {
MainFrame.getMainFrame().getFeedbackManager().setStatusBarError("Error! Unable to restore high-level ids for project.");
}
}
restoreStateIds();
}

/**
Expand Down Expand Up @@ -1256,7 +1248,8 @@ private void openVersionFive(FileReader fr, String parentPath) throws IOExceptio


/**
* This is a helper function restores the ids to high-level operators
* The VSA file contains operators and their DM ID numbers. This helper connects the Soar IDs loaded from the datamap
* to the high-level operator nodes loaded from the VSA file.
*/
private void restoreStateIds() {
Enumeration<TreeNode> nodeEnum = ((OperatorRootNode) getModel().getRoot()).breadthFirstEnumeration();
Expand Down Expand Up @@ -1930,7 +1923,7 @@ private String[] integrityCheckV4(Vector<String> lines, Vector<FeedbackListEntry
//The last line should be 'END'.
String lastLine = lines.lastElement().trim();
if (! lastLine.equals("END")) {
errors.add(new FeedbackListEntry("[line " + lines.size() + "] Project file (.vsa) is truncated. Some operator nodes may be missing."));
errors.add(new FeedbackListEntry("[line " + lines.size() + "] Project file (.vsa) is truncated. Some operator nodes may be missing.", true));
}
lines.remove(lines.size() - 1); //remove "END" so remaining lines are consistent

Expand All @@ -1940,7 +1933,7 @@ private String[] integrityCheckV4(Vector<String> lines, Vector<FeedbackListEntry
for(int i = 0; i < lines.size(); ++i) {
String line = lines.get(i).trim();
if (line.length() == 0) {
errors.add(new FeedbackListEntry("Error on line " + (i+skipped+1) + " of .vsa file: illegal blank line ignored."));
errors.add(new FeedbackListEntry("Error on line " + (i+skipped+1) + " of .vsa file: illegal blank line ignored.", true));
skipped++;
continue;
}
Expand All @@ -1961,7 +1954,7 @@ private String[] integrityCheckV4(Vector<String> lines, Vector<FeedbackListEntry
String err = "Root node has improper format.";
err += " Expecting '0\\tROOT <name> <name> 1'";
err += " Received '" + nodeLines[0] + "' instead.";
errors.add(new FeedbackListEntry(err));
errors.add(new FeedbackListEntry(err, true));

//Replace this node with something in the correct format
String projName = findIdentifier(nodeLines[0]);
Expand Down Expand Up @@ -2009,7 +2002,11 @@ private void readVersionFourSafe(Reader r) {

//Report errors
if (errors.size() > 0) {
MainFrame.getMainFrame().getFeedbackManager().showFeedback(errors);
FeedbackManager fb = MainFrame.getMainFrame().getFeedbackManager();
fb.showFeedback(errors);
fb.showFeedback(new FeedbackListEntry("TAKE NOTE: Saving this project may result in data loss. " +
"If you think you can fix the above errors yourself, please fix them and re-open the project.",
true));

//TODO: At this point we could present the user with these choices:
// a) abort the project load operation
Expand Down Expand Up @@ -2271,13 +2268,13 @@ private boolean verifyV4OperatorLine(String line, int maxOpId, Vector<FeedbackLi
//The line should contain at least 6 words
String[] words = line.split("[ \\t]"); //split on spaces and tabs
if (words.length < 6) {
errors.add(new FeedbackListEntry("Incomplete operator node line found: " + line));
errors.add(new FeedbackListEntry("Incomplete operator node line found: " + line, true));
return false;
}
//High level operators need 8 words
boolean isHLOperator = words[2].startsWith("HL");
if ( isHLOperator && (words.length < 8) ) {
errors.add(new FeedbackListEntry("Incomplete high-level operator node line found in .vsa file: " + line));
errors.add(new FeedbackListEntry("Incomplete high-level operator node line found in .vsa file: " + line, true));
return false;
}

Expand All @@ -2287,7 +2284,7 @@ private boolean verifyV4OperatorLine(String line, int maxOpId, Vector<FeedbackLi
Integer.parseInt(words[0]);
}
catch(NumberFormatException nfe) {
errors.add(new FeedbackListEntry("Error! Operator node line has invalid id number: " + line));
errors.add(new FeedbackListEntry("Error! Operator node line has invalid id number: " + line, true));
return false;
}

Expand All @@ -2297,11 +2294,11 @@ private boolean verifyV4OperatorLine(String line, int maxOpId, Vector<FeedbackLi
try {
parentId = Integer.parseInt(words[1]);
} catch (NumberFormatException nfe) {
errors.add(new FeedbackListEntry("Error! Operator node line has invalid parent id number: " + line));
errors.add(new FeedbackListEntry("Error! Operator node line has invalid parent id number: " + line, true));
return false;
}
if ( (parentId < 0) || (parentId > maxOpId)) {
errors.add(new FeedbackListEntry("Error: Operator node line (\"" + line + "\") has an invalid parent id number: " + parentId));
errors.add(new FeedbackListEntry("Error: Operator node line (\"" + line + "\") has an invalid parent id number: " + parentId, true));
return false;
}

Expand All @@ -2318,26 +2315,26 @@ private boolean verifyV4OperatorLine(String line, int maxOpId, Vector<FeedbackLi
}
}
if (!isValid) {
errors.add(new FeedbackListEntry("Error! Operator node line has unknown type: " + line));
errors.add(new FeedbackListEntry("Error! Operator node line has unknown type: " + line, true));
return false;
}

//Check for extraneous ROOT
if (words[2].equals("ROOT")) {
errors.add(new FeedbackListEntry("Error! Non-root operator node line has ROOT type: " + line));
errors.add(new FeedbackListEntry("Error! Non-root operator node line has ROOT type: " + line, true));
return false;
}

//Check for types that aren't used in V4
if ( (words[2].equals("LINK")) || (words[2].equals("FILE")) ) {
errors.add(new FeedbackListEntry("Error!: Operator node line has obsolete type: " + line));
errors.add(new FeedbackListEntry("Error!: Operator node line has obsolete type: " + line, true));
return false;
}

//--------------------------------------------------------------------
//3. Node Name
if (! operatorNameIsValid(words[3])) {
errors.add(new FeedbackListEntry("Error!: Operator node line has invalid name: " + line));
errors.add(new FeedbackListEntry("Error!: Operator node line has invalid name: " + line, true));
return false;
}

Expand Down Expand Up @@ -2371,15 +2368,14 @@ private boolean verifyV4OperatorLine(String line, int maxOpId, Vector<FeedbackLi
//For high-level operators verify that an integer datamap id is present.
//The value of this id can not be checked with the datamap since the
// datamap has not yet been loaded. So, we
// The value of this id can not be checked with the datamap since the datamap has not yet been loaded. So, we
//just verify it is a positive number
if (words[2].startsWith("HL")) {
try {
int datamapId = Integer.parseInt(words[7]);
if (datamapId < 1) throw new NumberFormatException();
}
catch(NumberFormatException nfe) {
errors.add(new FeedbackListEntry("Error: Operator node line is missing a valid datamap root id: " + line));
errors.add(new FeedbackListEntry("Error: Operator node line is missing a valid datamap root id: " + line, true));
return false;
}
}
Expand Down

0 comments on commit c109256

Please sign in to comment.