More verbosity in logs/output panel in case of errors (#105)

* More verbosity in logs/output panel in case of errors

* Wrap exception in collectDefinitionLocations

* Revert exception widening in SmithyLanguageServer

Description of changes:

While fighting some issues in a project not loading (recently seeing some similar to #100), I found that we were discarding some useful information, such as stack traces, which makes it more difficult to debug issues on remote machines (such as my colleagues').

This PR adds more e.printStackTrace() in cases where we only printed the exception messages. While this isn't perfect, and it's far from ideal UX when you see potentially duplicate stack traces, I find that in this case more is better than none.

Many of the issues I'm seeing anytime something goes wrong in loading a project are NPEs on the project field in SmithyTextDocumentService, so I also marked that as nullable to make it clearer that it's not a safe field to use in all cases. We can amend the usages of that field in future PRs so that they have reasonable fallbacks.

Author: kubukoz

src/main/java/software/amazon/smithy/lsp/SmithyLanguageServer.java

@@ -69,7 +69,7 @@ public CompletableFuture<InitializeResult> initialize(InitializeParams params) {
         loadSmithyBuild(workspaceRoot);
       } catch (Exception e) {
         LspLog.println("Failure trying to load extensions from workspace root: " + workspaceRoot.getAbsolutePath());
-        LspLog.println(e.toString());
+        e.printStackTrace();
       }
     } else {
       LspLog.println("Workspace root was null");
@@ -78,7 +78,7 @@ public CompletableFuture<InitializeResult> initialize(InitializeParams params) {
     if (params.getWorkspaceFolders() == null) {
       try {
         tempWorkspaceRoot = Files.createTempDirectory("smithy-lsp-workspace").toFile();
-        System.out.println("Created temporary workspace root: " + tempWorkspaceRoot);
+        LspLog.println("Created temporary workspace root: " + tempWorkspaceRoot);
         tempWorkspaceRoot.deleteOnExit();
         WorkspaceFolder workspaceFolder = new WorkspaceFolder(tempWorkspaceRoot.toURI().toString());
         params.setWorkspaceFolders(ListUtils.of(workspaceFolder));
@@ -95,6 +95,7 @@ public CompletableFuture<InitializeResult> initialize(InitializeParams params) {
         loadSmithyBuild(root);
       } catch (Exception e) {
         LspLog.println("Error when loading workspace folder " + ws.toString() + ": " + e);
+        e.printStackTrace();
       }
     }
 

src/main/java/software/amazon/smithy/lsp/SmithyTextDocumentService.java

@@ -38,6 +38,7 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
+import javax.annotation.Nullable;
 import org.eclipse.lsp4j.CodeAction;
 import org.eclipse.lsp4j.CodeActionParams;
 import org.eclipse.lsp4j.Command;
@@ -102,6 +103,7 @@ public class SmithyTextDocumentService implements TextDocumentService {
     private final List<CompletionItem> baseCompletions = new ArrayList<>();
     private Optional<LanguageClient> client;
     private final List<Location> noLocations = Collections.emptyList();
+    @Nullable
     private SmithyProject project;
     private final File temporaryFolder;
 
@@ -142,12 +144,17 @@ public Optional<File> getRoot() {
     public void createProject(SmithyBuildExtensions ext, File root) {
         Either<Exception, SmithyProject> loaded = SmithyProject.load(ext, root);
         if (loaded.isRight()) {
-            this.project = loaded.getRight();
+            SmithyProject project = loaded.getRight();
+            this.project = project;
             clearAllDiagnostics();
-            sendInfo("Project loaded with " + this.project.getExternalJars().size() + " external jars and "
-                    + this.project.getSmithyFiles().size() + " discovered smithy files");
+            sendInfo("Project loaded with " + project.getExternalJars().size() + " external jars and "
+                    + project.getSmithyFiles().size() + " discovered smithy files");
         } else {
-            sendError("Failed to create Smithy project: " + loaded.getLeft().toString());
+            sendError(
+                "Failed to create Smithy project. See output panel for details. Uncaught exception: "
+                    + loaded.getLeft().toString()
+            );
+            loaded.getLeft().printStackTrace();
         }
     }
 
@@ -170,6 +177,7 @@ public void createProject(File root) {
                     LspLog.println("Loaded build extensions " + local + " from " + smithyBuild.getAbsolutePath());
                 } catch (Exception e) {
                     LspLog.println("Failed to load config from" + smithyBuild + ": " + e);
+                    e.printStackTrace();
                     brokenFiles.add(smithyBuild.toString());
                 }
             }
@@ -178,7 +186,10 @@ public void createProject(File root) {
         if (brokenFiles.isEmpty()) {
             createProject(result.build(), root);
         } else {
-            sendError("Failed to load the build, following files have problems: \n" + String.join("\n", brokenFiles));
+            sendError(
+                "Failed to load the build, the following build files have problems: \n"
+                    + String.join("\n", brokenFiles)
+            );
         }
     }
 
@@ -210,6 +221,7 @@ public CompletableFuture<Either<List<CompletionItem>, CompletionList>> completio
         } catch (Exception e) {
             LspLog.println(
                     "Failed to identify token for completion in " + params.getTextDocument().getUri() + ": " + e);
+            e.printStackTrace();
         }
         return Utils.completableFuture(Either.forLeft(baseCompletions));
     }
@@ -438,7 +450,7 @@ public CompletableFuture<List<Either<SymbolInformation, DocumentSymbol>>> docume
                     .collect(Collectors.toList())
             );
         } catch (Exception e) {
-            e.printStackTrace(System.err);
+            e.printStackTrace();
 
             return Utils.completableFuture(Collections.emptyList());
         }
@@ -526,6 +538,7 @@ public CompletableFuture<Hover> hover(HoverParams params) {
             }
         } catch (Exception e) {
             LspLog.println("Failed to determine hover content: " + e);
+            e.printStackTrace();
         }
 
         hover.setContents(content);
@@ -612,6 +625,7 @@ public void didChange(DidChangeTextDocumentParams params) {
         } catch (Exception e) {
             LspLog.println("Failed to write temporary contents for file " + original + " into temporary file "
                     + tempFile + " : " + e);
+            e.printStackTrace();
         }
 
         report(recompile(original, Optional.ofNullable(tempFile)));

src/main/java/software/amazon/smithy/lsp/ext/FileCachingCollector.java

@@ -63,7 +63,12 @@ public Map<ShapeId, Location> collectDefinitionLocations(Model model) {
         this.membersToUpdateMap = new HashMap<>();
 
         for (ModelFile modelFile : this.fileCache.values()) {
-            collectContainerShapeLocationsInModelFile(modelFile);
+            try {
+                collectContainerShapeLocationsInModelFile(modelFile);
+            } catch (Exception e) {
+                throw new RuntimeException("Exception while collecting container shape locations in model file: "
+                        + modelFile.filename, e);
+            }
         }
 
         operationsWithInlineInputOutputMap.forEach((this::collectInlineInputOutputLocations));

src/main/java/software/amazon/smithy/lsp/ext/SmithyProject.java

@@ -172,7 +172,13 @@ private static Either<Exception, SmithyProject> load(
             return Either.forLeft(model.getLeft());
         } else {
             model.getRight().getValidationEvents().forEach(LspLog::println);
-            return Either.forRight(new SmithyProject(imports, smithyFiles, externalJars, root, model.getRight()));
+
+            try {
+                SmithyProject p = new SmithyProject(imports, smithyFiles, externalJars, root, model.getRight());
+                return Either.forRight(p);
+            } catch (Exception e) {
+                return Either.forLeft(e);
+            }
         }
     }