diff --git a/service/dap/daptest/client.go b/service/dap/daptest/client.go index 3ad61ae1..53ba1b25 100644 --- a/service/dap/daptest/client.go +++ b/service/dap/daptest/client.go @@ -511,6 +511,15 @@ func (c *Client) UnknownEvent() { c.send(event) } +// BadRequest triggers an unmarshal error. +func (c *Client) BadRequest() { + content := []byte("{malformedString}") + contentLengthHeaderFmt := "Content-Length: %d\r\n\r\n" + header := fmt.Sprintf(contentLengthHeaderFmt, len(content)) + c.conn.Write([]byte(header)) + c.conn.Write(content) +} + // KnownEvent passes decode checks, but delve has no 'case' to // handle it. This behaves the same way a new request type // added to go-dap, but not to delve. diff --git a/service/dap/server.go b/service/dap/server.go index d88b832c..f9fe0fc4 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -358,18 +358,24 @@ func (s *Server) serveDAPCodec() { s.reader = bufio.NewReader(s.conn) for { request, err := dap.ReadProtocolMessage(s.reader) - // TODO(polina): Differentiate between errors and handle them - // gracefully. For example, + // Handle dap.DecodeProtocolMessageFieldError errors gracefully by responding with an ErrorResponse. + // For example: // -- "Request command 'foo' is not supported" means we // potentially got some new DAP request that we do not yet have // decoding support for, so we can respond with an ErrorResponse. - // TODO(polina): to support this add Seq to - // dap.DecodeProtocolMessageFieldError. + // + // Other errors, such as unmarshalling errors, will log the error and cause the server to trigger + // a stop. if err != nil { select { case <-s.stopTriggered: default: if err != io.EOF { + if decodeErr, ok := err.(*dap.DecodeProtocolMessageFieldError); ok { + // Send an error response to the users if we were unable to process the message. + s.sendInternalErrorResponse(decodeErr.Seq, err.Error()) + continue + } s.log.Error("DAP error: ", err) } s.triggerServerStop() diff --git a/service/dap/server_test.go b/service/dap/server_test.go index e7a0c31f..615a8f1a 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -4869,7 +4869,7 @@ func TestBadlyFormattedMessageToServer(t *testing.T) { runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) { // Send a badly formatted message to the server, and expect it to close the // connection. - client.UnknownRequest() + client.BadRequest() time.Sleep(100 * time.Millisecond) _, err := client.ReadMessage() @@ -4878,4 +4878,17 @@ func TestBadlyFormattedMessageToServer(t *testing.T) { t.Errorf("got err=%v, want io.EOF", err) } }) + runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) { + // Send an unknown request message to the server, and expect it to send + // an error response. + client.UnknownRequest() + err := client.ExpectErrorResponse(t) + if err.Body.Error.Format != "Internal Error: Request command 'unknown' is not supported (seq: 1)" || err.RequestSeq != 1 { + t.Errorf("got %v, want RequestSeq=1 Error=\"Internal Error: Request command 'unknown' is not supported (seq: 1)\"", err) + } + + // Make sure that the unknown request did not kill the server. + client.InitializeRequest() + client.ExpectInitializeResponse(t) + }) }