LSP: Report error through the LSP when the preview can't open

Instead of panicking the whole server

Issue #204

tools/lsp/common.rs

@@ -673,6 +673,8 @@ pub enum PreviewToLspMessage {
     RequestState { unused: bool },
     /// Pass a `WorkspaceEdit` on to the editor
     SendWorkspaceEdit { label: Option<String>, edit: lsp_types::WorkspaceEdit },
+    /// Pass a `ShowMessage` notification on to the editor
+    SendShowMessage { message: lsp_types::ShowMessageParams },
 }
 
 /// Information on the Element types available

tools/lsp/main.rs

@@ -22,10 +22,7 @@ use i_slint_compiler::CompilerConfiguration;
 use lsp_types::notification::{
     DidChangeConfiguration, DidChangeTextDocument, DidOpenTextDocument, Notification,
 };
-use lsp_types::{
+use lsp_types::{DidChangeTextDocumentParams, DidOpenTextDocumentParams, InitializeParams, Url};
-    DidChangeTextDocumentParams, DidOpenTextDocumentParams, InitializeParams, ShowMessageParams,
-    Url,
-};
 
 use clap::{Args, Parser, Subcommand};
 use itertools::Itertools;
@@ -161,7 +158,7 @@ impl ServerNotifier {
             let _ = self.send_notification::<common::LspToPreviewMessage>(message);
         } else {
             #[cfg(feature = "preview-builtin")]
-            preview::lsp_to_preview_message(message, self);
+            preview::lsp_to_preview_message(message);
         }
     }
 
@@ -287,6 +284,9 @@ fn main_loop(connection: Connection, init_param: InitializeParams, cli_args: Cli
         preview_to_lsp_sender,
     };
 
+    #[cfg(feature = "preview-builtin")]
+    preview::set_server_notifier(server_notifier.clone());
+
     let mut compiler_config =
         CompilerConfiguration::new(i_slint_compiler::generator::OutputFormat::Interpreter);
 
@@ -435,7 +435,7 @@ async fn handle_notification(req: lsp_server::Notification, ctx: &Rc<Context>) -
                     LspErrorCode::RequestFailed => ctx
                         .server_notifier
                         .send_notification::<lsp_types::notification::ShowMessage>(
-                        ShowMessageParams {
+                        lsp_types::ShowMessageParams {
                             typ: lsp_types::MessageType::ERROR,
                             message: e.message,
                         },
@@ -514,6 +514,10 @@ async fn handle_preview_to_lsp_message(
         M::SendWorkspaceEdit { label, edit } => {
             let _ = send_workspace_edit(ctx.server_notifier.clone(), label, Ok(edit)).await;
         }
+        M::SendShowMessage { message } => {
+            ctx.server_notifier
+                .send_notification::<lsp_types::notification::ShowMessage>(message)?;
+        }
     }
     Ok(())
 }

tools/lsp/preview.rs

@@ -1,10 +1,9 @@
 // Copyright © SixtyFPS GmbH <info@slint.dev>
 // SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0
 
-use crate::common::properties;
 use crate::common::{
-    self, component_catalog, rename_component, ComponentInformation, ElementRcNode,
+    self, component_catalog, properties, rename_component, ComponentInformation, ElementRcNode,
-    PreviewComponent, PreviewConfig,
+    PreviewComponent, PreviewConfig, PreviewToLspMessage,
 };
 use crate::lsp_ext::Health;
 use crate::preview::element_selection::ElementSelection;
@@ -16,7 +15,7 @@ use i_slint_core::component_factory::FactoryContext;
 use i_slint_core::lengths::{LogicalPoint, LogicalRect, LogicalSize};
 use i_slint_core::model::VecModel;
 use lsp_types::Url;
-use slint::Model;
+use slint::{Model, PlatformError};
 use slint_interpreter::{ComponentDefinition, ComponentHandle, ComponentInstance};
 use std::cell::RefCell;
 use std::collections::{HashMap, HashSet};
@@ -131,13 +130,12 @@ struct PreviewState {
 }
 thread_local! {static PREVIEW_STATE: std::cell::RefCell<PreviewState> = Default::default();}
 
-struct DummyWaker();
-
-impl std::task::Wake for DummyWaker {
-    fn wake(self: std::sync::Arc<Self>) {}
-}
-
 pub fn poll_once<F: std::future::Future>(future: F) -> Option<F::Output> {
+    struct DummyWaker();
+    impl std::task::Wake for DummyWaker {
+        fn wake(self: std::sync::Arc<Self>) {}
+    }
+
     let waker = std::sync::Arc::new(DummyWaker()).into();
     let mut ctx = std::task::Context::from_waker(&waker);
 
@@ -149,7 +147,7 @@ pub fn poll_once<F: std::future::Future>(future: F) -> Option<F::Output> {
     }
 }
 
-pub fn set_contents(url: &common::VersionedUrl, content: String) {
+fn set_contents(url: &common::VersionedUrl, content: String) {
     let mut cache = CONTENT_CACHE.get_or_init(Default::default).lock().unwrap();
     let old = cache.source_code.insert(url.url().clone(), (*url.version(), content.clone()));
 
@@ -803,10 +801,7 @@ fn send_workspace_edit(label: String, edit: lsp_types::WorkspaceEdit) -> bool {
     });
 
     if !workspace_edit_sent {
-        send_message_to_lsp(common::PreviewToLspMessage::SendWorkspaceEdit {
+        send_message_to_lsp(PreviewToLspMessage::SendWorkspaceEdit { label: Some(label), edit });
-            label: Some(label),
-            edit,
-        });
         true
     } else {
         false
@@ -903,7 +898,7 @@ fn finish_parsing(ok: bool) {
     }
 }
 
-pub fn config_changed(config: PreviewConfig) {
+fn config_changed(config: PreviewConfig) {
     if let Some(cache) = CONTENT_CACHE.get() {
         let mut cache = cache.lock().unwrap();
         if cache.config != config {
@@ -941,8 +936,11 @@ fn get_path_from_cache(path: &Path) -> Option<(common::UrlVersion, String)> {
     get_url_from_cache(&url)
 }
 
+#[derive(Copy, Clone, PartialEq, Eq)]
 pub enum LoadBehavior {
+    /// We reload the preview, most likely because a file has changed
     Reload,
+    /// We load the preview because the user asked for it. The UI should become visible if it wasn't already
     Load,
 }
 
@@ -950,13 +948,14 @@ pub fn load_preview(preview_component: PreviewComponent, behavior: LoadBehavior)
     {
         let mut cache = CONTENT_CACHE.get_or_init(Default::default).lock().unwrap();
         match behavior {
-            LoadBehavior::Reload => { /* do nothing */ }
+            LoadBehavior::Reload => {
+                if !cache.ui_is_visible {
+                    return;
+                }
+            }
             LoadBehavior::Load => cache.push_component(preview_component),
         }
 
-        if !cache.ui_is_visible {
-            return;
-        }
         match cache.loading_state {
             PreviewFutureState::Pending => (),
             PreviewFutureState::PreLoading => return,
@@ -969,7 +968,7 @@ pub fn load_preview(preview_component: PreviewComponent, behavior: LoadBehavior)
         cache.loading_state = PreviewFutureState::PreLoading;
     };
 
-    run_in_ui_thread(move || async move {
+    let result = run_in_ui_thread(move || async move {
         let (selected, notify_editor) = PREVIEW_STATE.with(|preview_state| {
             let mut preview_state = preview_state.borrow_mut();
             let notify_editor = preview_state.notify_editor_about_selection_after_update;
@@ -984,7 +983,7 @@ pub fn load_preview(preview_component: PreviewComponent, behavior: LoadBehavior)
                 cache.clear_style_of_component();
 
                 assert_eq!(cache.loading_state, PreviewFutureState::PreLoading);
-                if !cache.ui_is_visible {
+                if !cache.ui_is_visible && behavior != LoadBehavior::Load {
                     cache.loading_state = PreviewFutureState::Pending;
                     return;
                 }
@@ -1013,7 +1012,15 @@ pub fn load_preview(preview_component: PreviewComponent, behavior: LoadBehavior)
                 }
             });
 
-            reload_preview_impl(preview_component, style, config).await;
+            match reload_preview_impl(preview_component, style, config).await {
+                Ok(()) => {}
+                Err(e) => {
+                    CONTENT_CACHE.get_or_init(Default::default).lock().unwrap().loading_state =
+                        PreviewFutureState::Pending;
+                    send_platform_error_notification(&e.to_string());
+                    return;
+                }
+            }
 
             let mut cache = CONTENT_CACHE.get_or_init(Default::default).lock().unwrap();
             match cache.loading_state {
@@ -1061,6 +1068,12 @@ pub fn load_preview(preview_component: PreviewComponent, behavior: LoadBehavior)
             }
         }
     });
+
+    if let Err(e) = result {
+        CONTENT_CACHE.get_or_init(Default::default).lock().unwrap().loading_state =
+            PreviewFutureState::Pending;
+        send_platform_error_notification(&e);
+    }
 }
 
 async fn parse_source(
@@ -1103,7 +1116,11 @@ async fn parse_source(
 }
 
 // Must be inside the thread running the slint event loop
-async fn reload_preview_impl(component: PreviewComponent, style: String, config: PreviewConfig) {
+async fn reload_preview_impl(
+    component: PreviewComponent,
+    style: String,
+    config: PreviewConfig,
+) -> Result<(), PlatformError> {
     start_parsing();
 
     let path = component.url.to_file_path().unwrap_or(PathBuf::from(&component.url.to_string()));
@@ -1125,8 +1142,19 @@ async fn reload_preview_impl(component: PreviewComponent, style: String, config:
     notify_diagnostics(&diagnostics);
 
     let success = compiled.is_some();
-    update_preview_area(compiled);
+    update_preview_area(compiled)?;
     finish_parsing(success);
+    Ok(())
+}
+
+/// Sends a notification back to the editor when the preview fails to load because of a slint::PlatormError.
+fn send_platform_error_notification(platform_error_str: &str) {
+    let message = format!("Error displaying the Slint preview window: {platform_error_str}");
+    // Also output the message in the console in case the user missed the notification in the editor
+    eprintln!("{message}");
+    send_message_to_lsp(PreviewToLspMessage::SendShowMessage {
+        message: lsp_types::ShowMessageParams { typ: lsp_types::MessageType::ERROR, message },
+    })
 }
 
 /// This sets up the preview area to show the ComponentInstance
@@ -1174,7 +1202,7 @@ pub fn highlight(url: Option<Url>, offset: u32) {
     let selected = selected_element();
 
     if cache.highlight.as_ref().map_or(true, |(url, _)| cache.dependency.contains(url)) {
-        run_in_ui_thread(move || async move {
+        let _ = run_in_ui_thread(move || async move {
             let Some(path) = url.and_then(|u| Url::to_file_path(&u).ok()) else {
                 return;
             };
@@ -1184,7 +1212,7 @@ pub fn highlight(url: Option<Url>, offset: u32) {
                 return;
             }
             element_selection::select_element_at_source_code_position(path, offset, None, false);
-        })
+        });
     }
 }
 
@@ -1376,7 +1404,7 @@ fn document_cache_from(preview_state: &PreviewState) -> Option<Rc<common::Docume
 }
 
 fn set_show_preview_ui(show_preview_ui: bool) {
-    run_in_ui_thread(move || async move {
+    let _ = run_in_ui_thread(move || async move {
         PREVIEW_STATE.with(|preview_state| {
             let preview_state = preview_state.borrow();
             if let Some(ui) = &preview_state.ui {
@@ -1440,14 +1468,14 @@ fn set_diagnostics(diagnostics: &[slint_interpreter::Diagnostic]) {
     .unwrap();
 }
 
-/// This runs `set_preview_factory` in the UI thread
+/// This ensure that the preview window is visible and runs `set_preview_factory`
-fn update_preview_area(compiled: Option<ComponentDefinition>) {
+fn update_preview_area(compiled: Option<ComponentDefinition>) -> Result<(), PlatformError> {
     PREVIEW_STATE.with(move |preview_state| {
         let mut preview_state = preview_state.borrow_mut();
         preview_state.workspace_edit_sent = false;
 
         #[cfg(not(target_arch = "wasm32"))]
-        native::open_ui_impl(&mut preview_state);
+        native::open_ui_impl(&mut preview_state)?;
 
         let ui = preview_state.ui.as_ref().unwrap();
 
@@ -1470,15 +1498,13 @@ fn update_preview_area(compiled: Option<ComponentDefinition>) {
             reset_selections(ui);
         }
 
-        ui.show().unwrap();
+        ui.show()
-    });
+    })?;
     element_selection::reselect_element();
+    Ok(())
 }
 
-pub fn lsp_to_preview_message(
+pub fn lsp_to_preview_message(message: crate::common::LspToPreviewMessage) {
-    message: crate::common::LspToPreviewMessage,
-    #[cfg(not(target_arch = "wasm32"))] sender: &crate::ServerNotifier,
-) {
     use crate::common::LspToPreviewMessage as M;
     match message {
         M::SetContents { url, contents } => {
@@ -1488,8 +1514,6 @@ pub fn lsp_to_preview_message(
             config_changed(config);
         }
         M::ShowPreview(pc) => {
-            #[cfg(not(target_arch = "wasm32"))]
-            native::open_ui(sender);
             load_preview(pc, LoadBehavior::Load);
         }
         M::HighlightFromEditor { url, offset } => {

tools/lsp/preview/native.rs

@@ -4,6 +4,7 @@
 // cSpell: ignore condvar
 
 use super::PreviewState;
+use crate::common::PreviewToLspMessage;
 use crate::lsp_ext::Health;
 use crate::ServerNotifier;
 use once_cell::sync::Lazy;
@@ -11,7 +12,7 @@ use slint_interpreter::ComponentHandle;
 use std::future::Future;
 use std::sync::{Condvar, Mutex};
 
-#[derive(PartialEq)]
+#[derive(PartialEq, Debug)]
 enum RequestedGuiEventLoopState {
     /// The UI event loop hasn't been started yet because no preview has been requested
     Uninitialized,
@@ -22,6 +23,8 @@ enum RequestedGuiEventLoopState {
     LoopStarted,
     /// The LSP thread requested the application to be terminated
     QuitLoop,
+    /// There was an error when initializing the UI thread
+    InitializationError(String),
 }
 
 static GUI_EVENT_LOOP_NOTIFIER: Lazy<Condvar> = Lazy::new(Condvar::new);
@@ -32,7 +35,7 @@ thread_local! {static CLI_ARGS: std::cell::OnceCell<crate::Cli> = Default::defau
 
 pub fn run_in_ui_thread<F: Future<Output = ()> + 'static>(
     create_future: impl Send + FnOnce() -> F + 'static,
-) {
+) -> Result<(), String> {
     // Wake up the main thread to start the event loop, if possible
     {
         let mut state_request = GUI_EVENT_LOOP_STATE_REQUEST.lock().unwrap();
@@ -44,19 +47,28 @@ pub fn run_in_ui_thread<F: Future<Output = ()> + 'static>(
         while *state_request == RequestedGuiEventLoopState::StartLoop {
             state_request = GUI_EVENT_LOOP_NOTIFIER.wait(state_request).unwrap();
         }
+
+        if let RequestedGuiEventLoopState::InitializationError(err) = &*state_request {
+            return Err(err.clone());
+        }
     }
     i_slint_core::api::invoke_from_event_loop(move || {
         i_slint_core::future::spawn_local(create_future()).unwrap();
     })
     .unwrap();
+    Ok(())
 }
 
+/// This is the main entry for the Slint event loop. It runs on the main thread,
+/// but only runs the event loop if a preview is requested to avoid potential
+/// crash so that the LSP works without preview in that case.
 pub fn start_ui_event_loop(cli_args: crate::Cli) {
     CLI_ARGS.with(|f| f.set(cli_args).ok());
 
     {
         let mut state_requested = GUI_EVENT_LOOP_STATE_REQUEST.lock().unwrap();
 
+        // Wait until we either quit, or the LSP thread request to start the loop
         while *state_requested == RequestedGuiEventLoopState::Uninitialized {
             state_requested = GUI_EVENT_LOOP_NOTIFIER.wait(state_requested).unwrap();
         }
@@ -67,7 +79,18 @@ pub fn start_ui_event_loop(cli_args: crate::Cli) {
 
         if *state_requested == RequestedGuiEventLoopState::StartLoop {
             // make sure the backend is initialized
-            i_slint_backend_selector::with_platform(|_| Ok(())).unwrap();
+            match i_slint_backend_selector::with_platform(|_| Ok(())) {
+                Ok(_) => {}
+                Err(err) => {
+                    *state_requested =
+                        RequestedGuiEventLoopState::InitializationError(err.to_string());
+                    GUI_EVENT_LOOP_NOTIFIER.notify_one();
+                    while *state_requested != RequestedGuiEventLoopState::QuitLoop {
+                        state_requested = GUI_EVENT_LOOP_NOTIFIER.wait(state_requested).unwrap();
+                    }
+                    return;
+                }
+            };
             // Send an event so that once the loop is started, we notify the LSP thread that it can send more events
             i_slint_core::api::invoke_from_event_loop(|| {
                 let mut state_request = GUI_EVENT_LOOP_STATE_REQUEST.lock().unwrap();
@@ -96,49 +119,11 @@ pub fn quit_ui_event_loop() {
 
     let _ = i_slint_core::api::quit_event_loop();
 
-    // Make sure then sender channel gets dropped.
+    // Make sure then sender channel gets dropped, otherwise the lsp thread will never quit
-    if let Some(sender) = SERVER_NOTIFIER.get() {
+    *SERVER_NOTIFIER.lock().unwrap() = None
-        let mut sender = sender.lock().unwrap();
-        *sender = None;
-    };
 }
 
-pub fn open_ui(sender: &ServerNotifier) {
+pub(super) fn open_ui_impl(preview_state: &mut PreviewState) -> Result<(), slint::PlatformError> {
-    // Wake up the main thread to start the event loop, if possible
-    {
-        let mut state_request = GUI_EVENT_LOOP_STATE_REQUEST.lock().unwrap();
-        if *state_request == RequestedGuiEventLoopState::Uninitialized {
-            *state_request = RequestedGuiEventLoopState::StartLoop;
-            GUI_EVENT_LOOP_NOTIFIER.notify_one();
-        }
-        // We don't want to call post_event before the loop is properly initialized
-        while *state_request == RequestedGuiEventLoopState::StartLoop {
-            state_request = GUI_EVENT_LOOP_NOTIFIER.wait(state_request).unwrap();
-        }
-    }
-
-    {
-        let mut cache = super::CONTENT_CACHE.get_or_init(Default::default).lock().unwrap();
-        if cache.ui_is_visible {
-            return; // UI is already up!
-        }
-        cache.ui_is_visible = true;
-
-        let mut s = SERVER_NOTIFIER.get_or_init(Default::default).lock().unwrap();
-        *s = Some(sender.clone());
-    };
-
-    i_slint_core::api::invoke_from_event_loop(move || {
-        super::PREVIEW_STATE.with(|preview_state| {
-            let mut preview_state = preview_state.borrow_mut();
-
-            open_ui_impl(&mut preview_state);
-        });
-    })
-    .unwrap();
-}
-
-pub(super) fn open_ui_impl(preview_state: &mut PreviewState) {
     let (default_style, show_preview_ui, fullscreen) = {
         let cache = super::CONTENT_CACHE.get_or_init(Default::default).lock().unwrap();
         let style = cache.config.style.clone();
@@ -161,21 +146,23 @@ pub(super) fn open_ui_impl(preview_state: &mut PreviewState) {
         .map(|s| !s.is_empty() && s != "0")
         .unwrap_or(false);
 
-    let ui = preview_state
+    let ui = match preview_state.ui.as_ref() {
-        .ui
+        Some(ui) => ui,
-        .get_or_insert_with(|| super::ui::create_ui(default_style, experimental).unwrap());
+        None => {
+            let ui = super::ui::create_ui(default_style, experimental)?;
+            preview_state.ui.insert(ui)
+        }
+    };
+
     let api = ui.global::<crate::preview::ui::Api>();
     api.set_show_preview_ui(show_preview_ui);
     ui.window().set_fullscreen(fullscreen);
     ui.window().on_close_requested(|| {
         let mut cache = super::CONTENT_CACHE.get_or_init(Default::default).lock().unwrap();
         cache.ui_is_visible = false;
-
-        let mut sender = SERVER_NOTIFIER.get_or_init(Default::default).lock().unwrap();
-        *sender = None;
-
         slint::CloseRequestResponse::HideWindow
     });
+    Ok(())
 }
 
 pub fn close_ui() {
@@ -203,13 +190,17 @@ fn close_ui_impl(preview_state: &mut PreviewState) {
     }
 }
 
-static SERVER_NOTIFIER: std::sync::OnceLock<Mutex<Option<ServerNotifier>>> =
+static SERVER_NOTIFIER: Mutex<Option<ServerNotifier>> = Mutex::new(None);
-    std::sync::OnceLock::new();
+
+/// Give the UI thread a handle to send message back to the LSP thread
+pub fn set_server_notifier(sender: ServerNotifier) {
+    *SERVER_NOTIFIER.lock().unwrap() = Some(sender);
+}
 
 pub fn notify_diagnostics(diagnostics: &[slint_interpreter::Diagnostic]) -> Option<()> {
     super::set_diagnostics(diagnostics);
 
-    let Some(sender) = SERVER_NOTIFIER.get_or_init(Default::default).lock().unwrap().clone() else {
+    let Some(sender) = SERVER_NOTIFIER.lock().unwrap().clone() else {
         return Some(());
     };
 
@@ -222,7 +213,7 @@ pub fn notify_diagnostics(diagnostics: &[slint_interpreter::Diagnostic]) -> Opti
 }
 
 pub fn send_status(message: &str, health: Health) {
-    let Some(sender) = SERVER_NOTIFIER.get_or_init(Default::default).lock().unwrap().clone() else {
+    let Some(sender) = SERVER_NOTIFIER.lock().unwrap().clone() else {
         return;
     };
 
@@ -230,7 +221,7 @@ pub fn send_status(message: &str, health: Health) {
 }
 
 pub fn ask_editor_to_show_document(file: &str, selection: lsp_types::Range) {
-    let Some(sender) = SERVER_NOTIFIER.get_or_init(Default::default).lock().unwrap().clone() else {
+    let Some(sender) = SERVER_NOTIFIER.lock().unwrap().clone() else {
         return;
     };
     let Ok(url) = lsp_types::Url::from_file_path(file) else { return };
@@ -238,8 +229,8 @@ pub fn ask_editor_to_show_document(file: &str, selection: lsp_types::Range) {
     slint_interpreter::spawn_local(fut).unwrap(); // Fire and forget.
 }
 
-pub fn send_message_to_lsp(message: crate::common::PreviewToLspMessage) {
+pub fn send_message_to_lsp(message: PreviewToLspMessage) {
-    let Some(sender) = SERVER_NOTIFIER.get_or_init(Default::default).lock().unwrap().clone() else {
+    let Some(sender) = SERVER_NOTIFIER.lock().unwrap().clone() else {
         return;
     };
     sender.send_message_to_lsp(message);

tools/lsp/preview/wasm.rs

@@ -179,8 +179,9 @@ fn invoke_from_event_loop_wrapped_in_promise(
 
 pub fn run_in_ui_thread<F: Future<Output = ()> + 'static>(
     create_future: impl Send + FnOnce() -> F + 'static,
-) {
+) -> Result<(), String> {
-    i_slint_core::future::spawn_local(create_future()).unwrap();
+    i_slint_core::future::spawn_local(create_future()).map_err(|e| e.to_string())?;
+    Ok(())
 }
 
 pub fn resource_url_mapper(

tools/lsp/wasm_main.rs

@@ -302,6 +302,12 @@ impl SlintServer {
             M::SendWorkspaceEdit { label, edit } => {
                 send_workspace_edit(self.ctx.server_notifier.clone(), label, Ok(edit));
             }
+            M::SendShowMessage { message } => {
+                let _ = self
+                    .ctx
+                    .server_notifier
+                    .send_notification::<lsp_types::notification::ShowMessage>(message);
+            }
         }
         Ok(())
     }