Repository: go-delve/delve
Pull request #2099 (2020-07-07 17:32:22)

service/dap: Add launch request support for stackTraceDepth arg
Introduces a new struct type to track all launch arguments.
Reworks the test code to reduce duplicate boilerplate request sequences.

Updates #1515


Patch:
diff --git a/service/dap/server.go b/service/dap/server.go
index 8cac9d32..59a11b2f 100644
--- a/service/dap/server.go
+++ b/service/dap/server.go
@@ -52,12 +52,27 @@ type Server struct {
 	debugger *debugger.Debugger
 	// log is used for structured logging.
 	log *logrus.Entry
-	// stopOnEntry is set to automatically stop the debugee after start.
-	stopOnEntry bool
 	// binaryToRemove is the compiled binary to be removed on disconnect.
 	binaryToRemove string
 	// stackFrameHandles keep track of unique ids created across all goroutines.
 	stackFrameHandles *handlesMap
+	// args tracks special settings for handling debug session requests.
+	args launchAttachArgs
+}
+
+// launchAttachArgs captures arguments from launch/attach request that
+// impact handling of subsequent requests.
+type launchAttachArgs struct {
+	// stopOnEntry is set to automatically stop the debugee after start.
+	stopOnEntry bool
+	// stackTraceDepth is the maximum length of the returned list of stack frames.
+	stackTraceDepth int
+}
+
+// defaultArgs borrows the defaults for the arguments from the original vscode-go adapter.
+var defaultArgs = launchAttachArgs{
+	stopOnEntry:     false,
+	stackTraceDepth: 50,
 }
 
 // NewServer creates a new DAP Server. It takes an opened Listener
@@ -74,6 +89,7 @@ func NewServer(config *service.Config) *Server {
 		stopChan:          make(chan struct{}),
 		log:               logger,
 		stackFrameHandles: newHandlesMap(),
+		args:              defaultArgs,
 	}
 }
 
@@ -441,7 +457,12 @@ func (s *Server) onLaunchRequest(request *dap.LaunchRequest) {
 	}
 
 	stop, ok := request.Arguments["stopOnEntry"]
-	s.stopOnEntry = ok && stop == true
+	s.args.stopOnEntry = ok && stop == true
+
+	depth, ok := request.Arguments["stackTraceDepth"].(float64)
+	if ok && depth > 0 {
+		s.args.stackTraceDepth = int(depth)
+	}
 
 	var targetArgs []string
 	args, ok := request.Arguments["args"]
@@ -534,7 +555,7 @@ func (s *Server) onSetExceptionBreakpointsRequest(request *dap.SetExceptionBreak
 }
 
 func (s *Server) onConfigurationDoneRequest(request *dap.ConfigurationDoneRequest) {
-	if s.stopOnEntry {
+	if s.args.stopOnEntry {
 		e := &dap.StoppedEvent{
 			Event: *newEvent("stopped"),
 			Body:  dap.StoppedEventBody{Reason: "entry", ThreadId: 1, AllThreadsStopped: true},
@@ -542,7 +563,7 @@ func (s *Server) onConfigurationDoneRequest(request *dap.ConfigurationDoneReques
 		s.send(e)
 	}
 	s.send(&dap.ConfigurationDoneResponse{Response: *newResponse(request.Request)})
-	if !s.stopOnEntry {
+	if !s.args.stopOnEntry {
 		s.doContinue()
 	}
 }
@@ -636,11 +657,7 @@ type stackFrame struct {
 // This is a mandatory request to support.
 func (s *Server) onStackTraceRequest(request *dap.StackTraceRequest) {
 	goroutineID := request.Arguments.ThreadId
-	// Borrow the defaults for the arguments from the original vscode-go adapter.
-	// TODO(polina): is 'startFrame'+'levels' a better alternative for depth?
-	// TODO(polina): support setting depth via launch/attach args
-	depth := 50 // length of the returned list of Stackframes
-	locs, err := s.debugger.Stacktrace(goroutineID, depth, 0, nil /*skip locals & args*/)
+	locs, err := s.debugger.Stacktrace(goroutineID, s.args.stackTraceDepth, 0, nil /*skip locals & args*/)
 	if err != nil {
 		s.sendErrorResponse(request.Request, UnableToProduceStackTrace, "Unable to produce stack trace", err.Error())
 		return


Test patch:
diff --git a/service/dap/server_test.go b/service/dap/server_test.go
index 3ed2b120..4223cc6b 100644
--- a/service/dap/server_test.go
+++ b/service/dap/server_test.go
@@ -315,21 +315,21 @@ func TestSetBreakpoint(t *testing.T) {
 		if len(stResp.Body.StackFrames) != 6 {
 			t.Errorf("\ngot %#v\nwant len(StackFrames)=6", stResp.Body.StackFrames)
 		} else {
-			expectStackFrame := func(got dap.StackFrame, id int, name string, sourceName string, line int) {
+			expectFrame := func(got dap.StackFrame, id int, name string, sourceName string, line int) {
 				t.Helper()
 				if got.Id != id || got.Name != name {
-					t.Errorf("\ngot  %#v\nwant id=%d name=%s", got, id, name)
+					t.Errorf("\ngot  %#v\nwant Id=%d Name=%s", got, id, name)
 				}
 				if (sourceName != "" && got.Source.Name != sourceName) || (line > 0 && got.Line != line) {
-					t.Errorf("\ngot  %#v\nwant source.name=%s line=%d", got, sourceName, line)
+					t.Errorf("\ngot  %#v\nwant Source.Name=%s Line=%d", got, sourceName, line)
 				}
 			}
-			expectStackFrame(stResp.Body.StackFrames[0], 1000, "main.Increment", "increment.go", 8)
-			expectStackFrame(stResp.Body.StackFrames[1], 1001, "main.Increment", "increment.go", 11)
-			expectStackFrame(stResp.Body.StackFrames[2], 1002, "main.Increment", "increment.go", 11)
-			expectStackFrame(stResp.Body.StackFrames[3], 1003, "main.main", "increment.go", 17)
-			expectStackFrame(stResp.Body.StackFrames[4], 1004, "runtime.main", "proc.go", -1)
-			expectStackFrame(stResp.Body.StackFrames[5], 1005, "runtime.goexit", "", -1)
+			expectFrame(stResp.Body.StackFrames[0], 1000, "main.Increment", "increment.go", 8)
+			expectFrame(stResp.Body.StackFrames[1], 1001, "main.Increment", "increment.go", 11)
+			expectFrame(stResp.Body.StackFrames[2], 1002, "main.Increment", "increment.go", 11)
+			expectFrame(stResp.Body.StackFrames[3], 1003, "main.main", "increment.go", 17)
+			expectFrame(stResp.Body.StackFrames[4], 1004, "runtime.main", "proc.go", -1)
+			expectFrame(stResp.Body.StackFrames[5], 1005, "runtime.goexit", "", -1)
 		}
 
 		// TODO(polina): add other status checking requests
@@ -345,115 +345,144 @@ func TestSetBreakpoint(t *testing.T) {
 	})
 }
 
+// expectStackFrames is a helper for verifying the values within StackTraceResponse.
+//     wantStartLine - file line of the first returned frame (non-positive values are ignored).
+//     wantStartID - id of the first frame returned (ignored if wantFrames is 0).
+//     wantFrames - number of frames returned.
+//     wantTotalFrames - total number of stack frames (StackTraceResponse.Body.TotalFrames).
+func expectStackFrames(t *testing.T, got *dap.StackTraceResponse,
+	wantStartLine, wantStartID, wantFrames, wantTotalFrames int) {
+	t.Helper()
+	if got.Body.TotalFrames != wantTotalFrames {
+		t.Errorf("\ngot  %#v\nwant TotalFrames=%d", got.Body.TotalFrames, wantTotalFrames)
+	}
+	if len(got.Body.StackFrames) != wantFrames {
+		t.Errorf("\ngot  len(StackFrames)=%d\nwant %d", len(got.Body.StackFrames), wantFrames)
+	} else {
+		// Verify that frame ids are consecutive numbers starting at wantStartID
+		for i := 0; i < wantFrames; i++ {
+			if got.Body.StackFrames[i].Id != wantStartID+i {
+				t.Errorf("\ngot  %#v\nwant Id=%d", got.Body.StackFrames[i], wantStartID+i)
+			}
+		}
+		// Verify the line corresponding to the first returned frame (if any).
+		// This is useful when the first frame is the frame corresponding to the breakpoint at
+		// a predefined line. Values < 0 are a signal to skip the check (which can be useful
+		// for frames in the third-party code, where we do not control the lines).
+		if wantFrames > 0 && wantStartLine > 0 && got.Body.StackFrames[0].Line != wantStartLine {
+			t.Errorf("\ngot  Line=%d\nwant %d", got.Body.StackFrames[0].Line, wantStartLine)
+		}
+	}
+}
+
 // TestStackTraceRequest executes to a breakpoint (similarly to TestSetBreakpoint
 // that includes more thorough checking of that sequence) and tests different
 // good and bad configurations of 'stackTrace' requests.
 func TestStackTraceRequest(t *testing.T) {
 	runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
-		client.InitializeRequest()
-		client.ExpectInitializeResponse(t)
-
-		client.LaunchRequest("exec", fixture.Path, !stopOnEntry)
-		client.ExpectInitializedEvent(t)
-		client.ExpectLaunchResponse(t)
-
-		client.SetBreakpointsRequest(fixture.Source, []int{8, 18})
-		client.ExpectSetBreakpointsResponse(t)
-
-		client.ConfigurationDoneRequest()
-		client.ExpectConfigurationDoneResponse(t)
-		// This triggers "continue"
-
 		var stResp *dap.StackTraceResponse
-		var expectFrames = func(wantStart int, wantFrames int, wantTotal int, wantLine int) {
-			t.Helper()
-			if stResp.Body.TotalFrames != wantTotal {
-				t.Errorf("\ngot  %#v\nwant TotalFrames=%d", stResp.Body.TotalFrames, wantTotal)
-			}
-			if len(stResp.Body.StackFrames) != wantFrames {
-				t.Errorf("\ngot  len=%d\nwant len=%d", len(stResp.Body.StackFrames), wantFrames)
-			} else {
-				for i := 0; i < wantFrames; i++ {
-					if stResp.Body.StackFrames[i].Id != wantStart+i {
-						t.Errorf("\ngot  %#v\nwant id=%d", stResp.Body.StackFrames[i], wantStart+i)
-					}
-				}
-				if wantFrames > 0 && wantLine > 0 && stResp.Body.StackFrames[0].Line != wantLine {
-					t.Errorf("\ngot  line=%d\nwant line=%d", stResp.Body.StackFrames[0].Line, wantLine)
-				}
-			}
-		}
-
-		client.ExpectStoppedEvent(t) // at line 8
-
-		client.StackTraceRequest(1, 0, 0)
-		stResp = client.ExpectStackTraceResponse(t)
-		expectFrames(1000, 6, 6, 8)
-
-		// Even though the stack frames are the same,
-		// repeated requests at the same breakpoint,
-		// would assign unique ids to them each time.
-		client.StackTraceRequest(1, -1, 0)
-		stResp = client.ExpectStackTraceResponse(t) // Or should this return an ErrorResponse?
-		expectFrames(1006, 6, 6, 8)
-
-		client.StackTraceRequest(1, 3, 0)
-		stResp = client.ExpectStackTraceResponse(t)
-		expectFrames(1015, 3, 6, 17)
-
-		client.StackTraceRequest(1, 6, 0)
-		stResp = client.ExpectStackTraceResponse(t)
-		expectFrames(0, 0, 6, 0)
-
-		client.StackTraceRequest(1, 7, 0)
-		stResp = client.ExpectStackTraceResponse(t) // Or should this return an ErrorResponse?
-		expectFrames(0, 0, 6, 0)
-
-		client.ContinueRequest(1)
-		client.ExpectContinueResponse(t)
-		// "Continue" is triggered after the response is sent
-
-		client.ExpectStoppedEvent(t) // at line 18
-
-		// Frame ids get reset at each breakpoint.
-		client.StackTraceRequest(1, 0, 0)
-		stResp = client.ExpectStackTraceResponse(t)
-		expectFrames(1000, 3, 3, 18)
-
-		client.StackTraceRequest(1, 0, -1) // Or should this return an ErrorResponse?
-		stResp = client.ExpectStackTraceResponse(t)
-		expectFrames(1003, 3, 3, 18)
-
-		client.StackTraceRequest(1, 0, 2)
-		stResp = client.ExpectStackTraceResponse(t)
-		expectFrames(1006, 2, 3, 18)
-
-		client.StackTraceRequest(1, 0, 3)
-		stResp = client.ExpectStackTraceResponse(t)
-		expectFrames(1009, 3, 3, 18)
-
-		client.StackTraceRequest(1, 0, 4)
-		stResp = client.ExpectStackTraceResponse(t)
-		expectFrames(1012, 3, 3, 18)
-
-		client.StackTraceRequest(1, 1, 2)
-		stResp = client.ExpectStackTraceResponse(t)
-		expectFrames(1016, 2, 3, -1) // Don't test for runtime line we don't control.
-
-		client.ContinueRequest(1)
-		client.ExpectContinueResponse(t)
-		// "Continue" is triggered after the response is sent
+		runDebugSessionWithBPs(t, client,
+			// Launch
+			func() {
+				client.LaunchRequest("exec", fixture.Path, !stopOnEntry)
+			},
+			// Set breakpoints
+			fixture.Source, []int{8, 18},
+			[]func(){
+				// Stop at line 8
+				func() {
+					t.Helper()
+					client.StackTraceRequest(1, 0, 0)
+					stResp = client.ExpectStackTraceResponse(t)
+					expectStackFrames(t, stResp, 8, 1000, 6, 6)
+
+					// Even though the stack frames are the same,
+					// repeated requests at the same breakpoint,
+					// would assign unique ids to them each time.
+					client.StackTraceRequest(1, -100, 0) // Negative startFrame is treated as 0
+					stResp = client.ExpectStackTraceResponse(t)
+					expectStackFrames(t, stResp, 8, 1006, 6, 6)
+
+					client.StackTraceRequest(1, 3, 0)
+					stResp = client.ExpectStackTraceResponse(t)
+					expectStackFrames(t, stResp, 17, 1015, 3, 6)
+
+					client.StackTraceRequest(1, 6, 0)
+					stResp = client.ExpectStackTraceResponse(t)
+					expectStackFrames(t, stResp, -1, -1, 0, 6)
+
+					client.StackTraceRequest(1, 7, 0) // Out of bounds startFrame is capped at len
+					stResp = client.ExpectStackTraceResponse(t)
+					expectStackFrames(t, stResp, -1, -1, 0, 6)
+				},
+				// Stop at line 18
+				func() {
+					t.Helper()
+					// Frame ids get reset at each breakpoint.
+					client.StackTraceRequest(1, 0, 0)
+					stResp = client.ExpectStackTraceResponse(t)
+					expectStackFrames(t, stResp, 18, 1000, 3, 3)
+
+					client.StackTraceRequest(1, 0, -100) // Negative levels is treated as 0
+					stResp = client.ExpectStackTraceResponse(t)
+					expectStackFrames(t, stResp, 18, 1003, 3, 3)
+
+					client.StackTraceRequest(1, 0, 2)
+					stResp = client.ExpectStackTraceResponse(t)
+					expectStackFrames(t, stResp, 18, 1006, 2, 3)
+
+					client.StackTraceRequest(1, 0, 3)
+					stResp = client.ExpectStackTraceResponse(t)
+					expectStackFrames(t, stResp, 18, 1009, 3, 3)
+
+					client.StackTraceRequest(1, 0, 4) // Out of bounds levels is capped at len
+					stResp = client.ExpectStackTraceResponse(t)
+					expectStackFrames(t, stResp, 18, 1012, 3, 3)
+
+					client.StackTraceRequest(1, 1, 2)
+					stResp = client.ExpectStackTraceResponse(t)
+					expectStackFrames(t, stResp, -1, 1016, 2, 3) // Don't test for runtime line we don't control
+				}})
+	})
+}
 
-		client.ExpectTerminatedEvent(t)
-		client.DisconnectRequest()
-		client.ExpectDisconnectResponse(t)
+// Tests that 'stackTraceDepth' from LaunchRequest is parsed and passed to
+// stacktrace requests handlers.
+func TestLaunchRequestWithStackTraceDepth(t *testing.T) {
+	runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
+		var stResp *dap.StackTraceResponse
+		runDebugSessionWithBPs(t, client,
+			// Launch
+			func() {
+				client.LaunchRequestWithArgs(map[string]interface{}{
+					"mode": "exec", "program": fixture.Path, "stackTraceDepth": 1,
+				})
+			},
+			// Set breakpoints
+			fixture.Source, []int{8},
+			[]func(){
+				// Stop at line 8
+				func() {
+					t.Helper()
+					client.StackTraceRequest(1, 0, 0)
+					stResp = client.ExpectStackTraceResponse(t)
+					expectStackFrames(t, stResp, 8, 1000, 2, 2)
+				}})
 	})
 }
 
-// runDebugSesion is a helper for executing the standard init and shutdown
+// runDebugSesionWithBPs is a helper for executing the common init and shutdown
 // 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()) {
+// while specifying breakpoints and unique launch criteria via parameters.
+//     launchRequest - a function that sends a launch request, so the test author
+//                     has full control of its arguments. Note that he rest of
+//                     the test sequence assumes that stopOneEntry is false.
+//     breakpoints   - list of lines, where breakpoints are to be set
+//     onBreakpoints - list of functions to be called at each of the above breakpoints.
+//                     These can be used to simulate additional editor-driven or
+//                     user-driven requests that could occur at each stopped breakpoint.
+func runDebugSessionWithBPs(t *testing.T, client *daptest.Client, launchRequest func(), source string, breakpoints []int, onBreakpoints []func()) {
+	t.Helper()
 	client.InitializeRequest()
 	client.ExpectInitializeResponse(t)
 
@@ -461,19 +490,36 @@ func runDebugSession(t *testing.T, client *daptest.Client, launchRequest func())
 	client.ExpectInitializedEvent(t)
 	client.ExpectLaunchResponse(t)
 
-	// Skip no-op setBreakpoints
+	client.SetBreakpointsRequest(source, breakpoints)
+	client.ExpectSetBreakpointsResponse(t)
+
 	// Skip no-op setExceptionBreakpoints
 
 	client.ConfigurationDoneRequest()
 	client.ExpectConfigurationDoneResponse(t)
 
-	// Program automatically continues to completion
+	// Program automatically continues to breakpoint or completion
+
+	for _, onBP := range onBreakpoints {
+		client.ExpectStoppedEvent(t)
+		onBP()
+		client.ContinueRequest(1)
+		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 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()) {
+	runDebugSessionWithBPs(t, client, launchRequest, "", nil, nil)
+}
+
 func TestLaunchDebugRequest(t *testing.T) {
 	runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
 		// We reuse the harness that builds, but ignore the built binary,