Adjusting the error handling for launches and adding extra tests for additional launch failure cases.

Author: ashgti

lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py

@@ -27,6 +27,34 @@ def test_default(self):
         lines = output.splitlines()
         self.assertIn(program, lines[0], "make sure program path is in first argument")
 
+    def test_failing_launch_program(self):
+        """
+        Tests launching with an invalid program.
+        """
+        program = self.getBuildArtifact("a.out")
+        self.create_debug_adapter()
+        response = self.launch(program, expectFailure=True)
+        self.assertFalse(response["success"])
+        self.assertEqual(
+            "'{0}' does not exist".format(program), response["body"]["error"]["format"]
+        )
+
+    def test_failing_launch_commands_and_run_in_terminal(self):
+        """
+        Tests launching with an invalid program.
+        """
+        program = self.getBuildArtifact("a.out")
+        self.create_debug_adapter()
+        response = self.launch(
+            program, launchCommands=["a b c"], runInTerminal=True, expectFailure=True
+        )
+        self.assertFalse(response["success"])
+        self.assertTrue(self.get_dict_value(response, ["body", "error", "showUser"]))
+        self.assertEqual(
+            "launchCommands and runInTerminal are mutually exclusive",
+            self.get_dict_value(response, ['body', 'error', 'format']),
+        )
+
     @skipIfWindows
     def test_termination(self):
         """

lldb/tools/lldb-dap/DAP.cpp

@@ -82,7 +82,7 @@ DAP::DAP(Log *log, const ReplMode default_repl_mode,
          std::vector<std::string> pre_init_commands, Transport &transport)
     : log(log), transport(transport), broadcaster("lldb-dap"),
       exception_breakpoints(), focus_tid(LLDB_INVALID_THREAD_ID),
-      is_attach(false), stop_at_entry(false),
+      stop_at_entry(false), is_attach(false),
       restarting_process_id(LLDB_INVALID_PROCESS_ID),
       configuration_done_sent(false), waiting_for_run_in_terminal(false),
       progress_event_reporter(
@@ -668,7 +668,7 @@ lldb::SBTarget DAP::CreateTarget(lldb::SBError &error) {
   // creation. We also use target triple and platform from the launch
   // configuration, if given, since in some cases ELF file doesn't contain
   // enough information to determine correct arch and platform (or ELF can be
-  // omitted at all), so it is good to leave the user an apportunity to specify
+  // omitted at all), so it is good to leave the user an opportunity to specify
   // those. Any of those three can be left empty.
   auto target = this->debugger.CreateTarget(
       configuration.program.value_or("").data(),
@@ -677,12 +677,6 @@ lldb::SBTarget DAP::CreateTarget(lldb::SBError &error) {
       true, // Add dependent modules.
       error);
 
-  if (error.Fail()) {
-    // Update message if there was an error.
-    error.SetErrorStringWithFormat(
-        "Could not create a target for a program: %s.", error.GetCString());
-  }
-
   return target;
 }
 

lldb/tools/lldb-dap/DAP.h

@@ -182,8 +182,8 @@ struct DAP {
   lldb::tid_t focus_tid;
   bool disconnecting = false;
   llvm::once_flag terminated_event_flag;
-  bool is_attach;
   bool stop_at_entry;
+  bool is_attach;
   // The process event thread normally responds to process exited events by
   // shutting down the entire adapter. When we're restarting, we keep the id of
   // the old process here so we can detect this case and keep running.

lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp

@@ -9,6 +9,7 @@
 #include "DAP.h"
 #include "EventHelper.h"
 #include "JSONUtils.h"
+#include "LLDBUtils.h"
 #include "Protocol/ProtocolRequests.h"
 #include "RequestHandler.h"
 #include "llvm/Support/Error.h"
@@ -25,19 +26,19 @@ Error LaunchRequestHandler::Run(const LaunchRequestArguments &arguments) const {
   dap.last_launch_request = arguments;
   dap.stop_at_entry = arguments.stopOnEntry;
 
-  if (!arguments.launchCommands.empty() && arguments.runInTerminal) {
-    return make_error<DAPError>("launchCommands and runInTerminal are mutually exclusive");
-  }
+  if (!arguments.launchCommands.empty() && arguments.runInTerminal)
+    return make_error<DAPError>(
+        "launchCommands and runInTerminal are mutually exclusive");
 
   PrintWelcomeMessage();
 
   // This is a hack for loading DWARF in .o files on Mac where the .o files
   // in the debug map of the main executable have relative paths which
   // require the lldb-dap binary to have its working directory set to that
   // relative root for the .o files in order to be able to load debug info.
-  const std::string debuggerRoot = dap.configuration.debuggerRoot.value_or("");
-  if (!debuggerRoot.empty())
-    sys::fs::set_current_path(debuggerRoot);
+  const std::string debugger_root = dap.configuration.debuggerRoot.value_or("");
+  if (!debugger_root.empty())
+    sys::fs::set_current_path(debugger_root);
 
   // Run any initialize LLDB commands the user specified in the launch.json.
   // This is run before target is created, so commands can't do anything with
@@ -47,10 +48,12 @@ Error LaunchRequestHandler::Run(const LaunchRequestArguments &arguments) const {
 
   dap.ConfigureSourceMaps();
 
-  lldb::SBError status;
-  dap.SetTarget(dap.CreateTarget(status));
-  if (status.Fail())
-    return make_error<DAPError>(status.GetCString());
+  lldb::SBError error;
+  lldb::SBTarget target = dap.CreateTarget(error);
+  if (error.Fail())
+    return ToError(error);
+
+  dap.SetTarget(target);
 
   // Run any pre run LLDB commands the user specified in the launch.json
   if (Error err = dap.RunPreRunCommands())

lldb/tools/lldb-dap/Handler/RequestHandler.h

@@ -176,6 +176,9 @@ class RequestHandler : public BaseRequestHandler {
 
   protocol::ErrorResponseBody ToResponse(llvm::Error err) const {
     protocol::ErrorMessage error_message;
+    // Default to showing the user errors unless otherwise specified by a
+    // DAPError.
+    error_message.showUser = true;
     error_message.sendTelemetry = false;
     if (llvm::Error unhandled = llvm::handleErrors(
             std::move(err), [&](const DAPError &E) -> llvm::Error {
@@ -185,8 +188,9 @@ class RequestHandler : public BaseRequestHandler {
               error_message.url = E.getURL();
               error_message.urlLabel = E.getURLLabel();
               return llvm::Error::success();
-            }))
+            })) {
       error_message.format = llvm::toString(std::move(unhandled));
+    }
     protocol::ErrorResponseBody body;
     body.error = error_message;
     return body;

lldb/tools/lldb-dap/LLDBUtils.cpp

@@ -7,11 +7,19 @@
 //===----------------------------------------------------------------------===//
 
 #include "LLDBUtils.h"
-#include "DAP.h"
 #include "JSONUtils.h"
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBFrame.h"
 #include "lldb/API/SBStringList.h"
-
+#include "lldb/API/SBThread.h"
+#include "lldb/lldb-enumerations.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/raw_ostream.h"
 #include <mutex>
+#include <system_error>
 
 namespace lldb_dap {