Merge pull request #428 from ggalmazor/issue_401_sentry_sensitive_info

This is the smallest change that reduces sensitive info sent to Sentry in the short term.

The best possible solution would be to explicitly express our intent of sending error reports to Sentry. We need that to send to Sentry only actionable errors, avoiding to send errors related to the user's environment or issues with specific forms and submission files.

src/org/opendatakit/aggregate/parser/BaseFormParserForJavaRosa.java

@@ -123,12 +123,13 @@ private static final void initializeJavaRosa() {
 
   private static void redirectOutput() {
     File jrLogFile = new File(new StorageLocation().getBriefcaseFolder(), ".briefcase-javarosa.log");
+    log.info("Redirecting javarosa output to {}", jrLogFile);
     try {
       PrintStream jrOut = new PrintStream(jrLogFile);
       Std.setOut(jrOut);
       Std.setErr(jrOut);
     } catch (FileNotFoundException e) {
-      log.warn("failed to redirect javarosa output to " + jrLogFile);
+      log.warn("Failed to redirect javarosa output");
     }
   }
 

src/org/opendatakit/briefcase/model/XmlDocumentFetchException.java

@@ -17,13 +17,13 @@
 package org.opendatakit.briefcase.model;
 
 public class XmlDocumentFetchException extends Exception {
-
-  /**
-   * 
-   */
   private static final long serialVersionUID = -2163850446028219296L;
 
   public XmlDocumentFetchException(String message) {
     super(message);
   }
+
+  public XmlDocumentFetchException(String message, Throwable cause) {
+    super(message, cause);
+  }
 }

src/org/opendatakit/briefcase/operations/Common.java

@@ -36,19 +36,15 @@ static void bootCache(String storageDir) {
     BriefcasePreferences.setBriefcaseDirectoryProperty(storageDir);
     File f = new StorageLocation().getBriefcaseFolder();
 
+    LOGGER.info("Trying to use {} as storage directory", f);
     if (!f.exists()) {
-      boolean success = f.mkdirs();
-      if (success) {
-        LOGGER.info("Successfully created directory. Using: " + f.getAbsolutePath());
-      } else {
-        LOGGER.error("Unable to create directory: " + f.getAbsolutePath());
+      if (!f.mkdirs()) {
+        LOGGER.error("Unable to create directory");
         System.exit(1);
       }
     } else if (f.exists() && !f.isDirectory()) {
-      LOGGER.error("Not a directory.  " + f.getAbsolutePath());
+      LOGGER.error("Not a directory");
       System.exit(1);
-    } else if (f.exists() && f.isDirectory()) {
-      LOGGER.info("Directory found, using " + f.getAbsolutePath());
     }
 
     if (BriefcasePreferences.appScoped().getBriefcaseDirectoryOrNull() != null) {

src/org/opendatakit/briefcase/ui/MainBriefcaseWindow.java

@@ -129,42 +129,42 @@ public static void main(String[] args) {
 
       // required for all operations
       if (!cmd.hasOption(FORM_ID) || !cmd.hasOption(STORAGE_DIRECTORY)) {
-        log.error(FORM_ID + " and " + STORAGE_DIRECTORY + " are required");
+        System.err.println(FORM_ID + " and " + STORAGE_DIRECTORY + " are required");
         showHelp(options);
         System.exit(1);
       }
 
       // pull from collect or aggregate, not both
       if (cmd.hasOption(ODK_DIR) && cmd.hasOption(AGGREGATE_URL)) {
-        log.error("Can only have one of " + ODK_DIR + " or " + AGGREGATE_URL);
+        System.err.println("Can only have one of " + ODK_DIR + " or " + AGGREGATE_URL);
         showHelp(options);
         System.exit(1);
       }
 
       // pull from aggregate
       if (cmd.hasOption(AGGREGATE_URL) && (!(cmd.hasOption(ODK_USERNAME) && cmd.hasOption(ODK_PASSWORD)))) {
-        log.error(ODK_USERNAME + " and " + ODK_PASSWORD + " required when " + AGGREGATE_URL + " is specified");
+        System.err.println(ODK_USERNAME + " and " + ODK_PASSWORD + " required when " + AGGREGATE_URL + " is specified");
         showHelp(options);
         System.exit(1);
       }
 
       // export files
       if (cmd.hasOption(EXPORT_DIRECTORY) && !cmd.hasOption(EXPORT_FILENAME) || !cmd.hasOption(EXPORT_DIRECTORY) && cmd.hasOption(EXPORT_FILENAME)) {
-        log.error(EXPORT_DIRECTORY + " and " + EXPORT_FILENAME + " are both required to export");
+        System.err.println(EXPORT_DIRECTORY + " and " + EXPORT_FILENAME + " are both required to export");
         showHelp(options);
         System.exit(1);
       }
 
       if (cmd.hasOption(EXPORT_END_DATE)) {
         if (!testDateFormat(cmd.getOptionValue(EXPORT_END_DATE))) {
-          log.error("Invalid date format in " + EXPORT_END_DATE);
+          System.err.println("Invalid date format in " + EXPORT_END_DATE);
           showHelp(options);
           System.exit(1);
         }
       }
       if (cmd.hasOption(EXPORT_START_DATE)) {
         if (!testDateFormat(cmd.getOptionValue(EXPORT_START_DATE))) {
-          log.error("Invalid date format in " + EXPORT_START_DATE);
+          System.err.println("Invalid date format in " + EXPORT_START_DATE);
           showHelp(options);
           System.exit(1);
         }

src/org/opendatakit/briefcase/ui/StorageLocation.java

@@ -53,10 +53,11 @@ void establishBriefcaseStorageLocation(Window errorParentWindow, UiStateChangeLi
       if (dir.exists() && dir.isDirectory()) {
         if (BriefcaseFolderChooser.testAndMessageBadBriefcaseStorageLocationParentFolder(dir, errorParentWindow)) {
           try {
+            log.info("Trying {} as briefcase storage location", dir);
             assertBriefcaseStorageLocationParentFolder(dir);
             enableUi = true;
           } catch (FileSystemException e1) {
-            String msg = "Unable to create " + StorageLocation.BRIEFCASE_DIR;
+            String msg = "Unable to create briefcase storage location";
             log.error(msg, e1);
             ODKOptionPane.showErrorDialog(errorParentWindow, msg, "Failed to Create " + StorageLocation.BRIEFCASE_DIR);
           }

src/org/opendatakit/briefcase/util/AggregateUtils.java

@@ -67,8 +67,6 @@ public class AggregateUtils {
 
   private static final CharSequence HTTP_CONTENT_TYPE_APPLICATION_XML = "application/xml";
 
-  private static final String FETCH_FAILED_DETAILED_REASON = "Fetch of %1$s failed. Detailed reason: ";
-
   public static interface ResponseAction {
     void doAction(DocumentFetchResult result) throws MetadataUpdateException;
   }
@@ -97,19 +95,24 @@ public static class DocumentFetchResult {
    */
   public static final void commonDownloadFile(ServerConnectionInfo serverInfo, File f, String downloadUrl) throws URISyntaxException, IOException, TransmissionException {
 
+    log.info("Downloading URL {} into {}", downloadUrl, f);
+
     // OK. We need to download it because we either:
     // (1) don't have it
     // (2) don't know if it is changed because the hash is not md5
     // (3) know it is changed
     URI u = null;
     try {
+      log.info("Parsing URL {}", downloadUrl);
       URL uurl = new URL(downloadUrl);
       u = uurl.toURI();
     } catch (MalformedURLException | URISyntaxException e) {
-      log.error("bad download url " + downloadUrl, e);
+      log.warn("bad download url", e);
       throw e;
     }
 
+
+
     HttpClient httpclient = WebUtils.createHttpClient();
 
     // get shared HttpContext so that authentication and cookies are retained.
@@ -131,8 +134,7 @@ public static final void commonDownloadFile(ServerConnectionInfo serverInfo, Fil
         WebUtils.resetHttpContext();
         throw new TransmissionException("Authentication failure");
       } else if (statusCode != 200) {
-        String errMsg = String.format(FETCH_FAILED_DETAILED_REASON, f.getAbsolutePath())
-            + response.getStatusLine().getReasonPhrase() + " (" + statusCode + ")";
+        String errMsg = "Fetch failed. Detailed reason: " + response.getStatusLine().getReasonPhrase() + " (" + statusCode + ")";
         log.error(errMsg);
         flushEntityBytes(response.getEntity());
         throw new TransmissionException(errMsg);
@@ -165,20 +167,20 @@ public static final DocumentFetchResult getXmlDocument(String urlString,
                                                          DocumentDescription description, ResponseAction action)
       throws XmlDocumentFetchException {
 
+    log.info("Parsing URL {}", urlString);
     URI u = null;
     try {
       URL url = new URL(urlString);
       u = url.toURI();
     } catch (MalformedURLException e) {
-      String msg = description.getFetchDocFailed() + "Invalid url: " + urlString + ".\nFailed with error: " + e.getMessage();
+      String msg = description.getFetchDocFailed() + "Invalid url. Failed with error: " + e.getMessage();
       if (!urlString.toLowerCase().startsWith("http://") && !urlString.toLowerCase().startsWith("https://")) {
-        msg += "\nDid you forget to prefix the address with 'http://' or 'https://' ?";
+        msg += ". Did you forget to prefix the address with 'http://' or 'https://' ?";
       }
       log.warn(msg, e);
       throw new XmlDocumentFetchException(msg);
     } catch (URISyntaxException e) {
-      String msg = description.getFetchDocFailed() + "Invalid uri: " + urlString + ".\nFailed with error: "
-          + e.getMessage();
+      String msg = description.getFetchDocFailed() + "Invalid uri. Failed with error: " + e.getMessage();
       log.warn(msg, e);
       throw new XmlDocumentFetchException(msg);
     }
@@ -227,6 +229,7 @@ private static final DocumentFetchResult httpRetrieveXmlDocument(HttpUriRequest
     HttpClientContext localContext = WebUtils.getHttpContext();
 
     URI uri = request.getURI();
+    log.info("Attempting URI {}", uri);
 
     WebUtils.setCredentials(localContext, serverInfo, uri, alwaysResetCredentials);
 
@@ -273,16 +276,14 @@ private static final DocumentFetchResult httpRetrieveXmlDocument(HttpUriRequest
                   + "\nPlease verify that the URL, your user credentials and your permissions are all correct.");
         }
       } else if (entity == null) {
-        log.warn("No entity body returned from: " + uri.toString() + " is not text/xml");
+        log.warn("No entity body returned");
         ex = new XmlDocumentFetchException(description.getFetchDocFailed()
-            + " Server unexpectedly returned no content while accessing: " + uri.toString());
+            + " Server unexpectedly returned no content");
       } else if (!(lcContentType.contains(HTTP_CONTENT_TYPE_TEXT_XML) || lcContentType
           .contains(HTTP_CONTENT_TYPE_APPLICATION_XML))) {
-        log.warn("ContentType: " + entity.getContentType().getValue() + "returned from: "
-            + uri.toString() + " is not text/xml");
+        log.warn("Wrong ContentType: " + entity.getContentType().getValue() + "returned");
         ex = new XmlDocumentFetchException(description.getFetchDocFailed()
-            + "A non-XML document was returned while accessing: " + uri.toString()
-            + "\nA network login screen may be interfering with the transmission to the server.");
+            + "A non-XML document was returned. A network login screen may be interfering with the transmission to the server.");
       }
 
       if (ex != null) {
@@ -323,7 +324,7 @@ private static final DocumentFetchResult httpRetrieveXmlDocument(HttpUriRequest
         }
       } catch (Exception e) {
         log.warn("Parsing failed with " + e.getMessage(), e);
-        throw new XmlDocumentFetchException(description.getFetchDocFailed() + " while accessing: " + uri.toString());
+        throw new XmlDocumentFetchException(description.getFetchDocFailed());
       }
 
       // examine header fields...
@@ -358,6 +359,7 @@ private static final DocumentFetchResult httpRetrieveXmlDocument(HttpUriRequest
         try {
           URL url = new URL(locations[0].getValue());
           URI uNew = url.toURI();
+          log.info("Redirection to URI {}", uNew);
           if (uri.getHost().equalsIgnoreCase(uNew.getHost())) {
             // trust the server to tell us a new location
             // ... and possibly to use https instead.
@@ -367,8 +369,7 @@ private static final DocumentFetchResult httpRetrieveXmlDocument(HttpUriRequest
           } else {
             // Don't follow a redirection attempt to a different host.
             // We can't tell if this is a spoof or not.
-            String msg = description.getFetchDocFailed() + "Unexpected redirection attempt to a different host: "
-                + uNew.toString();
+            String msg = description.getFetchDocFailed() + "Unexpected redirection attempt";
             log.warn(msg);
             throw new XmlDocumentFetchException(msg);
           }
@@ -384,7 +385,7 @@ private static final DocumentFetchResult httpRetrieveXmlDocument(HttpUriRequest
       }
       return result;
     } catch (UnknownHostException e) {
-      String msg = description.getFetchDocFailed() + "Unknown host: " + e.getMessage();
+      String msg = description.getFetchDocFailed() + "Unknown host";
       log.warn(msg, e);
       throw new XmlDocumentFetchException(msg);
     } catch (IOException | MetadataUpdateException e) {
@@ -413,19 +414,20 @@ public static final URI testServerConnectionWithHeadRequest(ServerConnectionInfo
       urlString = urlString + "/" + actionAddr;
     }
 
+    log.info("Parsing URL {}", urlString);
     URI u;
     try {
       URL url = new URL(urlString);
       u = url.toURI();
     } catch (MalformedURLException e) {
-      String msg = "Invalid url: " + urlString + " for " + actionAddr + ".\nFailed with error: " + e.getMessage();
+      String msg = "Invalid url for " + actionAddr + ". Failed with error: " + e.getMessage();
       if (!urlString.toLowerCase().startsWith("http://") && !urlString.toLowerCase().startsWith("https://")) {
-        msg += "\nDid you forget to prefix the address with 'http://' or 'https://' ?";
+        msg += ". Did you forget to prefix the address with 'http://' or 'https://' ?";
       }
       log.warn(msg, e);
       throw new TransmissionException(msg);
     } catch (URISyntaxException e) {
-      String msg = "Invalid uri: " + urlString + " for " + actionAddr + ".\nFailed with error: " + e.getMessage();
+      String msg = "Invalid uri for " + actionAddr + ". Failed with error: " + e.getMessage();
       log.warn(msg, e);
       throw new TransmissionException(msg);
     }
@@ -453,8 +455,7 @@ public static final URI testServerConnectionWithHeadRequest(ServerConnectionInfo
         } else if (statusCode == 204) {
           Header[] openRosaVersions = response.getHeaders(WebUtils.OPEN_ROSA_VERSION_HEADER);
           if (openRosaVersions == null || openRosaVersions.length == 0) {
-            String msg = "Url: " + u.toString()
-                + ", header missing: " + WebUtils.OPEN_ROSA_VERSION_HEADER;
+            String msg = "Header missing: " + WebUtils.OPEN_ROSA_VERSION_HEADER;
             log.warn(msg);
             throw new TransmissionException(msg);
           }
@@ -463,6 +464,7 @@ public static final URI testServerConnectionWithHeadRequest(ServerConnectionInfo
             try {
               URL url = new URL(locations[0].getValue());
               URI uNew = url.toURI();
+              log.info("Redirection to URI {}", uNew);
               if (u.getHost().equalsIgnoreCase(uNew.getHost())) {
                 // trust the server to tell us a new location
                 // ... and possibly to use https instead.
@@ -475,19 +477,17 @@ public static final URI testServerConnectionWithHeadRequest(ServerConnectionInfo
               } else {
                 // Don't follow a redirection attempt to a different host.
                 // We can't tell if this is a spoof or not.
-                String msg = "Starting url: " + u.toString()
-                    + " unexpected redirection attempt to a different host: " + uNew.toString();
+                String msg = "Unexpected redirection attempt";
                 log.warn(msg);
                 throw new TransmissionException(msg);
               }
             } catch (Exception e) {
-              String msg = "Starting url: " + u + " unexpected exception: " + e.getLocalizedMessage();
+              String msg = "Unexpected exception: " + e.getLocalizedMessage();
               log.warn(msg, e);
               throw new TransmissionException(msg);
             }
           } else {
-            String msg = "The url: " + u.toString()
-                + " is not ODK Aggregate - status code on Head request: " + statusCode;
+            String msg = "The url is not ODK Aggregate - status code on Head request: " + statusCode;
             log.warn(msg);
             throw new TransmissionException(msg);
           }
@@ -506,13 +506,12 @@ public static final URI testServerConnectionWithHeadRequest(ServerConnectionInfo
               log.error("failed to process http stream", e);
             }
           }
-          String msg = "The username or password may be incorrect or the url: " + u.toString()
-              + " is not ODK Aggregate - status code on Head request: " + statusCode;
+          String msg = "The username or password may be incorrect or the url is not ODK Aggregate - status code on Head request: " + statusCode;
           log.warn(msg);
           throw new TransmissionException(msg);
         }
       } catch (Exception e) {
-        String msg = "Starting url: " + u.toString() + " unexpected exception: " + e.getLocalizedMessage();
+        String msg = "Unexpected exception: " + e.getLocalizedMessage();
         log.warn(msg, e);
         throw new TransmissionException(msg);
       }
@@ -557,6 +556,7 @@ public static final boolean uploadFilesToServer(ServerConnectionInfo serverInfo,
 
       for (; j < files.size(); j++) {
         File f = files.get(j);
+        log.info("Trying file {}", f);
         String fileName = f.getName();
         int idx = fileName.lastIndexOf(".");
         String extension = "";
@@ -605,7 +605,7 @@ public static final boolean uploadFilesToServer(ServerConnectionInfo serverInfo,
           fb = new FileBody(f, ContentType.create("application/octet-stream"));
           builder.addPart(f.getName(), fb);
           byteCount += f.length();
-          log.warn("added unrecognized file (application/octet-stream) " + f.getName());
+          log.warn("added unrecognized file (application/octet-stream)");
         }
 
         // we've added at least one attachment to the request...

src/org/opendatakit/briefcase/util/BadXMLFixer.java

@@ -46,23 +46,17 @@ public final class BadXMLFixer {
   private static final String ENCODING = "UTF-8";
 
   public static Document fixBadXML(File xmlFile) throws CannotFixXMLException {
-    log.warn("Trying to fix the submission " + xmlFile.getAbsolutePath());
+    log.info("Trying to fix the submission {} ", xmlFile.getAbsolutePath());
 
     try {
       String originalXML = FileUtils.readFileToString(xmlFile, ENCODING);
       String fixedXML = fixXML(originalXML);
       File tempFile = File.createTempFile(xmlFile.getName(), ".fixed.xml");
       FileUtils.writeStringToFile(tempFile, fixedXML, ENCODING);
       return XmlManipulationUtils.parseXml(tempFile);
-    } catch (IOException e) {
-      log.error(e.getMessage(), e);
-      throw new CannotFixXMLException("Cannot fix " + xmlFile.getAbsolutePath(), e);
-    } catch (ParsingException e) {
-      log.error(e.getMessage(), e);
-      throw new CannotFixXMLException("Cannot fix " + xmlFile.getAbsolutePath(), e);
-    } catch (FileSystemException e) {
-      log.error(e.getMessage(), e);
-      throw new CannotFixXMLException("Cannot fix " + xmlFile.getAbsolutePath(), e);
+    } catch (IOException | ParsingException | FileSystemException e) {
+      log.error("Cannot fix xml", e);
+      throw new CannotFixXMLException("Cannot fix xml", e);
     }
   }