service/dap: make next while nexting error more clear (#2622)

To make it more clear that the user can resume the program when they encounter a next while nexting error, make the exception information have instructions for resuming the program. This implements the suggestions outlined by @polinasok in #2443.

service/dap/server.go

@@ -2656,6 +2656,7 @@ func (s *Server) onExceptionInfoRequest(request *dap.ExceptionInfoRequest) {
 		bpState = g.Thread.Breakpoint()
 	}
 	// Check if this goroutine ID is stopped at a breakpoint.
+	includeStackTrace := true
 	if bpState != nil && bpState.Breakpoint != nil && (bpState.Breakpoint.Name == proc.FatalThrow || bpState.Breakpoint.Name == proc.UnrecoveredPanic) {
 		switch bpState.Breakpoint.Name {
 		case proc.FatalThrow:
@@ -2691,7 +2692,7 @@ func (s *Server) onExceptionInfoRequest(request *dap.ExceptionInfoRequest) {
 			s.sendErrorResponse(request.Request, UnableToGetExceptionInfo, "Unable to get exception info", err.Error())
 			return
 		}
-		if state == nil || state.CurrentThread == nil || g.Thread == nil || state.CurrentThread.ID != g.Thread.ThreadID() {
+		if s.exceptionErr.Error() != "next while nexting" && (state == nil || state.CurrentThread == nil || g.Thread == nil || state.CurrentThread.ID != g.Thread.ThreadID()) {
 			s.sendErrorResponse(request.Request, UnableToGetExceptionInfo, "Unable to get exception info", fmt.Sprintf("no exception found for goroutine %d", goroutineID))
 			return
 		}
@@ -2700,28 +2701,20 @@ func (s *Server) onExceptionInfoRequest(request *dap.ExceptionInfoRequest) {
 		if body.Description == "bad access" {
 			body.Description = BetterBadAccessError
 		}
+		if body.Description == "next while nexting" {
+			body.ExceptionId = "invalid command"
+			body.Description = BetterNextWhileNextingError
+			includeStackTrace = false
+		}
 	}
 
-	frames, err := s.debugger.Stacktrace(goroutineID, s.args.stackTraceDepth, 0)
+	if includeStackTrace {
-	if err == nil {
+		frames, err := s.stacktrace(goroutineID, g)
-		apiFrames, err := s.debugger.ConvertStacktrace(frames, nil)
+		if err != nil {
-		if err == nil {
+			body.Details.StackTrace = fmt.Sprintf("Error getting stack trace: %s", err.Error())
-			var buf bytes.Buffer
+		} else {
-			fmt.Fprintln(&buf, "Stack:")
+			body.Details.StackTrace = frames
-			userLoc := g.UserCurrent()
-			userFuncPkg := fnPackageName(&userLoc)
-			api.PrintStack(s.toClientPath, &buf, apiFrames, "\t", false, func(s api.Stackframe) bool {
-				// Include all stack frames if the stack trace is for a system goroutine,
-				// otherwise, skip runtime stack frames.
-				if userFuncPkg == "runtime" {
-					return true
-				}
-				return s.Location.Function != nil && !strings.HasPrefix(s.Location.Function.Name(), "runtime.")
-			})
-			body.Details.StackTrace = buf.String()
 		}
-	} else {
-		body.Details.StackTrace = fmt.Sprintf("Error getting stack trace: %s", err.Error())
 	}
 	response := &dap.ExceptionInfoResponse{
 		Response: *newResponse(request.Request),
@@ -2730,6 +2723,31 @@ func (s *Server) onExceptionInfoRequest(request *dap.ExceptionInfoRequest) {
 	s.send(response)
 }
 
+func (s *Server) stacktrace(goroutineID int, g *proc.G) (string, error) {
+	frames, err := s.debugger.Stacktrace(goroutineID, s.args.stackTraceDepth, 0)
+	if err != nil {
+		return "", err
+	}
+	apiFrames, err := s.debugger.ConvertStacktrace(frames, nil)
+	if err != nil {
+		return "", err
+	}
+
+	var buf bytes.Buffer
+	fmt.Fprintln(&buf, "Stack:")
+	userLoc := g.UserCurrent()
+	userFuncPkg := fnPackageName(&userLoc)
+	api.PrintStack(s.toClientPath, &buf, apiFrames, "\t", false, func(s api.Stackframe) bool {
+		// Include all stack frames if the stack trace is for a system goroutine,
+		// otherwise, skip runtime stack frames.
+		if userFuncPkg == "runtime" {
+			return true
+		}
+		return s.Location.Function != nil && !strings.HasPrefix(s.Location.Function.Name(), "runtime.")
+	})
+	return buf.String(), nil
+}
+
 func (s *Server) throwReason(goroutineID int) (string, error) {
 	return s.getExprString("s", goroutineID, 0)
 }
@@ -2819,6 +2837,8 @@ func newEvent(event string) *dap.Event {
 
 const BetterBadAccessError = `invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation]
 Unable to propagate EXC_BAD_ACCESS signal to target process and panic (see https://github.com/go-delve/delve/issues/852)`
+const BetterNextWhileNextingError = `Unable to step while the previous step is interrupted by a breakpoint.
+Use 'Continue' to resume the original step command.`
 
 func (s *Server) resetHandlesForStoppedEvent() {
 	s.stackFrameHandles.reset()
@@ -2903,6 +2923,12 @@ func (s *Server) doRunCommand(command string, asyncSetupDone chan struct{}) {
 		if stopped.Body.Text == "bad access" {
 			stopped.Body.Text = BetterBadAccessError
 		}
+		if stopped.Body.Text == "next while nexting" {
+			stopped.Body.Description = "invalid command"
+			stopped.Body.Text = BetterNextWhileNextingError
+			s.logToConsole(fmt.Sprintf("%s: %s", stopped.Body.Description, stopped.Body.Text))
+		}
+
 		state, err := s.debugger.State( /*nowait*/ true)
 		if err == nil {
 			stopped.Body.ThreadId = stoppedGoroutineID(state)
@@ -2913,6 +2939,11 @@ func (s *Server) doRunCommand(command string, asyncSetupDone chan struct{}) {
 	// error while this one completes, it is possible that the error response
 	// will arrive after this stopped event.
 	s.send(stopped)
+
+	// Send an output event with more information if next is in progress.
+	if state != nil && state.NextInProgress {
+		s.logToConsole("Step interrupted by a breakpoint. Use 'Continue' to resume the original step command.")
+	}
 }
 
 func (s *Server) toClientPath(path string) string {

service/dap/server_test.go

@@ -3548,6 +3548,22 @@ func TestStepOutPreservesGoroutine(t *testing.T) {
 			}})
 	})
 }
+func checkStopOnNextWhileNextingError(t *testing.T, client *daptest.Client, threadID int) {
+	t.Helper()
+	oe := client.ExpectOutputEvent(t)
+	if oe.Body.Category != "console" || oe.Body.Output != fmt.Sprintf("invalid command: %s\n", BetterNextWhileNextingError) {
+		t.Errorf("\ngot  %#v\nwant Category=\"console\" Output=\"invalid command: %s\\n\"", oe, BetterNextWhileNextingError)
+	}
+	se := client.ExpectStoppedEvent(t)
+	if se.Body.ThreadId != threadID || se.Body.Reason != "exception" || se.Body.Description != "invalid command" || se.Body.Text != BetterNextWhileNextingError {
+		t.Errorf("\ngot  %#v\nwant ThreadId=%d Reason=\"exception\" Description=\"invalid command\" Text=\"%s\"", se, threadID, BetterNextWhileNextingError)
+	}
+	client.ExceptionInfoRequest(1)
+	eInfo := client.ExpectExceptionInfoResponse(t)
+	if eInfo.Body.ExceptionId != "invalid command" || eInfo.Body.Description != BetterNextWhileNextingError {
+		t.Errorf("\ngot  %#v\nwant ExceptionId=\"invalid command\" Text=\"%s\"", eInfo, BetterNextWhileNextingError)
+	}
+}
 
 func TestBadAccess(t *testing.T) {
 	if runtime.GOOS != "darwin" || testBackend != "lldb" {
@@ -3588,15 +3604,99 @@ func TestBadAccess(t *testing.T) {
 
 					client.NextRequest(1)
 					client.ExpectNextResponse(t)
-					expectStoppedOnError("next while nexting")
+					checkStopOnNextWhileNextingError(t, client, 1)
 
 					client.StepInRequest(1)
 					client.ExpectStepInResponse(t)
-					expectStoppedOnError("next while nexting")
+					checkStopOnNextWhileNextingError(t, client, 1)
 
 					client.StepOutRequest(1)
 					client.ExpectStepOutResponse(t)
-					expectStoppedOnError("next while nexting")
+					checkStopOnNextWhileNextingError(t, client, 1)
+				},
+				disconnect: true,
+			}})
+	})
+}
+
+// TestNextWhileNexting is inspired by command_test.TestIssue387 and tests
+// that when 'next' is interrupted by a 'breakpoint', calling 'next'
+// again will produce an error with a helpful message, and 'continue'
+// will resume the program.
+func TestNextWhileNexting(t *testing.T) {
+	if runtime.GOOS == "freebsd" {
+		t.Skip("test is not valid on FreeBSD")
+	}
+	// a breakpoint triggering during a 'next' operation will interrupt 'next''
+	// Unlike the test for the terminal package, we cannot be certain
+	// of the number of breakpoints we expect to hit, since multiple
+	// breakpoints being hit at the same time is not supported in dap stopped
+	// events.
+	runTest(t, "issue387", func(client *daptest.Client, fixture protest.Fixture) {
+		runDebugSessionWithBPs(t, client, "launch",
+			// Launch
+			func() {
+				client.LaunchRequest("exec", fixture.Path, !stopOnEntry)
+			},
+			// Set breakpoints
+			fixture.Source, []int{15},
+			[]onBreakpoint{{ // Stop at line 15
+				execute: func() {
+					checkStop(t, client, 1, "main.main", 15)
+
+					client.SetBreakpointsRequest(fixture.Source, []int{8})
+					client.ExpectSetBreakpointsResponse(t)
+
+					client.ContinueRequest(1)
+					client.ExpectContinueResponse(t)
+
+					bpSe := client.ExpectStoppedEvent(t)
+					threadID := bpSe.Body.ThreadId
+					checkStop(t, client, threadID, "main.dostuff", 8)
+
+					for pos := 9; pos < 11; pos++ {
+						client.NextRequest(threadID)
+						client.ExpectNextResponse(t)
+
+						stepInProgress := true
+						for stepInProgress {
+							m := client.ExpectStoppedEvent(t)
+							switch m.Body.Reason {
+							case "step":
+								if !m.Body.AllThreadsStopped {
+									t.Errorf("got %#v, want Reason=\"step\", AllThreadsStopped=true", m)
+								}
+								checkStop(t, client, m.Body.ThreadId, "main.dostuff", pos)
+								stepInProgress = false
+							case "breakpoint":
+								if !m.Body.AllThreadsStopped {
+									t.Errorf("got %#v, want Reason=\"breakpoint\", AllThreadsStopped=true", m)
+								}
+
+								if stepInProgress {
+									// We encountered a breakpoint on a different thread. We should have to resume execution
+									// using continue.
+									oe := client.ExpectOutputEvent(t)
+									if oe.Body.Category != "console" || !strings.Contains(oe.Body.Output, "Step interrupted by a breakpoint.") {
+										t.Errorf("\ngot  %#v\nwant Category=\"console\" Output=\"Step interrupted by a breakpoint.\"", oe)
+									}
+									client.NextRequest(m.Body.ThreadId)
+									client.ExpectNextResponse(t)
+									checkStopOnNextWhileNextingError(t, client, m.Body.ThreadId)
+									// Continue since we have not finished the step request.
+									client.ContinueRequest(threadID)
+									client.ExpectContinueResponse(t)
+								} else {
+									checkStop(t, client, m.Body.ThreadId, "main.dostuff", 8)
+									// Switch to stepping on this thread instead.
+									pos = 8
+									threadID = m.Body.ThreadId
+								}
+							default:
+								t.Fatalf("got %#v, want StoppedEvent on step or breakpoint", m)
+							}
+						}
+					}
 				},
 				disconnect: true,
 			}})