google / go-dap

Go implementation of the Debug Adapter Protocol
Apache License 2.0
127 stars 22 forks source link

Support for custom DAP requests #69

Closed corneliusweig closed 1 year ago

corneliusweig commented 1 year ago

Many debug adapters make use of custom DAP messages, i.e. messages that have the shape of a regular DAP message, which are not part of the protocol. This works, because client-side extensions intercept these DAP messages and do not pass them on to VS Code.

However, go-dap does not have a way to work with such custom DAP messages. I'd like to introduce a new type Codec in the codec.go file with the following API:

// NewCodec constructs a new codec that extends the vanilla DAP protocol.
// Unless you need to register custom DAP messages, use
// DecodeProtocolMessage instead.
func NewCodec() *Codec { .. }

// RegisterRequest registers a new DAP command, so that it can be
// unmarshalled by DecodeMessage. Returns an error when the event already
// exists.
func (c *Codec) RegisterRequest(command string, requestCtor, responseCtor MessageCtor) error {  .. }
// OR RegisterCustomRequest

// RegisterEvent registers a new DAP event, so that it can be unmarshalled
// by DecodeMessage. Returns an error when the event already exists.
func (c *Codec) RegisterEvent(event string, ctor MessageCtor) error { .. }

// DecodeMessage parses the JSON-encoded data and returns the result of
// the appropriate type within the ProtocolMessage hierarchy. If message type,
// command, etc cannot be cast, returns DecodeProtocolMessageFieldError.
// See also godoc for json.Unmarshal, which is used for underlying decoding.
func (c *Codec) DecodeMessage(data []byte) (Message, error) { .. }

The last function is the equivalent of DecodeProtocolMessage().

NOTE: This means that MessageCtor is now exported, while it was not before.

@polinasok Would such an extension be acceptable?

hyangah commented 1 year ago

Isn't it possible that the server watches the error of dap.ReadProtocolMessage or dap.DecodeProtocolMessage, and if the error is DecodeProtocolMessageFieldError, takes the custom message decode path?

suzmue commented 1 year ago

Both of the above suggestions seem like reasonable approaches to me (1. Allow debug adapter to register custom requests and events and 2. Debug adapters implement decoding of protocol themselves ).

I think exporting messageCtor is exposing a bit too much of an implementation detail. Maybe some middle ground could be allowing the server to register names of custom requests and events and then have json.RawMessage as the args/bodies of the types. Something like:

// NewCodec constructs a new codec that extends the vanilla DAP protocol.
// Unless you need to register custom DAP messages, use
// DecodeProtocolMessage instead.
func NewCodec() *Codec { .. }

// RegisterRequest registers a new DAP command, so that it can be
// unmarshalled by DecodeMessage. Returns an error when the event already
// exists.
func (c *Codec) RegisterRequest(command string) error {  .. }
// OR RegisterCustomRequest

// RegisterEvent registers a new DAP event, so that it can be unmarshalled
// by DecodeMessage. Returns an error when the event already exists.
func (c *Codec) RegisterEvent(event string) error { .. }

// DecodeMessage parses the JSON-encoded data and returns the result of
// the appropriate type within the ProtocolMessage hierarchy. If message type,
// command, etc cannot be cast, returns DecodeProtocolMessageFieldError.
// See also godoc for json.Unmarshal, which is used for underlying decoding.
func (c *Codec) DecodeMessage(data []byte) (Message, error) { .. }

// CustomRequest: 
// Since this request is custom, the arguments for this request are not part of this specification.
type CustomRequest struct {
    Request

    Arguments json.RawMessage `json:"arguments"`
}

func (r *CustomRequest) GetRequest() *Request          { return &r.Request }
func (r *CustomRequest) GetArguments() json.RawMessage { return r.Arguments }

// CustomResponse: Response to a custom request. 
type CustomResponse struct {
    Response

    Body json.RawMessage `json:"body"`
}

func (r *CustomResponse) GetResponse() *Response { return &r.Response }

// CustomEvent
type CustomEvent struct {
    Event

    Body json.RawMessage `json:"body"`
}

func (e *StoppedEvent) GetEvent() *Event { return &e.Event }

Might be better at that point to just have the server handling all of the decoding.

corneliusweig commented 1 year ago

Isn't it possible that the server watches the error of dap.ReadProtocolMessage or dap.DecodeProtocolMessage

It is possible, but not very ergonomic. The easiest API is https://github.com/google/go-dap/blob/c3d6893484e6e885d51667a11f2d6db468769538/io.go#L131, which reads a blob and parses it. But the blob is lost when parsing fails, so the caller needs to call into DecodeProtocolMessage() and hold on to the blob.


The alternative suggestion to decode custom requests into a generic message with a json.RawMessage doesn't look like a good alternative, because it means that the caller still has to do the parsing. So it is effectively the same as looking for DecodeProtocolMessageFieldError, but with a bigger API surface, which isn't great.


Overall, if you think that there's no value in supporting the parsing of custom additions to the DAP protocol in this library, then no change is needed, and I'd rather close this issue as WAI.

suzmue commented 1 year ago

@corneliusweig Having go-dap be able to parse custom messages seems like a useful feature that will make this package more easily extensible. Let's start with the original proposal, but let's not export the messageCtor type for now to make the API more clear (just use func() Message).

Feel free to send a PR for this and @hyangah and I can review!