improve exception reporting

mr-c · mr-c · commit c6d4da38f0f7 · 2023-01-01T18:44:12.000+01:00
diff --git a/src/main/java/org/commonwl/view/cwl/CWLService.java b/src/main/java/org/commonwl/view/cwl/CWLService.java
@@ -965,6 +965,9 @@ private String extractDoc(Map<String, Object> cwlFile) {
     if (cwlFile != null) {
       if (cwlFile.containsKey(DOC)) {
         Object doc = cwlFile.get(DOC);
+        if (doc == null) {
+          return null;
+        }
         if (doc.getClass().isAssignableFrom(String.class)) {
           return (String) doc;
         }
diff --git a/src/main/java/org/commonwl/view/cwl/CWLToolRunner.java b/src/main/java/org/commonwl/view/cwl/CWLToolRunner.java
@@ -33,6 +33,8 @@
 import org.commonwl.view.workflow.Workflow;
 import org.commonwl.view.workflow.WorkflowRepository;
 import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.errors.TransportException;
+import org.eclipse.jgit.errors.MissingObjectException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -105,22 +107,66 @@ public void createWorkflowFromQueued(QueuedWorkflow queuedWorkflow)
       queuedWorkflow.setCwltoolStatus(CWLToolStatus.SUCCESS);
 
     } catch (QueryException ex) {
-      logger.error("Jena query exception ", ex);
+      logger.error("Jena query exception for workflow " + queuedWorkflow.getId(), ex);
       queuedWorkflow.setCwltoolStatus(CWLToolStatus.ERROR);
       queuedWorkflow.setMessage("An error occurred when executing a query on the SPARQL store");
       FileUtils.deleteGitRepository(repo);
     } catch (CWLValidationException ex) {
-      logger.error(ex.getMessage(), ex);
+      String message = ex.getMessage();
+      logger.error(
+          "Workflow " + queuedWorkflow.getId() + " from " + gitInfo.toSummary() + " : " + message,
+          ex);
       queuedWorkflow.setCwltoolStatus(CWLToolStatus.ERROR);
-      queuedWorkflow.setMessage(ex.getMessage());
+      queuedWorkflow.setMessage(message);
+      FileUtils.deleteGitRepository(repo);
+    } catch (TransportException ex) {
+      String message = ex.getMessage();
+      logger.error(
+          "Workflow retrieval error while processing "
+              + queuedWorkflow.getId()
+              + " from "
+              + gitInfo.toSummary()
+              + " : "
+              + message,
+          ex);
+      queuedWorkflow.setCwltoolStatus(CWLToolStatus.ERROR);
+      if (message.contains(
+          "Authentication is required but no CredentialsProvider has been registered")) {
+        queuedWorkflow.setMessage(
+            "Unable to retrieve the Git repository: it may be private, misnamed, or removed. "
+                + message);
+      } else {
+        queuedWorkflow.setMessage(message);
+      }
+      FileUtils.deleteGitRepository(repo);
+    } catch (MissingObjectException ex) {
+      String message = ex.getMessage();
+      logger.error(
+          "Workflow retrieval error while processing "
+              + queuedWorkflow.getId()
+              + " from "
+              + gitInfo.toSummary()
+              + " : "
+              + message,
+          ex);
+      queuedWorkflow.setCwltoolStatus(CWLToolStatus.ERROR);
+      queuedWorkflow.setMessage("Unable to retrieve a needed Git object: " + message);
       FileUtils.deleteGitRepository(repo);
     } catch (Exception ex) {
-      logger.error("Unexpected error", ex);
+      logger.error(
+          "Unexpected error processing workflow "
+              + queuedWorkflow.getId()
+              + " from "
+              + gitInfo.toSummary()
+              + " : "
+              + ex.getMessage(),
+          ex);
       queuedWorkflow.setCwltoolStatus(CWLToolStatus.ERROR);
       queuedWorkflow.setMessage(
           "Whoops! Cwltool ran successfully, but an unexpected "
               + "error occurred in CWLViewer!\n"
-              + "Help us by reporting it on Gitter or a Github issue\n");
+              + ex.getMessage()
+              + "\nHelp us by reporting it at https://github.com/common-workflow-language/cwlviewer/issues/new/choose\n");
       FileUtils.deleteGitRepository(repo);
     } finally {
       gitSemaphore.release(repoUrl);
diff --git a/src/main/java/org/commonwl/view/git/GitDetails.java b/src/main/java/org/commonwl/view/git/GitDetails.java
@@ -271,4 +271,9 @@ public boolean equals(Object o) {
   public int hashCode() {
     return Objects.hash(repoUrl, branch, path, packedId);
   }
+
+  public String toSummary() {
+    return String.format(
+        "repoUrl: %s branch: %s path: %s packedId: %s", repoUrl, branch, path, packedId);
+  }
 }
diff --git a/src/main/java/org/commonwl/view/researchobject/ROBundleFactory.java b/src/main/java/org/commonwl/view/researchobject/ROBundleFactory.java
@@ -63,7 +63,7 @@ public ROBundleFactory(ROBundleService roBundleService, WorkflowRepository workf
    */
   @Async
   public void createWorkflowRO(Workflow workflow) throws IOException, InterruptedException {
-    logger.info("Creating Research Object Bundle");
+    logger.info("Creating Research Object Bundle for workflow " + workflow.getID());
 
     // Get the whole containing folder, not just the workflow itself
     GitDetails githubInfo = workflow.getRetrievedFrom();
@@ -91,6 +91,6 @@ public void createWorkflowRO(Workflow workflow) throws IOException, InterruptedE
     // Add RO Bundle to associated workflow model
     workflow.setRoBundlePath(bundleLocation.toString());
     workflowRepository.save(workflow);
-    logger.info("Finished saving Research Object Bundle");
+    logger.info("Finished saving Research Object Bundle for workflow " + workflow.getID());
   }
 }
diff --git a/src/main/java/org/commonwl/view/workflow/WorkflowNotFoundException.java b/src/main/java/org/commonwl/view/workflow/WorkflowNotFoundException.java
@@ -24,4 +24,13 @@
 
 /** Exception thrown when a workflow ID does not exist */
 @ResponseStatus(value = HttpStatus.NOT_FOUND)
-public class WorkflowNotFoundException extends RuntimeException {}
+public class WorkflowNotFoundException extends RuntimeException {
+
+  public WorkflowNotFoundException(String message) {
+    super(message);
+  }
+
+  public WorkflowNotFoundException() {
+    super();
+  }
+}
diff --git a/src/main/java/org/commonwl/view/workflow/WorkflowService.java b/src/main/java/org/commonwl/view/workflow/WorkflowService.java
@@ -343,7 +343,8 @@ public QueuedWorkflow createQueuedWorkflow(GitDetails gitInfo)
           List<WorkflowOverview> overviews =
               cwlService.getWorkflowOverviewsFromPacked(workflowFile.toFile());
           if (overviews.size() == 0) {
-            throw new IOException("No workflow was found within the packed CWL file");
+            throw new IOException(
+                "No workflow was found within the packed CWL file. " + gitInfo.toSummary());
           } else {
             // Dummy queued workflow object to return the list
             QueuedWorkflow overviewList = new QueuedWorkflow();
@@ -375,14 +376,14 @@ public QueuedWorkflow createQueuedWorkflow(GitDetails gitInfo)
       try {
         cwlToolRunner.createWorkflowFromQueued(queuedWorkflow);
       } catch (Exception e) {
-        logger.error("Could not update workflow with cwltool", e);
+        logger.error("Could not update workflow with cwltool: " + gitInfo.toSummary(), e);
       }
 
     } catch (GitAPIException | RuntimeException | IOException e) {
       logger.warn(
           String.format(
-              "Failed to create Queued Workflow: %s - Temporary files will be deleted",
-              e.getMessage()),
+              "Failed to create Queued Workflow: %s - Temporary files will be deleted for %s.",
+              e.getMessage(), gitInfo.toSummary()),
           e);
       FileUtils.deleteGitRepository(repo);
       throw e;
@@ -406,7 +407,7 @@ public void retryCwltool(QueuedWorkflow queuedWorkflow) {
     try {
       cwlToolRunner.createWorkflowFromQueued(queuedWorkflow);
     } catch (Exception e) {
-      logger.error("Could not update workflow with cwltool", e);
+      logger.error("Could not update workflow " + queuedWorkflow.getId() + " with cwltool.", e);
     }
   }
 
@@ -481,13 +482,14 @@ public PathResource getWorkflowGraph(String format, GitDetails gitDetails)
         extension = "dot";
         break;
       default:
-        throw new WorkflowNotFoundException();
+        throw new WorkflowNotFoundException("Format " + format + " not recognized.");
     }
 
     // Get workflow
     Workflow workflow = getWorkflow(gitDetails);
     if (workflow == null) {
-      throw new WorkflowNotFoundException();
+      throw new WorkflowNotFoundException(
+          "Unable to retrieve workflow for " + gitDetails.toSummary());
     }
 
     // Generate graph and serve the file
@@ -506,7 +508,9 @@ private void generateROBundle(Workflow workflow) {
     try {
       ROBundleFactory.createWorkflowRO(workflow);
     } catch (Exception ex) {
-      logger.error("Error creating RO Bundle", ex);
+      logger.error(
+          "Error creating RO Bundle for workflow from " + workflow.getRetrievedFrom().toSummary(),
+          ex);
     }
   }
 
@@ -520,9 +524,9 @@ private void removeWorkflow(Workflow workflow) {
     if (workflow.getRoBundlePath() != null) {
       File roBundle = new File(workflow.getRoBundlePath());
       if (roBundle.delete()) {
-        logger.debug("Deleted Research Object Bundle");
+        logger.debug("Deleted Research Object Bundle for workflow " + workflow.getID());
       } else {
-        logger.debug("Failed to delete Research Object Bundle");
+        logger.info("Failed to delete Research Object Bundle for workflow " + workflow.getID());
       }
     }
 
@@ -556,7 +560,8 @@ private boolean cacheExpired(Workflow workflow) {
         if (expirationDate.before(new Date())) {
           // Cache expiry time has elapsed
           // Check current head of the branch with the cached head
-          logger.info("Time has expired for caching, checking commits...");
+          logger.info(
+              "Time has expired for caching, checking commits for workflow " + workflow.getID());
           String currentHead;
           boolean safeToAccess = gitSemaphore.acquire(workflow.getRetrievedFrom().getRepoUrl());
           try {
@@ -565,7 +570,13 @@ private boolean cacheExpired(Workflow workflow) {
           } finally {
             gitSemaphore.release(workflow.getRetrievedFrom().getRepoUrl());
           }
-          logger.info("Current: " + workflow.getLastCommit() + ", HEAD: " + currentHead);
+          logger.info(
+              "Current: "
+                  + workflow.getLastCommit()
+                  + ", HEAD: "
+                  + currentHead
+                  + " for workflow "
+                  + workflow.getID());
 
           // Reset date in database if there are still no changes
           boolean expired = !workflow.getLastCommit().equals(currentHead);
@@ -579,7 +590,10 @@ private boolean cacheExpired(Workflow workflow) {
         }
       } catch (Exception ex) {
         // Default to no expiry if there was an API error
-        logger.error("API Error when checking for latest commit ID for caching", ex);
+        logger.error(
+            "API Error when checking for latest commit ID for caching for workflow "
+                + workflow.getID(),
+            ex);
       }
     }
     return false;

Original file line number	Diff line number	Diff line change
`@@ -271,4 +271,9 @@ public boolean equals(Object o) {`
`271`	`271`	`public int hashCode() {`
`272`	`272`	`return Objects.hash(repoUrl, branch, path, packedId);`
`273`	`273`	`}`
	`274`	`+`
	`275`	`+ public String toSummary() {`
	`276`	`+ return String.format(`
	`277`	`+ "repoUrl: %s branch: %s path: %s packedId: %s", repoUrl, branch, path, packedId);`
	`278`	`+ }`
`274`	`279`	`}`