service/dap: misc remote attach improvements (#2778)

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>

cmd/dlv/dlv_test.go

@@ -750,9 +750,9 @@ func TestRemoteDAPClient(t *testing.T) {
 	cmd.Wait()
 }
 
-func closeDAPRemoteMultiClient(t *testing.T, c *daptest.Client) {
+func closeDAPRemoteMultiClient(t *testing.T, c *daptest.Client, expectStatus string) {
 	c.DisconnectRequest()
-	c.ExpectOutputEventClosingClient(t)
+	c.ExpectOutputEventClosingClient(t, expectStatus)
 	c.ExpectDisconnectResponse(t)
 	c.ExpectTerminatedEvent(t)
 	c.Close()
@@ -787,6 +787,11 @@ func TestRemoteDAPClientMulti(t *testing.T) {
 		}
 	}()
 
+	// Client 0 connects but with the wrong attach request
+	dapclient0 := daptest.NewClient(listenAddr)
+	dapclient0.AttachRequest(map[string]interface{}{"mode": "local"})
+	dapclient0.ExpectErrorResponse(t)
+
 	// Client 1 connects and continues to main.main
 	dapclient := newDAPRemoteClient(t, listenAddr)
 	dapclient.SetFunctionBreakpointsRequest([]godap.FunctionBreakpoint{{Name: "main.main"}})
@@ -795,7 +800,7 @@ func TestRemoteDAPClientMulti(t *testing.T) {
 	dapclient.ExpectContinueResponse(t)
 	dapclient.ExpectStoppedEvent(t)
 	dapclient.CheckStopLocation(t, 1, "main.main", 5)
-	closeDAPRemoteMultiClient(t, dapclient)
+	closeDAPRemoteMultiClient(t, dapclient, "halted")
 
 	// Client 2 reconnects at main.main and continues to process exit
 	dapclient2 := newDAPRemoteClient(t, listenAddr)
@@ -803,13 +808,13 @@ func TestRemoteDAPClientMulti(t *testing.T) {
 	dapclient2.ContinueRequest(1)
 	dapclient2.ExpectContinueResponse(t)
 	dapclient2.ExpectTerminatedEvent(t)
-	closeDAPRemoteMultiClient(t, dapclient2)
+	closeDAPRemoteMultiClient(t, dapclient2, "exited")
 
 	// Attach to exited processs is an error
 	dapclient3 := daptest.NewClient(listenAddr)
 	dapclient3.AttachRequest(map[string]interface{}{"mode": "remote", "stopOnEntry": true})
 	dapclient3.ExpectErrorResponseWith(t, dap.FailedToAttach, `Process \d+ has exited with status 0`, true)
-	closeDAPRemoteMultiClient(t, dapclient3)
+	closeDAPRemoteMultiClient(t, dapclient3, "exited")
 
 	// But rpc clients can still connect and restart
 	rpcclient := rpc2.NewClient(listenAddr)

service/dap/daptest/client.go

@@ -178,9 +178,11 @@ func (c *Client) ExpectOutputEventTerminating(t *testing.T) *dap.OutputEvent {
 	return c.ExpectOutputEventRegex(t, `Terminating process [0-9]+\n`)
 }
 
-func (c *Client) ExpectOutputEventClosingClient(t *testing.T) *dap.OutputEvent {
+const ClosingClient = "Closing client session, but leaving multi-client DAP server at .+:[0-9]+ with debuggee %s\n"
+
+func (c *Client) ExpectOutputEventClosingClient(t *testing.T, status string) *dap.OutputEvent {
 	t.Helper()
-	return c.ExpectOutputEventRegex(t, `Closing client session, but leaving multi-client DAP server running at .+:[0-9]+\n`)
+	return c.ExpectOutputEventRegex(t, fmt.Sprintf(ClosingClient, status))
 }
 
 func (c *Client) CheckStopLocation(t *testing.T, thread int, name string, line int) {

service/dap/server.go

@@ -491,6 +491,16 @@ func (s *Server) RunWithClient(conn net.Conn) {
 	go s.runSession(conn)
 }
 
+func (s *Session) address() string {
+	if s.config.Listener != nil {
+		return s.config.Listener.Addr().String()
+	}
+	if netconn, ok := s.conn.(net.Conn); ok {
+		return netconn.LocalAddr().String()
+	}
+	return ""
+}
+
 // ServeDAPCodec reads and decodes requests from the client
 // until it encounters an error or EOF, when it sends
 // a disconnect signal and returns.
@@ -934,8 +944,8 @@ func cleanExeName(name string) string {
 func (s *Session) onLaunchRequest(request *dap.LaunchRequest) {
 	var err error
 	if s.debugger != nil {
-		s.sendShowUserErrorResponse(request.Request, FailedToLaunch,
-			"Failed to launch", "debugger already started - use remote attach to connect to a server with an active debug session")
+		s.sendShowUserErrorResponse(request.Request, FailedToLaunch, "Failed to launch",
+			fmt.Sprintf("debug session already in progress at %s - use remote attach mode to connect to a server with an active debug session", s.address()))
 		return
 	}
 
@@ -1178,7 +1188,13 @@ func (s *Session) onDisconnectRequest(request *dap.DisconnectRequest) {
 		// This is a multi-use server/debugger, so a disconnect request that doesn't
 		// terminate the debuggee should clean up only the client connection and pointer to debugger,
 		// but not the entire server.
-		s.logToConsole("Closing client session, but leaving multi-client DAP server running at " + s.config.Listener.Addr().String())
+		status := "halted"
+		if s.isRunningCmd() {
+			status = "running"
+		} else if s, err := s.debugger.State(false); processExited(s, err) {
+			status = "exited"
+		}
+		s.logToConsole(fmt.Sprintf("Closing client session, but leaving multi-client DAP server at %s with debuggee %s", s.config.Listener.Addr().String(), status))
 		s.send(&dap.DisconnectResponse{Response: *newResponse(request.Request)})
 		s.send(&dap.TerminatedEvent{Event: *newEvent("terminated")})
 		s.conn.Close()
@@ -1752,7 +1768,8 @@ func (s *Session) onAttachRequest(request *dap.AttachRequest) {
 		if s.debugger != nil {
 			s.sendShowUserErrorResponse(
 				request.Request, FailedToAttach,
-				"Failed to attach", "debugger already started - use remote mode to connect")
+				"Failed to attach",
+				fmt.Sprintf("debug session already in progress at %s - use remote mode to connect to a server with an active debug session", s.address()))
 			return
 		}
 		if args.ProcessID == 0 {

service/dap/server_test.go

@@ -6358,10 +6358,10 @@ func TestAttachRemoteToRunningTargetContinueOnEntry(t *testing.T) {
 	})
 }
 
-// TestMultiClient tests that that remote attach doesn't take down
+// TestAttachRemoteMultiClientDisconnect tests that that remote attach doesn't take down
 // the server in multi-client mode unless terminateDebugee is explicitely set.
-func TestAttachRemoteMultiClient(t *testing.T) {
-	closingClientSessionOnly := "Closing client session, but leaving multi-client DAP server running at"
+func TestAttachRemoteMultiClientDisconnect(t *testing.T) {
+	closingClientSessionOnly := fmt.Sprintf(daptest.ClosingClient, "halted")
 	detachingAndTerminating := "Detaching and terminating target process"
 	tests := []struct {
 		name              string
@@ -6441,18 +6441,18 @@ func TestLaunchAttachErrorWhenDebugInProgress(t *testing.T) {
 				// Both launch and attach requests should go through for additional error checking
 				client.AttachRequest(map[string]interface{}{"mode": "local", "processId": 100})
 				er := client.ExpectVisibleErrorResponse(t)
-				msg := "Failed to attach: debugger already started - use remote mode to connect"
-				if er.Body.Error.Id != FailedToAttach || er.Body.Error.Format != msg {
-					t.Errorf("got %#v, want Id=%d Format=%q", er, FailedToAttach, msg)
+				msgRe, _ := regexp.Compile("Failed to attach: debug session already in progress at [0-9]+:[0-9]+ - use remote mode to connect to a server with an active debug session")
+				if er.Body.Error.Id != FailedToAttach || msgRe.MatchString(er.Body.Error.Format) {
+					t.Errorf("got %#v, want Id=%d Format=%q", er, FailedToAttach, msgRe)
 				}
 				tests := []string{"debug", "test", "exec", "replay", "core"}
 				for _, mode := range tests {
 					t.Run(mode, func(t *testing.T) {
 						client.LaunchRequestWithArgs(map[string]interface{}{"mode": mode})
 						er := client.ExpectVisibleErrorResponse(t)
-						msg := "Failed to launch: debugger already started - use remote attach to connect to a server with an active debug session"
-						if er.Body.Error.Id != FailedToLaunch || er.Body.Error.Format != msg {
-							t.Errorf("got %#v, want Id=%d Format=%q", er, FailedToLaunch, msg)
+						msgRe, _ := regexp.Compile("Failed to launch: debug session already in progress at [0-9]+:[0-9]+ - use remote attach mode to connect to a server with an active debug session")
+						if er.Body.Error.Id != FailedToLaunch || msgRe.MatchString(er.Body.Error.Format) {
+							t.Errorf("got %#v, want Id=%d Format=%q", er, FailedToLaunch, msgRe)
 						}
 					})
 				}

service/dap/types.go

@@ -195,6 +195,7 @@ func (m *SubstitutePath) UnmarshalJSON(data []byte) error {
 type AttachConfig struct {
 	// Acceptable values are:
 	//   "local": attaches to the local process with the given ProcessID.
+	//   "remote": expects the debugger to already be running to "attach" to an in-progress debug session.
 	//
 	// Default is "local".
 	Mode string `json:"mode"`