DAP: Refactor handling of Event and Request protocol types

This change refactors the DAP `Event` type, the `helix_dap` module and
the `helix_dap::transport` module to be closer to the equivalent
implementations in `helix_lsp`. The DAP `Event` type is similar to LSP's
`Notification` so this change rewrites the enum as a trait which is
implemented by empty types (for example `enum Initialized {}`).

`Event` is then reintroduced as an enum in `helix_dap` with a helper
function to convert from the `Event` as the transport knows it. By
separating the definitions of `Event` between lib and transport, we can
handle incoming events which are not known to our `Event` enum. For
example debugpy sends `"debugpySockets"` which is unknown. With this
change, unknown events are discarded with an info-level log message.

The `Request` type was already a trait but did not have an enum with the
known values. This change also introduces a `helix_dap::Request` enum
similar to `helix_dap::Event`. This resolves a TODO comment about
avoiding `unwrap` when parsing the request's arguments.

helix-dap/src/lib.rs

@@ -3,10 +3,11 @@ mod transport;
 mod types;
 
 pub use client::{Client, ConnectionType};
-pub use events::Event;
 pub use transport::{Payload, Response, Transport};
 pub use types::*;
 
+use serde::de::DeserializeOwned;
+
 use thiserror::Error;
 #[derive(Error, Debug)]
 pub enum Error {
@@ -18,9 +19,84 @@ pub enum Error {
     Timeout(u64),
     #[error("server closed the stream")]
     StreamClosed,
+    #[error("Unhandled")]
+    Unhandled,
     #[error(transparent)]
     ExecutableNotFound(#[from] helix_stdx::env::ExecutableNotFoundError),
     #[error(transparent)]
     Other(#[from] anyhow::Error),
 }
 pub type Result<T> = core::result::Result<T, Error>;
+
+#[derive(Debug)]
+pub enum Request {
+    RunInTerminal(<requests::RunInTerminal as types::Request>::Arguments),
+}
+
+impl Request {
+    pub fn parse(command: &str, arguments: Option<serde_json::Value>) -> Result<Self> {
+        use crate::types::Request as _;
+
+        let arguments = arguments.unwrap_or_default();
+        let request = match command {
+            requests::RunInTerminal::COMMAND => Self::RunInTerminal(parse_value(arguments)?),
+            _ => return Err(Error::Unhandled),
+        };
+
+        Ok(request)
+    }
+}
+
+#[derive(Debug)]
+pub enum Event {
+    Initialized(<events::Initialized as events::Event>::Body),
+    Stopped(<events::Stopped as events::Event>::Body),
+    Continued(<events::Continued as events::Event>::Body),
+    Exited(<events::Exited as events::Event>::Body),
+    Terminated(<events::Terminated as events::Event>::Body),
+    Thread(<events::Thread as events::Event>::Body),
+    Output(<events::Output as events::Event>::Body),
+    Breakpoint(<events::Breakpoint as events::Event>::Body),
+    Module(<events::Module as events::Event>::Body),
+    LoadedSource(<events::LoadedSource as events::Event>::Body),
+    Process(<events::Process as events::Event>::Body),
+    Capabilities(<events::Capabilities as events::Event>::Body),
+    // ProgressStart(),
+    // ProgressUpdate(),
+    // ProgressEnd(),
+    // Invalidated(),
+    Memory(<events::Memory as events::Event>::Body),
+}
+
+impl Event {
+    pub fn parse(event: &str, body: Option<serde_json::Value>) -> Result<Self> {
+        use crate::events::Event as _;
+
+        let body = body.unwrap_or_default();
+        let event = match event {
+            events::Initialized::EVENT => Self::Initialized(parse_value(body)?),
+            events::Stopped::EVENT => Self::Stopped(parse_value(body)?),
+            events::Continued::EVENT => Self::Continued(parse_value(body)?),
+            events::Exited::EVENT => Self::Exited(parse_value(body)?),
+            events::Terminated::EVENT => Self::Terminated(parse_value(body)?),
+            events::Thread::EVENT => Self::Thread(parse_value(body)?),
+            events::Output::EVENT => Self::Output(parse_value(body)?),
+            events::Breakpoint::EVENT => Self::Breakpoint(parse_value(body)?),
+            events::Module::EVENT => Self::Module(parse_value(body)?),
+            events::LoadedSource::EVENT => Self::LoadedSource(parse_value(body)?),
+            events::Process::EVENT => Self::Process(parse_value(body)?),
+            events::Capabilities::EVENT => Self::Capabilities(parse_value(body)?),
+            events::Memory::EVENT => Self::Memory(parse_value(body)?),
+            _ => return Err(Error::Unhandled),
+        };
+
+        Ok(event)
+    }
+}
+
+fn parse_value<T>(value: serde_json::Value) -> Result<T>
+where
+    T: DeserializeOwned,
+{
+    serde_json::from_value(value).map_err(|err| err.into())
+}

helix-dap/src/transport.rs

@@ -1,4 +1,4 @@
-use crate::{Error, Event, Result};
+use crate::{Error, Result};
 use anyhow::Context;
 use log::{error, info, warn};
 use serde::{Deserialize, Serialize};
@@ -32,11 +32,17 @@ pub struct Response {
     pub body: Option<Value>,
 }
 
+#[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
+pub struct Event {
+    pub event: String,
+    pub body: Option<Value>,
+}
+
 #[derive(Debug, Clone, Deserialize, Serialize)]
 #[serde(tag = "type", rename_all = "camelCase")]
 pub enum Payload {
     // type = "event"
-    Event(Box<Event>),
+    Event(Event),
     // type = "response"
     Response(Response),
     // type = "request"

helix-dap/src/types.rs

@@ -759,33 +759,30 @@ pub mod requests {
 pub mod events {
     use super::*;
 
-    #[derive(Debug, Clone, Serialize, Deserialize)]
-    #[serde(rename_all = "camelCase")]
-    #[serde(tag = "event", content = "body")]
-    // seq is omitted as unused and is not sent by some implementations
-    pub enum Event {
-        Initialized(Option<DebuggerCapabilities>),
-        Stopped(Stopped),
-        Continued(Continued),
-        Exited(Exited),
-        Terminated(Option<Terminated>),
-        Thread(Thread),
-        Output(Output),
-        Breakpoint(Breakpoint),
-        Module(Module),
-        LoadedSource(LoadedSource),
-        Process(Process),
-        Capabilities(Capabilities),
-        // ProgressStart(),
-        // ProgressUpdate(),
-        // ProgressEnd(),
-        // Invalidated(),
-        Memory(Memory),
+    pub trait Event {
+        type Body: serde::de::DeserializeOwned + serde::Serialize;
+        const EVENT: &'static str;
+    }
+
+    #[derive(Debug)]
+    pub enum Initialized {}
+
+    impl Event for Initialized {
+        type Body = Option<DebuggerCapabilities>;
+        const EVENT: &'static str = "initialized";
+    }
+
+    #[derive(Debug)]
+    pub enum Stopped {}
+
+    impl Event for Stopped {
+        type Body = StoppedBody;
+        const EVENT: &'static str = "stopped";
     }
 
     #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
     #[serde(rename_all = "camelCase")]
-    pub struct Stopped {
+    pub struct StoppedBody {
         pub reason: String,
         #[serde(skip_serializing_if = "Option::is_none")]
         pub description: Option<String>,
@@ -801,37 +798,77 @@ pub mod events {
         pub hit_breakpoint_ids: Option<Vec<usize>>,
     }
 
+    #[derive(Debug)]
+    pub enum Continued {}
+
+    impl Event for Continued {
+        type Body = ContinuedBody;
+        const EVENT: &'static str = "continued";
+    }
+
     #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
     #[serde(rename_all = "camelCase")]
-    pub struct Continued {
+    pub struct ContinuedBody {
         pub thread_id: ThreadId,
         #[serde(skip_serializing_if = "Option::is_none")]
         pub all_threads_continued: Option<bool>,
     }
 
-    #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
-    #[serde(rename_all = "camelCase")]
-    pub struct Exited {
-        pub exit_code: usize,
+    #[derive(Debug)]
+    pub enum Exited {}
+
+    impl Event for Exited {
+        type Body = ExitedBody;
+        const EVENT: &'static str = "exited";
     }
 
     #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
     #[serde(rename_all = "camelCase")]
-    pub struct Terminated {
+    pub struct ExitedBody {
+        pub exit_code: usize,
+    }
+
+    #[derive(Debug)]
+    pub enum Terminated {}
+
+    impl Event for Terminated {
+        type Body = Option<TerminatedBody>;
+        const EVENT: &'static str = "terminated";
+    }
+
+    #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
+    #[serde(rename_all = "camelCase")]
+    pub struct TerminatedBody {
         #[serde(skip_serializing_if = "Option::is_none")]
         pub restart: Option<Value>,
     }
 
-    #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
-    #[serde(rename_all = "camelCase")]
-    pub struct Thread {
-        pub reason: String,
-        pub thread_id: ThreadId,
+    #[derive(Debug)]
+    pub enum Thread {}
+
+    impl Event for Thread {
+        type Body = ThreadBody;
+        const EVENT: &'static str = "thread";
     }
 
     #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
     #[serde(rename_all = "camelCase")]
-    pub struct Output {
+    pub struct ThreadBody {
+        pub reason: String,
+        pub thread_id: ThreadId,
+    }
+
+    #[derive(Debug)]
+    pub enum Output {}
+
+    impl Event for Output {
+        type Body = OutputBody;
+        const EVENT: &'static str = "output";
+    }
+
+    #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
+    #[serde(rename_all = "camelCase")]
+    pub struct OutputBody {
         pub output: String,
         #[serde(skip_serializing_if = "Option::is_none")]
         pub category: Option<String>,
@@ -849,30 +886,62 @@ pub mod events {
         pub data: Option<Value>,
     }
 
+    #[derive(Debug)]
+    pub enum Breakpoint {}
+
+    impl Event for Breakpoint {
+        type Body = BreakpointBody;
+        const EVENT: &'static str = "breakpoint";
+    }
+
     #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
     #[serde(rename_all = "camelCase")]
-    pub struct Breakpoint {
+    pub struct BreakpointBody {
         pub reason: String,
         pub breakpoint: super::Breakpoint,
     }
 
+    #[derive(Debug)]
+    pub enum Module {}
+
+    impl Event for Module {
+        type Body = ModuleBody;
+        const EVENT: &'static str = "module";
+    }
+
     #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
     #[serde(rename_all = "camelCase")]
-    pub struct Module {
+    pub struct ModuleBody {
         pub reason: String,
         pub module: super::Module,
     }
 
-    #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
-    #[serde(rename_all = "camelCase")]
-    pub struct LoadedSource {
-        pub reason: String,
-        pub source: super::Source,
+    #[derive(Debug)]
+    pub enum LoadedSource {}
+
+    impl Event for LoadedSource {
+        type Body = LoadedSourceBody;
+        const EVENT: &'static str = "loadedSource";
     }
 
     #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
     #[serde(rename_all = "camelCase")]
-    pub struct Process {
+    pub struct LoadedSourceBody {
+        pub reason: String,
+        pub source: super::Source,
+    }
+
+    #[derive(Debug)]
+    pub enum Process {}
+
+    impl Event for Process {
+        type Body = ProcessBody;
+        const EVENT: &'static str = "process";
+    }
+
+    #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
+    #[serde(rename_all = "camelCase")]
+    pub struct ProcessBody {
         pub name: String,
         #[serde(skip_serializing_if = "Option::is_none")]
         pub system_process_id: Option<usize>,
@@ -884,38 +953,55 @@ pub mod events {
         pub pointer_size: Option<usize>,
     }
 
+    #[derive(Debug)]
+    pub enum Capabilities {}
+
+    impl Event for Capabilities {
+        type Body = CapabilitiesBody;
+        const EVENT: &'static str = "capabilities";
+    }
+
     #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
     #[serde(rename_all = "camelCase")]
-    pub struct Capabilities {
+    pub struct CapabilitiesBody {
         pub capabilities: super::DebuggerCapabilities,
     }
 
     // #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
     // #[serde(rename_all = "camelCase")]
-    // pub struct Invalidated {
+    // pub struct InvalidatedBody {
     // pub areas: Vec<InvalidatedArea>,
     // pub thread_id: Option<ThreadId>,
     // pub stack_frame_id: Option<usize>,
     // }
 
+    #[derive(Debug)]
+    pub enum Memory {}
+
+    impl Event for Memory {
+        type Body = MemoryBody;
+        const EVENT: &'static str = "memory";
+    }
+
     #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
     #[serde(rename_all = "camelCase")]
-    pub struct Memory {
+    pub struct MemoryBody {
         pub memory_reference: String,
         pub offset: usize,
         pub count: usize,
     }
+}
 
 #[test]
 fn test_deserialize_module_id_from_number() {
     let raw = r#"{"id": 0, "name": "Name"}"#;
-    let module: super::Module = serde_json::from_str(raw).expect("Error!");
+    let module: Module = serde_json::from_str(raw).expect("Error!");
     assert_eq!(module.id, "0");
 }
 
 #[test]
 fn test_deserialize_module_id_from_string() {
     let raw = r#"{"id": "0", "name": "Name"}"#;
-    let module: super::Module = serde_json::from_str(raw).expect("Error!");
+    let module: Module = serde_json::from_str(raw).expect("Error!");
     assert_eq!(module.id, "0");
 }

helix-view/src/handlers/dap.rs

@@ -5,6 +5,7 @@ use helix_core::Selection;
 use helix_dap::{self as dap, Client, ConnectionType, Payload, Request, ThreadId};
 use helix_lsp::block_on;
 use log::warn;
+use serde_json::json;
 use std::fmt::Write;
 use std::path::PathBuf;
 
@@ -141,7 +142,6 @@ pub fn breakpoints_changed(
 
 impl Editor {
     pub async fn handle_debugger_message(&mut self, payload: helix_dap::Payload) -> bool {
-        use dap::requests::RunInTerminal;
         use helix_dap::{events, Event};
 
         let debugger = match self.debugger.as_mut() {
@@ -149,8 +149,20 @@ impl Editor {
             None => return false,
         };
         match payload {
-            Payload::Event(ev) => match *ev {
-                Event::Stopped(events::Stopped {
+            Payload::Event(event) => {
+                let event = match Event::parse(&event.event, event.body) {
+                    Ok(event) => event,
+                    Err(dap::Error::Unhandled) => {
+                        log::info!("Discarding unknown DAP event '{}'", event.event);
+                        return false;
+                    }
+                    Err(err) => {
+                        log::warn!("Discarding invalid DAP event '{}': {err}", event.event);
+                        return false;
+                    }
+                };
+                match event {
+                    Event::Stopped(events::StoppedBody {
                         thread_id,
                         description,
                         text,
@@ -161,7 +173,9 @@ impl Editor {
                         let all_threads_stopped = all_threads_stopped.unwrap_or_default();
 
                         if all_threads_stopped {
-                        if let Ok(response) = debugger.request::<dap::requests::Threads>(()).await {
+                            if let Ok(response) =
+                                debugger.request::<dap::requests::Threads>(()).await
+                            {
                                 for thread in response.threads {
                                     fetch_stack_trace(debugger, thread.id).await;
                                 }
@@ -192,7 +206,7 @@ impl Editor {
 
                         self.set_status(status);
                     }
-                Event::Continued(events::Continued { thread_id, .. }) => {
+                    Event::Continued(events::ContinuedBody { thread_id, .. }) => {
                         debugger
                             .thread_states
                             .insert(thread_id, "running".to_owned());
@@ -203,7 +217,7 @@ impl Editor {
                     Event::Thread(_) => {
                         // TODO: update thread_states, make threads request
                     }
-                Event::Breakpoint(events::Breakpoint { reason, breakpoint }) => {
+                    Event::Breakpoint(events::BreakpointBody { reason, breakpoint }) => {
                         match &reason[..] {
                             "new" => {
                                 if let Some(source) = breakpoint.source {
@@ -230,9 +244,10 @@ impl Editor {
                                             .message
                                             .clone()
                                             .or_else(|| breakpoints[i].message.take());
-                                    breakpoints[i].line = breakpoint
-                                        .line
-                                        .map_or(breakpoints[i].line, |line| line.saturating_sub(1));
+                                        breakpoints[i].line =
+                                            breakpoint.line.map_or(breakpoints[i].line, |line| {
+                                                line.saturating_sub(1)
+                                            });
                                         breakpoints[i].column =
                                             breakpoint.column.or(breakpoints[i].column);
                                     }
@@ -252,7 +267,7 @@ impl Editor {
                             }
                         }
                     }
-                Event::Output(events::Output {
+                    Event::Output(events::OutputBody {
                         category, output, ..
                     }) => {
                         let prefix = match category {
@@ -312,7 +327,8 @@ impl Editor {
                                     }
                                 };
 
-                            let relaunch_resp = if let ConnectionType::Launch = connection_type {
+                                let relaunch_resp = if let ConnectionType::Launch = connection_type
+                                {
                                     debugger.launch(restart_args).await
                                 } else {
                                     debugger.attach(restart_args).await
@@ -345,54 +361,48 @@ impl Editor {
                         log::warn!("Unhandled event {:?}", ev);
                         return false; // return early to skip render
                     }
-            },
+                }
+            }
             Payload::Response(_) => unreachable!(),
-            Payload::Request(request) => match request.command.as_str() {
-                RunInTerminal::COMMAND => {
-                    let arguments: dap::requests::RunInTerminalArguments =
-                        serde_json::from_value(request.arguments.unwrap_or_default()).unwrap();
-                    // TODO: no unwrap
-
-                    let config = match self.config().terminal.clone() {
-                        Some(config) => config,
-                        None => {
+            Payload::Request(request) => {
+                let reply = match Request::parse(&request.command, request.arguments) {
+                    Ok(Request::RunInTerminal(arguments)) => {
+                        let config = self.config();
+                        let Some(config) = config.terminal.as_ref() else {
                             self.set_error("No external terminal defined");
                             return true;
-                        }
                         };
 
-                    // Re-borrowing debugger to avoid issues when loading config
-                    let debugger = match self.debugger.as_mut() {
-                        Some(debugger) => debugger,
-                        None => return false,
-                    };
-
-                    let process = match std::process::Command::new(config.command)
-                        .args(config.args)
+                        let process = match std::process::Command::new(&config.command)
+                            .args(&config.args)
                             .arg(arguments.args.join(" "))
                             .spawn()
                         {
                             Ok(process) => process,
                             Err(err) => {
-                            self.set_error(format!("Error starting external terminal: {}", err));
+                                self.set_error(format!(
+                                    "Error starting external terminal: {}",
+                                    err
+                                ));
                                 return true;
                             }
                         };
 
-                    let _ = debugger
-                        .reply(
-                            request.seq,
-                            dap::requests::RunInTerminal::COMMAND,
-                            serde_json::to_value(dap::requests::RunInTerminalResponse {
+                        Ok(json!(dap::requests::RunInTerminalResponse {
                             process_id: Some(process.id()),
                             shell_process_id: None,
-                            })
-                            .map_err(|e| e.into()),
-                        )
-                        .await;
+                        }))
+                    }
+                    Err(err) => Err(err),
+                };
+
+                if let Some(debugger) = self.debugger.as_mut() {
+                    debugger
+                        .reply(request.seq, &request.command, reply)
+                        .await
+                        .ok();
+                }
             }
-                _ => log::error!("DAP reverse request not implemented: {:?}", request),
-            },
         }
         true
     }