Skip to content

Commit 93eb192

Browse files
authored
[LSP] Fix issue where vscode-generated baml_client would have stale data (#1791)
<!-- ELLIPSIS_HIDDEN --> > [!IMPORTANT] > Fixes stale data issue in `baml_client` by adjusting notification handling and session management. > > - **Behavior**: > - Replaces `ruff` with `baml` in `logging.rs` for log filtering. > - Modifies `DidSaveTextDocument` handling in `api.rs` to use `local_notification_task` instead of `background_notification_task` to prevent stale data. > - Adds `clear_unsaved_files()` in `session.rs` to clear unsaved files on save. > - **Session Management**: > - Adds `clear_unsaved_files()` method in `Session` to clear unsaved files. > - Updates `reload()` in `Session` to handle unsaved files correctly. > - **Connection Handling**: > - Changes `handle_shutdown()` in `connection.rs` to loop until an exit notification is received, handling unexpected messages. > - **Client Readiness**: > - Adds `clientReady` flag in `index.ts` to ensure client is ready before sending requests. > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=BoundaryML%2Fbaml&utm_source=github&utm_medium=referral)<sup> for 30c980b. It will automatically update as commits are pushed.</sup> <!-- ELLIPSIS_HIDDEN -->
1 parent 68fa88b commit 93eb192

6 files changed

Lines changed: 55 additions & 14 deletions

File tree

engine/language_server/src/logging.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ impl<S> tracing_subscriber::layer::Filter<S> for LogLevelFilter {
111111
meta: &tracing::Metadata<'_>,
112112
_: &tracing_subscriber::layer::Context<'_, S>,
113113
) -> bool {
114-
let filter = if meta.target().starts_with("ruff") {
114+
let filter = if meta.target().starts_with("baml") {
115115
self.filter.trace_level()
116116
} else {
117117
tracing::Level::INFO

engine/language_server/src/server/api.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ pub(super) fn request<'a>(req: lsp_server::Request) -> Task<'a> {
8383
let project = session
8484
.project_db_for_path(url.to_file_path().unwrap())
8585
.expect("Already checked for project's existence");
86-
project.lock().unwrap().update_runtime(Some(notifier));
86+
project.lock().unwrap().update_runtime(Some(notifier))?;
8787

8888
// TODO: I think we need to send ALL diagnostics for the project. Not sure how this report is different vs sending a signle diagnostic param message
8989
let diagnostics = file_diagnostics(project.clone(), &url);
@@ -183,11 +183,15 @@ pub(super) fn notification<'a>(notif: lsp_server::Notification) -> Vec<Task<'a>>
183183
}
184184
// --- DidSaveTextDocument now uses the simple local task helper ---
185185
notification::DidSaveTextDocument::METHOD => {
186+
tracing::info!("Did save text document---------");
186187
handle_notification_result_error::<notification::DidSaveTextDocument>(
187-
background_notification_task::<notification::DidSaveTextDocument>(
188-
notif,
189-
BackgroundSchedule::LatencySensitive,
190-
),
188+
// Do not use background notifs yet, as baml_client may not have an updated view of the project files
189+
// See the did_save_text_document.rs file for more details
190+
// background_notification_task::<notification::DidSaveTextDocument>(
191+
// notif,
192+
// BackgroundSchedule::LatencySensitive,
193+
// ),
194+
local_notification_task::<notification::DidSaveTextDocument>(notif),
191195
)
192196
}
193197

engine/language_server/src/server/api/notifications/did_save_text_document.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@ impl super::SyncNotificationHandler for DidSaveTextDocument {
1919
_requester: &mut Requester,
2020
params: types::DidSaveTextDocumentParams,
2121
) -> Result<()> {
22+
tracing::info!("Did save text document---------");
2223
let url = params.text_document.uri;
2324
let path = url
2425
.to_file_path()
2526
.internal_error_msg("Could not convert URL to path")?;
27+
session.clear_unsaved_files();
2628
session.reload(Some(notifier.clone())).internal_error()?;
2729
tracing::info!("About to run generator. URL path: {:?}", path);
2830
session
@@ -51,6 +53,8 @@ impl super::SyncNotificationHandler for DidSaveTextDocument {
5153
}
5254
}
5355

56+
// Do not use this yet, it seems it has an outdated view of the project files and it generates
57+
// stale baml clients
5458
impl super::BackgroundDocumentNotificationHandler for DidSaveTextDocument {
5559
fn document_url(params: &types::DidSaveTextDocumentParams) -> Cow<types::Url> {
5660
Cow::Borrowed(&params.text_document.uri)

engine/language_server/src/server/connection.rs

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -102,19 +102,40 @@ impl Connection {
102102
self.sender
103103
.send(lsp::Response::new_ok(id.clone(), ()).into())?;
104104
tracing::info!("Shutdown request received. Waiting for an exit notification...");
105-
match self.receiver.recv_timeout(std::time::Duration::from_secs(30))? {
106-
lsp::Message::Notification(lsp::Notification { method, .. }) if method == lsp_types::notification::Exit::METHOD => {
107-
tracing::info!("Exit notification received. Server shutting down...");
108-
Ok(true)
109-
},
110-
message => anyhow::bail!("Server received unexpected message {message:?} while waiting for exit notification")
105+
106+
loop {
107+
match &self
108+
.receiver
109+
.recv_timeout(std::time::Duration::from_secs(30))?
110+
{
111+
lsp::Message::Notification(lsp::Notification { method, .. })
112+
if method == lsp_types::notification::Exit::METHOD =>
113+
{
114+
tracing::info!("Exit notification received. Server shutting down...");
115+
return Ok(true);
116+
}
117+
lsp::Message::Request(lsp::Request { id, method, .. }) => {
118+
tracing::warn!(
119+
"Server received unexpected request {method} ({id}) while waiting for exit notification",
120+
);
121+
self.sender.send(lsp::Message::Response(lsp::Response::new_err(
122+
id.clone(),
123+
lsp::ErrorCode::InvalidRequest as i32,
124+
"Server received unexpected request while waiting for exit notification".to_string(),
125+
)))?;
126+
}
127+
message => {
128+
tracing::warn!(
129+
"Server received unexpected message while waiting for exit notification: {message:?}"
130+
);
131+
}
132+
}
111133
}
112134
}
113135
lsp::Message::Notification(lsp::Notification { method, .. })
114136
if method == lsp_types::notification::Exit::METHOD =>
115137
{
116-
tracing::error!("Server received an exit notification before a shutdown request was sent. Exiting...");
117-
Ok(true)
138+
anyhow::bail!("Server received an exit notification before a shutdown request was sent. Exiting...");
118139
}
119140
_ => Ok(false),
120141
}

engine/language_server/src/session.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,12 @@ impl Session {
197197

198198
Ok(())
199199
}
200+
pub fn clear_unsaved_files(&mut self) {
201+
tracing::info!("Clearing unsaved files");
202+
for (_folder, project) in self.projects_by_workspace_folder.lock().unwrap().iter_mut() {
203+
project.lock().unwrap().baml_project.unsaved_files.clear();
204+
}
205+
}
200206

201207
/// Creates a document snapshot with the URL referencing the document to snapshot.
202208
pub fn take_snapshot(&self, url: Url) -> Option<DocumentSnapshot> {

typescript/vscode-ext/packages/vscode/src/plugins/language-server/index.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { bamlConfig, getConfig } from './bamlConfig'
2727

2828
export { bamlConfig }
2929
const packageJson = require('../../../../package.json') // eslint-disable-line
30+
let clientReady = false
3031

3132
let client: LanguageClient
3233
let serverModule: string
@@ -46,6 +47,10 @@ export const requestDiagnostics = async () => {
4647
if (!currentFile.endsWith('.baml')) {
4748
return
4849
}
50+
if (!clientReady) {
51+
console.warn('client not ready')
52+
return
53+
}
4954
await client?.sendRequest('requestDiagnostics', { projectId: currentFile })
5055
}
5156

@@ -141,6 +146,7 @@ const activateClient = (
141146
.onReady()
142147
.then(() => {
143148
console.log('client ready')
149+
clientReady = true
144150
client.createDefaultErrorHandler(2)
145151
requestDiagnostics()
146152
client.onNotification('baml/showLanguageServerOutput', () => {

0 commit comments

Comments
 (0)