Repository: go-delve/delve
Pull request #2482 (2021-05-14 22:56:39)

service/dap: move build error to output event
Since the build error may be long, including it in the error response may not be a good way to get the info to the user. This change moves that information to an output event. It also includes the build command that produced the error.

Updates golang/vscode-go#1425


Patch:
diff --git a/pkg/gobuild/gobuild.go b/pkg/gobuild/gobuild.go
index 0f3cc051..bf571180 100644
--- a/pkg/gobuild/gobuild.go
+++ b/pkg/gobuild/gobuild.go
@@ -7,6 +7,7 @@ import (
 	"os"
 	"os/exec"
 	"runtime"
+	"strings"
 	"time"
 
 	"github.com/go-delve/delve/pkg/config"
@@ -60,7 +61,7 @@ func GoBuild(debugname string, pkgs []string, buildflags string) error {
 
 // GoBuildCombinedOutput builds non-test files in 'pkgs' with the specified 'buildflags'
 // and writes the output at 'debugname'.
-func GoBuildCombinedOutput(debugname string, pkgs []string, buildflags string) ([]byte, error) {
+func GoBuildCombinedOutput(debugname string, pkgs []string, buildflags string) (string, []byte, error) {
 	args := goBuildArgs(debugname, pkgs, buildflags, false)
 	return gocommandCombinedOutput("build", args...)
 }
@@ -74,7 +75,7 @@ func GoTestBuild(debugname string, pkgs []string, buildflags string) error {
 
 // GoTestBuildCombinedOutput builds test files 'pkgs' with the specified 'buildflags'
 // and writes the output at 'debugname'.
-func GoTestBuildCombinedOutput(debugname string, pkgs []string, buildflags string) ([]byte, error) {
+func GoTestBuildCombinedOutput(debugname string, pkgs []string, buildflags string) (string, []byte, error) {
 	args := goBuildArgs(debugname, pkgs, buildflags, true)
 	return gocommandCombinedOutput("test", args...)
 }
@@ -93,19 +94,21 @@ func goBuildArgs(debugname string, pkgs []string, buildflags string, isTest bool
 }
 
 func gocommandRun(command string, args ...string) error {
-	goBuild := gocommandExecCmd(command, args...)
+	_, goBuild := gocommandExecCmd(command, args...)
 	goBuild.Stderr = os.Stdout
 	goBuild.Stdout = os.Stderr
 	return goBuild.Run()
 }
 
-func gocommandCombinedOutput(command string, args ...string) ([]byte, error) {
-	return gocommandExecCmd(command, args...).CombinedOutput()
+func gocommandCombinedOutput(command string, args ...string) (string, []byte, error) {
+	buildCmd, goBuild := gocommandExecCmd(command, args...)
+	out, err := goBuild.CombinedOutput()
+	return buildCmd, out, err
 }
 
-func gocommandExecCmd(command string, args ...string) *exec.Cmd {
+func gocommandExecCmd(command string, args ...string) (string, *exec.Cmd) {
 	allargs := []string{command}
 	allargs = append(allargs, args...)
 	goBuild := exec.Command("go", allargs...)
-	return goBuild
+	return strings.Join(append([]string{"go"}, allargs...), " "), goBuild
 }
diff --git a/service/dap/server.go b/service/dap/server.go
index bf293d5d..d091a36a 100644
--- a/service/dap/server.go
+++ b/service/dap/server.go
@@ -723,17 +723,24 @@ func (s *Server) onLaunchRequest(request *dap.LaunchRequest) {
 		}
 
 		s.log.Debugf("building binary at %s", debugbinary)
+		var cmd string
 		var out []byte
 		switch mode {
 		case "debug":
-			out, err = gobuild.GoBuildCombinedOutput(debugbinary, []string{program}, buildFlags)
+			cmd, out, err = gobuild.GoBuildCombinedOutput(debugbinary, []string{program}, buildFlags)
 		case "test":
-			out, err = gobuild.GoTestBuildCombinedOutput(debugbinary, []string{program}, buildFlags)
+			cmd, out, err = gobuild.GoTestBuildCombinedOutput(debugbinary, []string{program}, buildFlags)
 		}
 		if err != nil {
+			s.send(&dap.OutputEvent{
+				Event: *newEvent("output"),
+				Body: dap.OutputEventBody{
+					Output:   fmt.Sprintf("Build Error: %s\n%s (%s)\n", cmd, strings.TrimSpace(string(out)), err.Error()),
+					Category: "stderr",
+				}})
 			s.sendErrorResponse(request.Request,
 				FailedToLaunch, "Failed to launch",
-				fmt.Sprintf("Build error: %s (%s)", strings.TrimSpace(string(out)), err.Error()))
+				"Build error: Check the debug console for details.")
 			return
 		}
 		program = debugbinary


Test patch:
diff --git a/service/dap/server_test.go b/service/dap/server_test.go
index 063673e3..18e05d67 100644
--- a/service/dap/server_test.go
+++ b/service/dap/server_test.go
@@ -437,9 +437,9 @@ func TestContinueOnEntry(t *testing.T) {
 				}
 				// Single current thread is sent when the program is running
 				// because DAP spec expects at least one thread.
-				// TODO(polina): accept empty already-terminated response here as well?
-				if len(m.Body.Threads) != 1 || m.Body.Threads[0].Id != -1 || m.Body.Threads[0].Name != "Current" {
-					t.Errorf("\ngot  %#v\nwant Id=-1, Name=\"Current\"", m.Body.Threads)
+				// Also accept empty already-terminated response.
+				if len(m.Body.Threads) != 0 && (len(m.Body.Threads) != 1 || m.Body.Threads[0].Id != -1 || m.Body.Threads[0].Name != "Current") {
+					t.Errorf("\ngot  %#v\nwant Id=-1, Name=\"Current\" or empty", m.Body.Threads)
 				}
 			case *dap.TerminatedEvent:
 			default:
@@ -502,8 +502,12 @@ func TestPreSetBreakpoint(t *testing.T) {
 			msg := client.ExpectMessage(t)
 			switch m := msg.(type) {
 			case *dap.ThreadsResponse:
-				if len(m.Body.Threads) != 1 || m.Body.Threads[0].Id != -1 || m.Body.Threads[0].Name != "Current" {
-					t.Errorf("\ngot  %#v\nwant Id=-1, Name=\"Current\"", m.Body.Threads)
+				// If the thread request arrived while the program was running, we expect to get the dummy response
+				// with a single goroutine "Current".
+				// If the thread request arrived after the stop, we should get the goroutine stopped at main.Increment.
+				if (len(m.Body.Threads) != 1 || m.Body.Threads[0].Id != -1 || m.Body.Threads[0].Name != "Current") &&
+					(len(m.Body.Threads) < 1 || m.Body.Threads[0].Id != 1 || !strings.HasPrefix(m.Body.Threads[0].Name, "* [Go 1] main.Increment")) {
+					t.Errorf("\ngot  %#v\nwant Id=-1, Name=\"Current\" or Id=1, Name=\"* [Go 1] main.Increment ...\"", m.Body.Threads)
 				}
 			case *dap.StoppedEvent:
 				if m.Body.Reason != "breakpoint" || m.Body.ThreadId != 1 || !m.Body.AllThreadsStopped {
@@ -3140,14 +3144,6 @@ func TestBadLaunchRequests(t *testing.T) {
 			}
 		}
 
-		expectFailedToLaunchWithMessageRegex := func(response *dap.ErrorResponse, errmsg string) {
-			t.Helper()
-			expectFailedToLaunch(response)
-			if matched, _ := regexp.MatchString(errmsg, response.Body.Error.Format); !matched {
-				t.Errorf("\ngot  %q\nwant %q", response.Body.Error.Format, errmsg)
-			}
-		}
-
 		// Test for the DAP-specific detailed error message.
 		client.LaunchRequest("exec", "", stopOnEntry)
 		expectFailedToLaunchWithMessage(client.ExpectInvisibleErrorResponse(t),
@@ -3229,18 +3225,34 @@ func TestBadLaunchRequests(t *testing.T) {
 		expectFailedToLaunch(client.ExpectInvisibleErrorResponse(t)) // No such file or directory
 
 		client.LaunchRequest("debug", fixture.Path+"_does_not_exist", stopOnEntry)
+		oe := client.ExpectOutputEvent(t)
+		if !strings.HasPrefix(oe.Body.Output, "Build Error: ") || oe.Body.Category != "stderr" {
+			t.Errorf("got %#v, want Category=\"stderr\" Output=\"Build Error: ...\"", oe)
+		}
 		expectFailedToLaunch(client.ExpectInvisibleErrorResponse(t))
 
 		client.LaunchRequest("" /*debug by default*/, fixture.Path+"_does_not_exist", stopOnEntry)
+		oe = client.ExpectOutputEvent(t)
+		if !strings.HasPrefix(oe.Body.Output, "Build Error: ") || oe.Body.Category != "stderr" {
+			t.Errorf("got %#v, want Category=\"stderr\" Output=\"Build Error: ...\"", oe)
+		}
 		expectFailedToLaunch(client.ExpectInvisibleErrorResponse(t))
 
 		client.LaunchRequest("exec", fixture.Source, stopOnEntry)
 		expectFailedToLaunch(client.ExpectInvisibleErrorResponse(t)) // Not an executable
 
 		client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug", "program": fixture.Source, "buildFlags": "-bad -flags"})
-		expectFailedToLaunchWithMessageRegex(client.ExpectInvisibleErrorResponse(t), `Failed to launch: Build error: .*flag provided but not defined.*`)
+		oe = client.ExpectOutputEvent(t)
+		if !strings.HasPrefix(oe.Body.Output, "Build Error: ") || oe.Body.Category != "stderr" {
+			t.Errorf("got %#v, want Category=\"stderr\" Output=\"Build Error: ...\"", oe)
+		}
+		expectFailedToLaunchWithMessage(client.ExpectInvisibleErrorResponse(t), "Failed to launch: Build error: Check the debug console for details.")
 		client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug", "program": fixture.Source, "noDebug": true, "buildFlags": "-bad -flags"})
-		expectFailedToLaunchWithMessageRegex(client.ExpectInvisibleErrorResponse(t), `Failed to launch: Build error: .*flag provided but not defined.*`)
+		oe = client.ExpectOutputEvent(t)
+		if !strings.HasPrefix(oe.Body.Output, "Build Error: ") || oe.Body.Category != "stderr" {
+			t.Errorf("got %#v, want Category=\"stderr\" Output=\"Build Error: ...\"", oe)
+		}
+		expectFailedToLaunchWithMessage(client.ExpectInvisibleErrorResponse(t), "Failed to launch: Build error: Check the debug console for details.")
 
 		// Bad "wd".
 		client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug", "program": fixture.Source, "noDebug": false, "cwd": "dir/invalid"})