service/dap: Add support for threads request (#1914)

* Add support for threads request

* Address review comments

* Relax threads test condition

* Address review comments

* Clean up unnecessary newline

* Respond to review comment
Co-authored-by: NPolina Sokolova <polinasok@users.noreply.github.com>

service/dap/daptest/client.go

@@ -4,7 +4,6 @@ package daptest
 
 import (
 	"bufio"
-	"encoding/json"
 	"fmt"
 	"log"
 	"net"
@@ -34,6 +33,7 @@ func NewClient(addr string) *Client {
 		log.Fatal("dialing:", err)
 	}
 	c := &Client{conn: conn, reader: bufio.NewReader(conn)}
+	c.seq = 1 // match VS Code numbering
 	return c
 }
 
@@ -43,8 +43,6 @@ func (c *Client) Close() {
 }
 
 func (c *Client) send(request dap.Message) {
-	jsonmsg, _ := json.Marshal(request)
-	fmt.Println("[client -> server]", string(jsonmsg))
 	dap.WriteProtocolMessage(c.conn, request)
 }
 
@@ -120,6 +118,16 @@ func (c *Client) ExpectConfigurationDoneResponse(t *testing.T) *dap.Configuratio
 	return c.expectReadProtocolMessage(t).(*dap.ConfigurationDoneResponse)
 }
 
+func (c *Client) ExpectThreadsResponse(t *testing.T) *dap.ThreadsResponse {
+	t.Helper()
+	return c.expectReadProtocolMessage(t).(*dap.ThreadsResponse)
+}
+
+func (c *Client) ExpectStackTraceResponse(t *testing.T) *dap.StackTraceResponse {
+	t.Helper()
+	return c.expectReadProtocolMessage(t).(*dap.StackTraceResponse)
+}
+
 // InitializeRequest sends an 'initialize' request.
 func (c *Client) InitializeRequest() {
 	request := &dap.InitializeRequest{Request: *c.newRequest("initialize")}
@@ -199,6 +207,18 @@ func (c *Client) ContinueRequest(thread int) {
 	c.send(request)
 }
 
+// ThreadsRequest sends a 'threads' request.
+func (c *Client) ThreadsRequest() {
+	request := &dap.ThreadsRequest{Request: *c.newRequest("threads")}
+	c.send(request)
+}
+
+// StackTraceRequest sends a 'stackTrace' request.
+func (c *Client) StackTraceRequest() {
+	request := &dap.StackTraceRequest{Request: *c.newRequest("stackTrace")}
+	c.send(request)
+}
+
 // UnknownRequest triggers dap.DecodeProtocolMessageFieldError.
 func (c *Client) UnknownRequest() {
 	request := c.newRequest("unknown")

service/dap/error_ids.go

@@ -11,6 +11,7 @@ const (
 	// TODO(polina): confirm if the extension expects specific ids
 	// for specific cases, and we must match the existing adaptor
 	// or if these codes can evolve.
-	FailedToContinue = 3000
+	FailedToContinue       = 3000
+	UnableToDisplayThreads = 2003
 	// Add more codes as we support more requests
 )

service/dap/server.go

@@ -244,7 +244,7 @@ func (s *Server) handleRequest(request dap.Message) {
 	case *dap.SourceRequest:
 		s.sendUnsupportedErrorResponse(request.Request)
 	case *dap.ThreadsRequest:
-		s.sendUnsupportedErrorResponse(request.Request)
+		s.onThreadsRequest(request)
 	case *dap.TerminateThreadsRequest:
 		s.sendUnsupportedErrorResponse(request.Request)
 	case *dap.EvaluateRequest:
@@ -453,6 +453,49 @@ func (s *Server) onContinueRequest(request *dap.ContinueRequest) {
 	s.doContinue()
 }
 
+func (s *Server) onThreadsRequest(request *dap.ThreadsRequest) {
+	if s.debugger == nil {
+		s.sendErrorResponse(request.Request, UnableToDisplayThreads, "Unable to display threads", "debugger is nil")
+		return
+	}
+	gs, _, err := s.debugger.Goroutines(0, 0)
+	if err != nil {
+		switch err.(type) {
+		case *proc.ErrProcessExited:
+			// If the program exits very quickly, the initial threads request will complete after it has exited.
+			// A TerminatedEvent has already been sent. Ignore the err returned in this case.
+			s.send(&dap.ThreadsResponse{Response: *newResponse(request.Request)})
+		default:
+			s.sendErrorResponse(request.Request, UnableToDisplayThreads, "Unable to display threads", err.Error())
+		}
+		return
+	}
+
+	threads := make([]dap.Thread, len(gs))
+	if len(threads) == 0 {
+		// Depending on the debug session stage, goroutines information
+		// might not be available. However, the DAP spec states that
+		// "even if a debug adapter does not support multiple threads,
+		// it must implement the threads request and return a single
+		// (dummy) thread".
+		threads = []dap.Thread{{Id: 1, Name: "Dummy"}}
+	} else {
+		for i, g := range gs {
+			threads[i].Id = g.ID
+			if loc := g.UserCurrentLoc; loc.Function != nil {
+				threads[i].Name = loc.Function.Name()
+			} else {
+				threads[i].Name = fmt.Sprintf("%s@%d", loc.File, loc.Line)
+			}
+		}
+	}
+	response := &dap.ThreadsResponse{
+		Response: *newResponse(request.Request),
+		Body:     dap.ThreadsResponseBody{Threads: threads},
+	}
+	s.send(response)
+}
+
 func (s *Server) sendErrorResponse(request dap.Request, id int, summary string, details string) {
 	er := &dap.ErrorResponse{}
 	er.Type = "response"

service/dap/server_test.go

@@ -6,6 +6,8 @@ import (
 	"net"
 	"os"
 	"path/filepath"
+	"reflect"
+	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -66,66 +68,190 @@ func runTest(t *testing.T, name string, test func(c *daptest.Client, f protest.F
 	test(client, fixture)
 }
 
+// TestStopOnEntry emulates the message exchange that can be observed with
+// VS Code for the most basic debug session with "stopOnEntry" enabled:
+// - User selects "Start Debugging":  1 >> initialize
+//                                 :  1 << initialize
+//                                 :  2 >> launch
+//                                 :    << initialized event
+//                                 :  2 << launch
+//                                 :  3 >> setBreakpoints (empty)
+//                                 :  3 << setBreakpoints
+//                                 :  4 >> setExceptionBreakpoints (empty)
+//                                 :  4 << setExceptionBreakpoints
+//                                 :  5 >> configurationDone
+// - Program stops upon launching  :    << stopped event
+//                                 :  5 << configurationDone
+//                                 :  6 >> threads
+//                                 :  6 << threads (Dummy)
+//                                 :  7 >> threads
+//                                 :  7 << threads (Dummy)
+//                                 :  8 >> stackTrace
+//                                 :  8 << stackTrace (Unable to produce stack trace)
+//                                 :  9 >> stackTrace
+//                                 :  9 << stackTrace (Unable to produce stack trace)
+// - User selects "Continue"       : 10 >> continue
+//                                 : 10 << continue
+// - Program runs to completion    :    << terminated event
+//                                 : 11 >> disconnect
+//                                 : 11 << disconnect
+// This test exhaustively tests Seq and RequestSeq on all messages from the
+// server. Other tests do not necessarily need to repeat all these checks.
 func TestStopOnEntry(t *testing.T) {
 	runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
-		// This test exhaustively tests Seq and RequestSeq on all messages from the
-		// server. Other tests shouldn't necessarily repeat these checks.
+		// 1 >> initialize, << initialize
 		client.InitializeRequest()
 		initResp := client.ExpectInitializeResponse(t)
-		if initResp.Seq != 0 || initResp.RequestSeq != 0 {
-			t.Errorf("got %#v, want Seq=0, RequestSeq=0", initResp)
+		if initResp.Seq != 0 || initResp.RequestSeq != 1 {
+			t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=1", initResp)
 		}
 
+		// 2 >> launch, << initialized, << launch
 		client.LaunchRequest("exec", fixture.Path, stopOnEntry)
-		initEv := client.ExpectInitializedEvent(t)
-		if initEv.Seq != 0 {
-			t.Errorf("got %#v, want Seq=0", initEv)
+		initEvent := client.ExpectInitializedEvent(t)
+		if initEvent.Seq != 0 {
+			t.Errorf("\ngot %#v\nwant Seq=0", initEvent)
 		}
-
 		launchResp := client.ExpectLaunchResponse(t)
-		if launchResp.Seq != 0 || launchResp.RequestSeq != 1 {
-			t.Errorf("got %#v, want Seq=0, RequestSeq=1", launchResp)
+		if launchResp.Seq != 0 || launchResp.RequestSeq != 2 {
+			t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=2", launchResp)
+		}
+
+		// 3 >> setBreakpoints, << setBreakpoints
+		client.SetBreakpointsRequest(fixture.Source, nil)
+		sbpResp := client.ExpectSetBreakpointsResponse(t)
+		if sbpResp.Seq != 0 || sbpResp.RequestSeq != 3 || len(sbpResp.Body.Breakpoints) != 0 {
+			t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=3, len(Breakpoints)=0", sbpResp)
 		}
 
+		// 4 >> setExceptionBreakpoints, << setExceptionBreakpoints
 		client.SetExceptionBreakpointsRequest()
-		sResp := client.ExpectSetExceptionBreakpointsResponse(t)
-		if sResp.Seq != 0 || sResp.RequestSeq != 2 {
-			t.Errorf("got %#v, want Seq=0, RequestSeq=2", sResp)
+		sebpResp := client.ExpectSetExceptionBreakpointsResponse(t)
+		if sebpResp.Seq != 0 || sebpResp.RequestSeq != 4 {
+			t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=4", sebpResp)
 		}
 
+		// 5 >> configurationDone, << stopped, << configurationDone
 		client.ConfigurationDoneRequest()
 		stopEvent := client.ExpectStoppedEvent(t)
 		if stopEvent.Seq != 0 ||
 			stopEvent.Body.Reason != "breakpoint" ||
 			stopEvent.Body.ThreadId != 1 ||
 			!stopEvent.Body.AllThreadsStopped {
-			t.Errorf("got %#v, want Seq=0, Body={Reason=\"breakpoint\", ThreadId=1, AllThreadsStopped=true}", stopEvent)
+			t.Errorf("\ngot %#v\nwant Seq=0, Body={Reason=\"breakpoint\", ThreadId=1, AllThreadsStopped=true}", stopEvent)
 		}
-
 		cdResp := client.ExpectConfigurationDoneResponse(t)
-		if cdResp.Seq != 0 || cdResp.RequestSeq != 3 {
-			t.Errorf("got %#v, want Seq=0, RequestSeq=3", cdResp)
+		if cdResp.Seq != 0 || cdResp.RequestSeq != 5 {
+			t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=5", cdResp)
+		}
+
+		// 6 >> threads, << threads
+		client.ThreadsRequest()
+		tResp := client.ExpectThreadsResponse(t)
+		if tResp.Seq != 0 || tResp.RequestSeq != 6 || len(tResp.Body.Threads) != 1 {
+			t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=6 len(Threads)=1", tResp)
+		}
+		if tResp.Body.Threads[0].Id != 1 || tResp.Body.Threads[0].Name != "Dummy" {
+			t.Errorf("\ngot %#v\nwant Id=1, Name=\"Dummy\"", tResp)
 		}
 
+		// 7 >> threads, << threads
+		client.ThreadsRequest()
+		tResp = client.ExpectThreadsResponse(t)
+		if tResp.Seq != 0 || tResp.RequestSeq != 7 || len(tResp.Body.Threads) != 1 {
+			t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=7 len(Threads)=1", tResp)
+		}
+
+		// 8 >> stackTrace, << stackTrace
+		client.StackTraceRequest()
+		stResp := client.ExpectErrorResponse(t)
+		if stResp.Seq != 0 || stResp.RequestSeq != 8 || stResp.Message != "Unsupported command" {
+			t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=8 Message=\"Unsupported command\"", stResp)
+		}
+
+		// 9 >> stackTrace, << stackTrace
+		client.StackTraceRequest()
+		stResp = client.ExpectErrorResponse(t)
+		if stResp.Seq != 0 || stResp.RequestSeq != 9 || stResp.Message != "Unsupported command" {
+			t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=9 Message=\"Unsupported command\"", stResp)
+		}
+
+		// 10 >> continue, << continue, << terminated
 		client.ContinueRequest(1)
 		contResp := client.ExpectContinueResponse(t)
-		if contResp.Seq != 0 || contResp.RequestSeq != 4 {
-			t.Errorf("got %#v, want Seq=0, RequestSeq=4", contResp)
+		if contResp.Seq != 0 || contResp.RequestSeq != 10 {
+			t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=10", contResp)
+		}
+		termEvent := client.ExpectTerminatedEvent(t)
+		if termEvent.Seq != 0 {
+			t.Errorf("\ngot %#v\nwant Seq=0", termEvent)
 		}
 
-		termEv := client.ExpectTerminatedEvent(t)
-		if termEv.Seq != 0 {
-			t.Errorf("got %#v, want Seq=0", termEv)
+		// 11 >> disconnect, << disconnect
+		client.DisconnectRequest()
+		dResp := client.ExpectDisconnectResponse(t)
+		if dResp.Seq != 0 || dResp.RequestSeq != 11 {
+			t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=11", dResp)
 		}
+	})
+}
 
+// Like the test above, except the program is configured to continue on entry.
+func TestContinueOnEntry(t *testing.T) {
+	runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
+		// 1 >> initialize, << initialize
+		client.InitializeRequest()
+		client.ExpectInitializeResponse(t)
+
+		// 2 >> launch, << initialized, << launch
+		client.LaunchRequest("exec", fixture.Path, !stopOnEntry)
+		client.ExpectInitializedEvent(t)
+		client.ExpectLaunchResponse(t)
+
+		// 3 >> setBreakpoints, << setBreakpoints
+		client.SetBreakpointsRequest(fixture.Source, nil)
+		client.ExpectSetBreakpointsResponse(t)
+
+		// 4 >> setExceptionBreakpoints, << setExceptionBreakpoints
+		client.SetExceptionBreakpointsRequest()
+		client.ExpectSetExceptionBreakpointsResponse(t)
+
+		// 5 >> configurationDone, << configurationDone
+		client.ConfigurationDoneRequest()
+		client.ExpectConfigurationDoneResponse(t)
+		// "Continue" happens behind the scenes
+
+		// For now continue is blocking and runs until a stop or
+		// termination. But once we upgrade the server to be async,
+		// a simultaneous threads request can be made while continue
+		// is running. Note that vscode-go just keeps track of the
+		// continue state and would just return a dummy response
+		// without talking to debugger if continue was in progress.
+		// TODO(polina): test this once it is possible
+
+		client.ExpectTerminatedEvent(t)
+
+		// It is possible for the program to terminate before the initial
+		// threads request is processed.
+
+		// 6 >> threads, << threads
+		client.ThreadsRequest()
+		tResp := client.ExpectThreadsResponse(t)
+		if tResp.Seq != 0 || tResp.RequestSeq != 6 || len(tResp.Body.Threads) != 0 {
+			t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=6 len(Threads)=0", tResp)
+		}
+
+		// 7 >> disconnect, << disconnect
 		client.DisconnectRequest()
 		dResp := client.ExpectDisconnectResponse(t)
-		if dResp.Seq != 0 || dResp.RequestSeq != 5 {
-			t.Errorf("got %#v, want Seq=0, RequestSeq=5", dResp)
+		if dResp.Seq != 0 || dResp.RequestSeq != 7 {
+			t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=7", dResp)
 		}
 	})
 }
 
+// TestSetBreakpoint corresponds to a debug session that is configured to
+// continue on entry with a pre-set breakpoint.
 func TestSetBreakpoint(t *testing.T) {
 	runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
 		client.InitializeRequest()
@@ -133,10 +259,7 @@ func TestSetBreakpoint(t *testing.T) {
 
 		client.LaunchRequest("exec", fixture.Path, !stopOnEntry)
 		client.ExpectInitializedEvent(t)
-		launchResp := client.ExpectLaunchResponse(t)
-		if launchResp.RequestSeq != 1 {
-			t.Errorf("got %#v, want RequestSeq=1", launchResp)
-		}
+		client.ExpectLaunchResponse(t)
 
 		client.SetBreakpointsRequest(fixture.Source, []int{8, 100})
 		sResp := client.ExpectSetBreakpointsResponse(t)
@@ -152,31 +275,50 @@ func TestSetBreakpoint(t *testing.T) {
 		client.ExpectSetExceptionBreakpointsResponse(t)
 
 		client.ConfigurationDoneRequest()
-		cdResp := client.ExpectConfigurationDoneResponse(t)
-		if cdResp.RequestSeq != 4 {
-			t.Errorf("got %#v, want RequestSeq=4", cdResp)
-		}
+		client.ExpectConfigurationDoneResponse(t)
+		// This triggers "continue"
+
+		// TODO(polina): add a no-op threads request
+		// with dummy response here once server becomes async
+		// to match what happens in VS Code.
 
-		client.ContinueRequest(1)
 		stopEvent1 := client.ExpectStoppedEvent(t)
 		if stopEvent1.Body.Reason != "breakpoint" ||
 			stopEvent1.Body.ThreadId != 1 ||
 			!stopEvent1.Body.AllThreadsStopped {
 			t.Errorf("got %#v, want Body={Reason=\"breakpoint\", ThreadId=1, AllThreadsStopped=true}", stopEvent1)
 		}
-		client.ExpectContinueResponse(t)
+
+		client.ThreadsRequest()
+		tResp := client.ExpectThreadsResponse(t)
+		if len(tResp.Body.Threads) < 2 { // 1 main + runtime
+			t.Errorf("\ngot  %#v\nwant len(Threads)>1", tResp.Body.Threads)
+		}
+		// TODO(polina): can we reliably test for these values?
+		wantMain := dap.Thread{Id: 1, Name: "main.Increment"}
+		wantRuntime := dap.Thread{Id: 2, Name: "runtime.gopark"}
+		for _, got := range tResp.Body.Threads {
+			if !reflect.DeepEqual(got, wantMain) && !strings.HasPrefix(got.Name, "runtime") {
+				t.Errorf("\ngot  %#v\nwant []dap.Thread{%#v, %#v, ...}", tResp.Body.Threads, wantMain, wantRuntime)
+			}
+		}
+
+		// TODO(polina): add other status checking requests
+		// that are not yet supported (stackTrace, scopes, variables)
 
 		client.ContinueRequest(1)
-		client.ExpectTerminatedEvent(t)
 		client.ExpectContinueResponse(t)
+		// "Continue" is triggered after the response is sent
 
+		client.ExpectTerminatedEvent(t)
 		client.DisconnectRequest()
 		client.ExpectDisconnectResponse(t)
 	})
 }
 
 // runDebugSesion is a helper for executing the standard init and shutdown
-// sequences while specifying unique launch criteria via parameters.
+// sequences for a program that does not stop on entry
+// while specifying unique launch criteria via parameters.
 func runDebugSession(t *testing.T, client *daptest.Client, launchRequest func()) {
 	client.InitializeRequest()
 	client.ExpectInitializeResponse(t)
@@ -185,9 +327,14 @@ func runDebugSession(t *testing.T, client *daptest.Client, launchRequest func())
 	client.ExpectInitializedEvent(t)
 	client.ExpectLaunchResponse(t)
 
+	// Skip no-op setBreakpoints
+	// Skip no-op setExceptionBreakpoints
+
 	client.ConfigurationDoneRequest()
 	client.ExpectConfigurationDoneResponse(t)
 
+	// Program automatically continues to completion
+
 	client.ExpectTerminatedEvent(t)
 	client.DisconnectRequest()
 	client.ExpectDisconnectResponse(t)
@@ -218,7 +365,7 @@ func TestLaunchTestRequest(t *testing.T) {
 
 func TestBadLaunchRequests(t *testing.T) {
 	runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
-		seqCnt := 0
+		seqCnt := 1
 		expectFailedToLaunch := func(response *dap.ErrorResponse) {
 			t.Helper()
 			if response.RequestSeq != seqCnt {