Fix silent error in JSON/XML importers (#2414)

* Add error handler for parse error

* Add test for parsing json with incorrect strecture

* Enable localization from front-end

* Add methods to get localized error messages

* Update returned exception message

* Remove unused log and fix file diff issue

* Test auto build

* Refactor getOptions in newly created test

* Use new exception to unwrap original message

* Undo unexpected fix

* Remove unused lines

* Fix exception logic

* Fix typo

main/src/com/google/refine/importers/JsonImporter.java

@@ -357,6 +357,8 @@ public class JsonImporter extends TreeImportingParserBase {
                     }
                 }
                 next = parser.nextToken();
+            } catch (JsonParseException e) {
+                throw new TreeReaderException(e.getOriginalMessage());
             } catch (IOException e) {
                 throw new TreeReaderException(e);
             }

main/src/com/google/refine/importers/tree/TreeImportingParserBase.java

@@ -40,6 +40,7 @@ import java.io.Reader;
 import java.util.List;
 
 import org.apache.commons.lang.NotImplementedException;
+import com.google.refine.importers.tree.TreeReaderException;
 
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.google.refine.ProjectMetadata;
@@ -220,8 +221,12 @@ abstract public class TreeImportingParserBase extends ImportingParserBase {
             assert filenameColumnIndex == 0;
         }
         
-        XmlImportUtilities.importTreeData(treeParser, project, recordPath, rootColumnGroup, limit2, 
-                new ImportParameters(trimStrings, storeEmptyStrings, guessCellValueTypes, includeFileSources,
-                        fileSource));
+        try {
+            XmlImportUtilities.importTreeData(treeParser, project, recordPath, rootColumnGroup, limit2,
+                    new ImportParameters(trimStrings, storeEmptyStrings, guessCellValueTypes, includeFileSources,
+                            fileSource));
+        } catch (Exception e){
+            exceptions.add(e);
+        }
     }
 }

main/src/com/google/refine/importers/tree/XmlImportUtilities.java

@@ -47,6 +47,8 @@ import javax.servlet.ServletException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.fasterxml.jackson.core.JsonParseException;
+
 import com.google.refine.importers.tree.TreeReader.Token;
 import com.google.refine.model.Cell;
 import com.google.refine.model.Project;
@@ -252,7 +254,7 @@ public class XmlImportUtilities extends TreeImportUtilities {
         ImportColumnGroup rootColumnGroup,
         int limit,
         ImportParameters parameters
-    ) {
+    ) throws TreeReaderException{
         if (logger.isTraceEnabled()) {
             logger.trace("importTreeData(TreeReader, Project, String[], ImportColumnGroup)");
         }
@@ -264,8 +266,8 @@ public class XmlImportUtilities extends TreeImportUtilities {
                 }
             }
         } catch (TreeReaderException e) {
-            // TODO: This error needs to be reported to the browser/user
             logger.error("Exception from XML parse",e);
+            throw e;
         }
     }
 

main/src/com/google/refine/importing/DefaultImportingController.java

@@ -47,6 +47,7 @@ import javax.servlet.http.HttpServletResponse;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.core.JsonParseException;
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.google.refine.RefineServlet;

main/tests/server/src/com/google/refine/importers/JsonImporterTests.java

@@ -37,7 +37,9 @@ import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.UnsupportedEncodingException;
+import java.util.ArrayList;
 import java.util.LinkedList;
+import java.util.List;
 
 import org.slf4j.LoggerFactory;
 import org.testng.Assert;
@@ -57,6 +59,8 @@ import com.google.refine.model.Row;
 import com.google.refine.util.JSONUtilities;
 import com.google.refine.util.ParsingUtilities;
 
+import com.google.refine.importers.tree.ImportColumnGroup;
+
 public class JsonImporterTests extends ImporterTest {
     @Override
     @BeforeTest
@@ -106,6 +110,43 @@ public class JsonImporterTests extends ImporterTest {
         Assert.assertEquals(row.getCell(1).value, "Author 1, The");
     }
 
+    @Test 
+    public void canThrowError(){
+        String errJSON = getSampleWithError();
+        ObjectNode options = SUT.createParserUIInitializationData(
+                job, new LinkedList<>(), "text/json");
+        ArrayNode path = ParsingUtilities.mapper.createArrayNode();
+        JSONUtilities.append(path, JsonImporter.ANONYMOUS);
+        JSONUtilities.safePut(options, "recordPath", path);
+        JSONUtilities.safePut(options, "trimStrings", false);
+        JSONUtilities.safePut(options, "storeEmptyStrings", true);
+        JSONUtilities.safePut(options, "guessCellValueTypes", false);
+
+        
+        try {
+            inputStream = new ByteArrayInputStream(errJSON.getBytes( "UTF-8" ) );
+        } catch (UnsupportedEncodingException e1) {
+            Assert.fail();
+        }
+        ImportColumnGroup rootColumnGroup = new ImportColumnGroup();
+        List<Exception> exceptions = new ArrayList<Exception>();
+
+        SUT.parseOneFile(
+                project,
+                metadata,
+                job,
+                "file-source",
+                inputStream,
+                rootColumnGroup,
+                -1,
+                options,
+                exceptions
+        );
+        Assert.assertFalse(exceptions.isEmpty());
+        Assert.assertEquals(exceptions.get(0).getMessage(), "Illegal unquoted " +
+                "character ((CTRL-CHAR, code 10)): has to be escaped using backslash to be included in string value");
+    }
+    
     @Test
     public void trimLeadingTrailingWhitespaceOnTrimStrings(){
         String ScraperwikiOutput = 
@@ -550,6 +591,13 @@ public class JsonImporterTests extends ImporterTest {
         return sb.toString();
     }
     
+    private static String getSampleWithError(){
+        StringBuilder sb = new StringBuilder();
+        sb.append("[");
+        sb.append("{\"id\":" + "\"\n\";");
+        sb.append("]");
+        return sb.toString();
+    }
 
     private void RunTest(String testString) {
         RunTest(testString, getOptions(job, SUT, JsonImporter.ANONYMOUS, false));

main/tests/server/src/com/google/refine/importers/XmlImportUtilitiesTests.java

@@ -209,9 +209,12 @@ public class XmlImportUtilitiesTests extends RefineTest {
         loadSampleXml();
 
         String[] recordPath = new String[]{"library","book"};
-        XmlImportUtilitiesStub.importTreeData(createXmlParser(), project, recordPath, columnGroup, -1, 
-                new ImportParameters(false, true, false));
-
+        try {
+            XmlImportUtilitiesStub.importTreeData(createXmlParser(), project, recordPath, columnGroup, -1,
+                    new ImportParameters(false, true, false));
+        } catch (Exception e){
+            Assert.fail();
+        }
         log(project);
         assertProjectCreated(project, 0, 6);
 
@@ -230,9 +233,12 @@ public class XmlImportUtilitiesTests extends RefineTest {
         loadData(XmlImporterTests.getSampleWithVaryingStructure());
 
         String[] recordPath = new String[]{"library", "book"};
-        XmlImportUtilitiesStub.importTreeData(createXmlParser(), project, recordPath, columnGroup, -1, 
-                new ImportParameters(false, true, false));
-
+        try {
+            XmlImportUtilitiesStub.importTreeData(createXmlParser(), project, recordPath, columnGroup, -1,
+                    new ImportParameters(false, true, false));
+        } catch (Exception e){
+            Assert.fail();
+        }
         log(project);
         assertProjectCreated(project, 0, 6);
         Assert.assertEquals(project.rows.get(0).cells.size(), 4);

main/webapp/modules/core/scripts/index/default-importing-controller/controller.js

@@ -371,4 +371,4 @@ Refine.TagsManager._getAllProjectTags = function() {
                  });
         }
     return self.allProjectTags;
-};
+};