Skip to content

Commit 17f6b9b

Browse files
prql-botclaude
andcommitted
fix: LSP server should only exit on the exit notification
The main loop returned on any notification, so the first non-exit notification a client sent (e.g. textDocument/didOpen) shut the server down. Returning also dropped the connection sender, panicking with "sending on a disconnected channel". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 7d867b3 commit 17f6b9b

2 files changed

Lines changed: 65 additions & 1 deletion

File tree

prqlc/prqlc/src/cli/lsp.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,13 @@ fn main_loop(
7575
}
7676
Message::Notification(not) => {
7777
eprintln!("got notification: {not:?}");
78-
return Ok(());
78+
// Only the `exit` notification should stop the loop. Other
79+
// notifications (e.g. `textDocument/didOpen`, `$/setTrace`)
80+
// are logged and ignored — returning on any notification
81+
// would shut the server down as soon as a client sends one.
82+
if not.method == "exit" {
83+
return Ok(());
84+
}
7985
}
8086
}
8187
}

prqlc/prqlc/src/cli/test.rs

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -715,3 +715,61 @@ fn lsp() {
715715
shutting down server
716716
"###);
717717
}
718+
719+
/// A non-`exit` notification must not terminate the server — only `exit`
720+
/// should. Before the fix, the loop returned on the first notification, so
721+
/// the `exit` notification below was never reached and never logged.
722+
#[cfg(feature = "lsp")]
723+
#[test]
724+
fn lsp_ignores_non_exit_notification() {
725+
let init = serde_json::to_string(&lsp_server::Message::Request(lsp_server::Request {
726+
method: "initialize".into(),
727+
id: lsp_server::RequestId::from(1),
728+
params: serde_json::json!({"capabilities": {}}),
729+
}))
730+
.unwrap();
731+
let initialized = serde_json::to_string(&lsp_server::Message::Notification(
732+
lsp_server::Notification {
733+
method: "initialized".into(),
734+
params: serde_json::json!({}),
735+
},
736+
))
737+
.unwrap();
738+
let set_trace = serde_json::to_string(&lsp_server::Message::Notification(
739+
lsp_server::Notification {
740+
method: "$/setTrace".into(),
741+
params: serde_json::json!({"value": "off"}),
742+
},
743+
))
744+
.unwrap();
745+
let exit = serde_json::to_string(&lsp_server::Message::Notification(
746+
lsp_server::Notification {
747+
method: "exit".into(),
748+
params: serde_json::Value::Null,
749+
},
750+
))
751+
.unwrap();
752+
753+
assert_cmd_snapshot!(prqlc_command().args(["lsp"])
754+
.pass_stdin(format!("Content-Length: {}\r\n\r\n{}Content-Length: {}\r\n\r\n{}Content-Length: {}\r\n\r\n{}Content-Length: {}\r\n\r\n{}",
755+
init.len(), init,
756+
initialized.len(), initialized,
757+
set_trace.len(), set_trace,
758+
exit.len(), exit))
759+
, @r###"
760+
success: true
761+
exit_code: 0
762+
----- stdout -----
763+
Content-Length: 78
764+
765+
{"jsonrpc":"2.0","id":1,"result":{"capabilities":{"definitionProvider":true}}}
766+
----- stderr -----
767+
starting PRQL LSP server
768+
starting main loop
769+
got msg: Notification(Notification { method: "$/setTrace", params: Object {"value": String("off")} })
770+
got notification: Notification { method: "$/setTrace", params: Object {"value": String("off")} }
771+
got msg: Notification(Notification { method: "exit", params: Null })
772+
got notification: Notification { method: "exit", params: Null }
773+
shutting down server
774+
"###);
775+
}

0 commit comments

Comments
 (0)