From f86ed675d8eec075b2ededb1f88990e5fc5dd9ea Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Tue, 13 Jul 2021 11:38:28 -0400 Subject: [PATCH] service/dap: add panic and throw text to stopped event (#2559) * service/dap: add panic and throw text to stopped event We can add more information to the stopped events on errors using the `Text` field in the stopped event. We already use this to display the runtime errors. Adding this information to the stopped reason will also help to show the user additional info when a stopped event is not associated with a particular goroutine. --- pkg/goversion/compat.go | 2 +- pkg/goversion/go_version.go | 8 +++--- service/dap/server.go | 51 +++++++++++++++++++++---------------- service/dap/server_test.go | 44 +++++++++++++++++++++----------- 4 files changed, 62 insertions(+), 43 deletions(-) diff --git a/pkg/goversion/compat.go b/pkg/goversion/compat.go index 62c82c1c23..4e230294d2 100644 --- a/pkg/goversion/compat.go +++ b/pkg/goversion/compat.go @@ -16,7 +16,7 @@ var ( // Compatible checks that the version specified in the producer string is compatible with // this version of delve. func Compatible(producer string) error { - ver := parseProducer(producer) + ver := ParseProducer(producer) if ver.IsDevel() { return nil } diff --git a/pkg/goversion/go_version.go b/pkg/goversion/go_version.go index 88a2d4e5cc..0d57fde8b8 100644 --- a/pkg/goversion/go_version.go +++ b/pkg/goversion/go_version.go @@ -174,17 +174,15 @@ const producerVersionPrefix = "Go cmd/compile " // ProducerAfterOrEqual checks that the DW_AT_producer version is // major.minor or a later version, or a development version. func ProducerAfterOrEqual(producer string, major, minor int) bool { - ver := parseProducer(producer) + ver := ParseProducer(producer) if ver.IsDevel() { return true } return ver.AfterOrEqual(GoVersion{major, minor, -1, 0, 0, ""}) } -func parseProducer(producer string) GoVersion { - if strings.HasPrefix(producer, producerVersionPrefix) { - producer = producer[len(producerVersionPrefix):] - } +func ParseProducer(producer string) GoVersion { + producer = strings.TrimPrefix(producer, producerVersionPrefix) ver, _ := Parse(producer) return ver } diff --git a/service/dap/server.go b/service/dap/server.go index 696bf6cb6b..cec104ad14 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -2515,34 +2515,20 @@ func (s *Server) onExceptionInfoRequest(request *dap.ExceptionInfoRequest) { switch bpState.Breakpoint.Name { case proc.FatalThrow: body.ExceptionId = "fatal error" - // Attempt to get the value of the throw reason. - // This is not currently working for Go 1.16 or 1.17: https://github.com/golang/go/issues/46425. - handleError := func(err error) { - if err != nil { - body.Description = fmt.Sprintf("Error getting throw reason: %s", err.Error()) - } - if goversion.ProducerAfterOrEqual(s.debugger.TargetGoVersion(), 1, 16) { + body.Description, err = s.throwReason(goroutineID) + if err != nil { + body.Description = fmt.Sprintf("Error getting throw reason: %s", err.Error()) + // This is not currently working for Go 1.16. + ver := goversion.ParseProducer(s.debugger.TargetGoVersion()) + if ver.Major == 1 && ver.Minor == 16 { body.Description = "Throw reason unavailable, see https://github.com/golang/go/issues/46425" } } - - exprVar, err := s.debugger.EvalVariableInScope(goroutineID, 1, 0, "s", DefaultLoadConfig) - if err == nil { - if exprVar.Value != nil { - body.Description = exprVar.Value.String() - } else { - handleError(exprVar.Unreadable) - } - } else { - handleError(err) - } case proc.UnrecoveredPanic: body.ExceptionId = "panic" // Attempt to get the value of the panic message. - exprVar, err := s.debugger.EvalVariableInScope(goroutineID, 0, 0, "(*msgs).arg.(data)", DefaultLoadConfig) - if err == nil { - body.Description = exprVar.Value.String() - } else { + body.Description, err = s.panicReason(goroutineID) + if err != nil { body.Description = fmt.Sprintf("Error getting panic message: %s", err.Error()) } } @@ -2599,6 +2585,25 @@ func (s *Server) onExceptionInfoRequest(request *dap.ExceptionInfoRequest) { s.send(response) } +func (s *Server) throwReason(goroutineID int) (string, error) { + return s.getExprString("s", goroutineID, 1) +} + +func (s *Server) panicReason(goroutineID int) (string, error) { + return s.getExprString("(*msgs).arg.(data)", goroutineID, 0) +} + +func (s *Server) getExprString(expr string, goroutineID, frame int) (string, error) { + exprVar, err := s.debugger.EvalVariableInScope(goroutineID, frame, 0, expr, DefaultLoadConfig) + if err != nil { + return "", err + } + if exprVar.Value == nil { + return "", exprVar.Unreadable + } + return exprVar.Value.String(), nil +} + // sendErrorResponseWithOpts offers configuration options. // showUser - if true, the error will be shown to the user (e.g. via a visible pop-up) func (s *Server) sendErrorResponseWithOpts(request dap.Request, id int, summary, details string, showUser bool) { @@ -2727,9 +2732,11 @@ func (s *Server) doRunCommand(command string, asyncSetupDone chan struct{}) { case proc.FatalThrow: stopped.Body.Reason = "exception" stopped.Body.Description = "fatal error" + stopped.Body.Text, _ = s.throwReason(stopped.Body.ThreadId) case proc.UnrecoveredPanic: stopped.Body.Reason = "exception" stopped.Body.Description = "panic" + stopped.Body.Text, _ = s.panicReason(stopped.Body.ThreadId) } if strings.HasPrefix(state.CurrentThread.Breakpoint.Name, functionBpPrefix) { stopped.Body.Reason = "function breakpoint" diff --git a/service/dap/server_test.go b/service/dap/server_test.go index f779cdae60..2308442528 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -3606,15 +3606,16 @@ func TestPanicBreakpointOnContinue(t *testing.T) { client.ContinueRequest(1) client.ExpectContinueResponse(t) + text := "\"BOOM!\"" se := client.ExpectStoppedEvent(t) - if se.Body.ThreadId != 1 || se.Body.Reason != "exception" || se.Body.Description != "panic" { - t.Errorf("\ngot %#v\nwant ThreadId=1 Reason=\"exception\" Description=\"panic\"", se) + if se.Body.ThreadId != 1 || se.Body.Reason != "exception" || se.Body.Description != "panic" || se.Body.Text != text { + t.Errorf("\ngot %#v\nwant ThreadId=1 Reason=\"exception\" Description=\"panic\" Text=%q", se, text) } client.ExceptionInfoRequest(1) eInfo := client.ExpectExceptionInfoResponse(t) - if eInfo.Body.ExceptionId != "panic" || eInfo.Body.Description != "\"BOOM!\"" { - t.Errorf("\ngot %#v\nwant ExceptionId=\"panic\" Description=\"\"BOOM!\"\"", eInfo) + if eInfo.Body.ExceptionId != "panic" || eInfo.Body.Description != text { + t.Errorf("\ngot %#v\nwant ExceptionId=\"panic\" Description=%q", eInfo, text) } client.StackTraceRequest(se.Body.ThreadId, 0, 20) @@ -3657,15 +3658,16 @@ func TestPanicBreakpointOnNext(t *testing.T) { client.NextRequest(1) client.ExpectNextResponse(t) + text := "\"BOOM!\"" se := client.ExpectStoppedEvent(t) - if se.Body.ThreadId != 1 || se.Body.Reason != "exception" || se.Body.Description != "panic" { - t.Errorf("\ngot %#v\nwant ThreadId=1 Reason=\"exception\" Description=\"panic\"", se) + if se.Body.ThreadId != 1 || se.Body.Reason != "exception" || se.Body.Description != "panic" || se.Body.Text != text { + t.Errorf("\ngot %#v\nwant ThreadId=1 Reason=\"exception\" Description=\"panic\" Text=%q", se, text) } client.ExceptionInfoRequest(1) eInfo := client.ExpectExceptionInfoResponse(t) - if eInfo.Body.ExceptionId != "panic" || eInfo.Body.Description != "\"BOOM!\"" { - t.Errorf("\ngot %#v\nwant ExceptionId=\"panic\" Description=\"\"BOOM!\"\"", eInfo) + if eInfo.Body.ExceptionId != "panic" || eInfo.Body.Description != text { + t.Errorf("\ngot %#v\nwant ExceptionId=\"panic\" Description=%q", eInfo, text) } }, disconnect: true, @@ -3689,14 +3691,21 @@ func TestFatalThrowBreakpoint(t *testing.T) { client.ContinueRequest(1) client.ExpectContinueResponse(t) + var text string + // This does not work for Go 1.16. + ver, _ := goversion.Parse(runtime.Version()) + if ver.Major != 1 || ver.Minor != 16 { + text = "\"go of nil func value\"" + } + se := client.ExpectStoppedEvent(t) - if se.Body.ThreadId != 1 || se.Body.Reason != "exception" || se.Body.Description != "fatal error" { - t.Errorf("\ngot %#v\nwant ThreadId=1 Reason=\"exception\" Description=\"fatal error\"", se) + if se.Body.ThreadId != 1 || se.Body.Reason != "exception" || se.Body.Description != "fatal error" || se.Body.Text != text { + t.Errorf("\ngot %#v\nwant ThreadId=1 Reason=\"exception\" Description=\"fatal error\" Text=%q", se, text) } - // TODO(suzmue): Enable this test for 1.17 when https://github.com/golang/go/issues/46425 is fixed. - errorPrefix := "\"go of nil func value\"" - if goversion.VersionAfterOrEqual(runtime.Version(), 1, 16) { + // This does not work for Go 1.16. + errorPrefix := text + if errorPrefix == "" { errorPrefix = "Throw reason unavailable, see https://github.com/golang/go/issues/46425" } client.ExceptionInfoRequest(1) @@ -3724,9 +3733,14 @@ func TestFatalThrowBreakpoint(t *testing.T) { client.ContinueRequest(1) client.ExpectContinueResponse(t) + // TODO(suzmue): Enable this test for 1.17 when https://github.com/golang/go/issues/46425 is fixed. + var text string + if !goversion.VersionAfterOrEqual(runtime.Version(), 1, 16) { + text = "\"all goroutines are asleep - deadlock!\"" + } se := client.ExpectStoppedEvent(t) - if se.Body.Reason != "exception" || se.Body.Description != "fatal error" { - t.Errorf("\ngot %#v\nwant Reason=\"exception\" Description=\"fatal error\"", se) + if se.Body.Reason != "exception" || se.Body.Description != "fatal error" || se.Body.Text != text { + t.Errorf("\ngot %#v\nwant Reason=\"exception\" Description=\"fatal error\" Text=%q", se, text) } // TODO(suzmue): Get the exception info for the thread and check the description