diff --git a/crates/repl/src/notebook/cell.rs b/crates/repl/src/notebook/cell.rs index 2baf05a2d5..92e4d42c73 100644 --- a/crates/repl/src/notebook/cell.rs +++ b/crates/repl/src/notebook/cell.rs @@ -820,6 +820,25 @@ impl CodeCell { self.is_executing } + /// Displays a kernel-level failure (e.g. the kernel failed to launch because + /// Python is not installed) as an error output on this cell, so the user gets + /// feedback instead of a spinner that never resolves. + pub fn show_kernel_error( + &mut self, + error_message: &str, + window: &mut Window, + cx: &mut Context, + ) { + self.outputs.push(Output::ErrorOutput(ErrorView { + ename: "Kernel Error".to_string(), + evalue: "cell could not be executed".to_string(), + traceback: cx.new(|cx| TerminalOutput::from(error_message, window, cx)), + })); + self.execution_start_time = None; + self.is_executing = false; + cx.notify(); + } + pub fn execution_duration(&self) -> Option { self.execution_duration } diff --git a/crates/repl/src/notebook/notebook_ui.rs b/crates/repl/src/notebook/notebook_ui.rs index d1c44667f9..235a711428 100644 --- a/crates/repl/src/notebook/notebook_ui.rs +++ b/crates/repl/src/notebook/notebook_ui.rs @@ -138,9 +138,15 @@ impl NotebookEditor { match &cell_entity { Cell::Code(code_cell) => { let cell_id_for_focus = cell_id.clone(); - cx.subscribe(code_cell, move |this, _cell, event, cx| match event { - CellEvent::Run(cell_id) => this.execute_cell(cell_id.clone(), cx), - CellEvent::FocusedIn(_) => this.select_cell_by_id(&cell_id_for_focus, cx), + cx.subscribe_in(code_cell, window, move |this, _cell, event, window, cx| { + match event { + CellEvent::Run(cell_id) => { + this.execute_cell(cell_id.clone(), window, cx) + } + CellEvent::FocusedIn(_) => { + this.select_cell_by_id(&cell_id_for_focus, cx) + } + } }) .detach(); @@ -509,7 +515,7 @@ impl NotebookEditor { } } - fn execute_cell(&mut self, cell_id: CellId, cx: &mut Context) { + fn execute_cell(&mut self, cell_id: CellId, window: &mut Window, cx: &mut Context) { let code = if let Some(Cell::Code(cell)) = self.cell_map.get(&cell_id) { let editor = cell.read(cx).editor().clone(); let buffer = editor.read(cx).buffer().read(cx); @@ -521,16 +527,6 @@ impl NotebookEditor { return; }; - if let Some(Cell::Code(cell)) = self.cell_map.get(&cell_id) { - cell.update(cx, |cell, cx| { - if cell.has_outputs() { - cell.clear_outputs(); - } - cell.start_execution(); - cx.notify(); - }); - } - let request = ExecuteRequest { code, ..Default::default() @@ -538,10 +534,35 @@ impl NotebookEditor { let message: JupyterMessage = request.into(); let msg_id = message.header.msg_id.clone(); - self.execution_requests.insert(msg_id, cell_id.clone()); + let send_result = match &mut self.kernel { + Kernel::RunningKernel(kernel) => kernel + .request_tx() + .try_send(message) + .map_err(|err| format!("failed to send execute request to kernel (the kernel process may have died): {err}")), + Kernel::StartingKernel(_) => Err("the kernel is still starting".to_string()), + Kernel::ErroredLaunch(error) => Err(format!("the kernel failed to launch: {error}")), + Kernel::ShuttingDown | Kernel::Shutdown => Err("the kernel is shut down".to_string()), + Kernel::Restarting => Err("the kernel is restarting".to_string()), + }; - if let Kernel::RunningKernel(kernel) = &mut self.kernel { - kernel.request_tx().try_send(message).ok(); + if let Some(Cell::Code(cell)) = self.cell_map.get(&cell_id) { + cell.update(cx, |cell, cx| { + if cell.has_outputs() { + cell.clear_outputs(); + } + if let Err(error) = &send_result { + cell.show_kernel_error(error, window, cx); + } else { + cell.start_execution(); + } + cx.notify(); + }); + } + + if let Err(error) = send_result { + log::error!("notebook: cannot execute cell: {error}"); + } else { + self.execution_requests.insert(msg_id, cell_id.clone()); } } @@ -575,7 +596,7 @@ impl NotebookEditor { fn run_cells(&mut self, window: &mut Window, cx: &mut Context) { for cell_id in self.cell_order.clone() { - self.execute_cell(cell_id, cx); + self.execute_cell(cell_id, window, cx); } } @@ -588,7 +609,7 @@ impl NotebookEditor { }; match cell { Cell::Code(_) => { - self.execute_cell(cell_id, cx); + self.execute_cell(cell_id, window, cx); } Cell::Markdown(markdown_cell) => { // for markdown, finish editing and move to next cell @@ -609,7 +630,7 @@ impl NotebookEditor { if let Some(cell) = self.cell_map.get(&cell_id) { match cell { Cell::Code(_) => { - self.execute_cell(cell_id, cx); + self.execute_cell(cell_id, window, cx); } Cell::Markdown(markdown_cell) => { if markdown_cell.read(cx).is_editing() { @@ -820,10 +841,14 @@ impl NotebookEditor { }); let cell_id_for_run = new_cell_id.clone(); - cx.subscribe(&code_cell, move |this, _cell, event, cx| match event { - CellEvent::Run(cell_id) => this.execute_cell(cell_id.clone(), cx), - CellEvent::FocusedIn(_) => this.select_cell_by_id(&cell_id_for_run, cx), - }) + cx.subscribe_in( + &code_cell, + window, + move |this, _cell, event, window, cx| match event { + CellEvent::Run(cell_id) => this.execute_cell(cell_id.clone(), window, cx), + CellEvent::FocusedIn(_) => this.select_cell_by_id(&cell_id_for_run, cx), + }, + ) .detach(); let cell_id_for_editor = new_cell_id.clone(); @@ -1904,3 +1929,172 @@ impl KernelSession for NotebookEditor { cx.notify(); } } + +#[cfg(test)] +mod tests { + use super::*; + use gpui::TestAppContext; + use project::{FakeFs, Project, ProjectItem as _}; + use serde_json::json; + use settings::SettingsStore; + use util::path; + use util::rel_path::rel_path; + + const NOTEBOOK_WITH_ONE_CODE_CELL: &str = r#"{ + "metadata": { + "kernelspec": { + "display_name": "Python 3", + "language": "python", + "name": "python3" + }, + "language_info": { + "name": "python" + } + }, + "nbformat": 4, + "nbformat_minor": 5, + "cells": [ + { + "cell_type": "code", + "id": "cell-one", + "metadata": {}, + "execution_count": null, + "outputs": [], + "source": ["print('hello')"] + } + ] + }"#; + + /// When the configured interpreter doesn't exist (e.g. Python isn't installed), + /// running a cell must not leave it stuck in the executing state. It should + /// instead surface the kernel launch error as an error output on the cell. + #[gpui::test] + async fn test_run_cell_with_missing_interpreter_shows_error(cx: &mut TestAppContext) { + cx.update(|cx| { + let settings_store = SettingsStore::test(cx); + cx.set_global(settings_store); + theme_settings::init(theme::LoadThemes::JustBase, cx); + editor::init(cx); + }); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/notebooks"), + json!({ "test.ipynb": NOTEBOOK_WITH_ONE_CODE_CELL }), + ) + .await; + + let project = Project::test(fs.clone(), [path!("/notebooks").as_ref()], cx).await; + cx.update(|cx| ReplStore::init(fs.clone(), cx)); + + let worktree_id = project.read_with(cx, |project, cx| { + project.worktrees(cx).next().unwrap().read(cx).id() + }); + + // Select a kernel whose interpreter doesn't exist, simulating a machine + // where Python isn't installed properly. This is the same path the + // kernel picker uses. + let missing_interpreter = path!("/nonexistent/python3"); + let broken_spec = KernelSpecification::Jupyter(LocalKernelSpecification { + name: "python3".to_string(), + path: PathBuf::from(missing_interpreter), + kernelspec: JupyterKernelspec { + argv: vec![ + missing_interpreter.to_string(), + "-m".to_string(), + "ipykernel_launcher".to_string(), + "-f".to_string(), + "{connection_file}".to_string(), + ], + display_name: "Python 3".to_string(), + language: "python".to_string(), + interrupt_mode: None, + metadata: None, + env: None, + }, + }); + cx.update(|cx| { + ReplStore::global(cx).update(cx, |store, cx| { + store.set_active_kernelspec(worktree_id, broken_spec, cx); + }) + }); + + let notebook_item = cx + .update(|cx| { + NotebookItem::try_open( + &project, + &ProjectPath { + worktree_id, + path: rel_path("test.ipynb").into(), + }, + cx, + ) + .expect("ipynb files should be openable as notebooks") + }) + .await + .expect("notebook should parse"); + + // Don't render the notebook UI itself: its animated kernel status icon + // schedules a new frame on every render, which makes `run_until_parked` + // spin forever in tests. The editor entity is created inside an empty + // window instead; we are testing execution behavior, not rendering. + let cx = cx.add_empty_window(); + + // Launching a kernel probes real TCP ports on localhost, which the + // deterministic test scheduler cannot drive. + cx.executor().allow_parking(); + + let editor = cx.update(|window, cx| { + cx.new(|cx| NotebookEditor::new(project.clone(), notebook_item, window, cx)) + }); + + // Creating the editor launches the kernel. Wait for the actual launch + // task, which fails because the interpreter cannot be spawned. + let pending_kernel = editor.read_with(cx, |editor, _| match &editor.kernel { + Kernel::StartingKernel(task) => task.clone(), + _ => panic!("kernel should be starting right after the editor is created"), + }); + pending_kernel.await; + + editor.read_with(cx, |editor, _| { + assert!( + matches!(editor.kernel, Kernel::ErroredLaunch(_)), + "kernel launch should fail, instead status is: {}", + editor.kernel.status().to_string() + ); + }); + + // Run the (only) cell via the production action handler. + editor.update_in(cx, |editor, window, cx| { + editor.run_current_cell(&Run, window, cx); + }); + + editor.read_with(cx, |editor, cx| { + let cell_id = editor.cell_order.first().expect("notebook has one cell"); + let Some(Cell::Code(cell)) = editor.cell_map.get(cell_id) else { + panic!("expected a code cell"); + }; + let cell = cell.read(cx); + + assert!( + !cell.is_executing(), + "cell must not be stuck in the executing state when the kernel is not running" + ); + + let nbformat::v4::Cell::Code { outputs, .. } = cell.to_nbformat_cell(cx) else { + panic!("expected a code cell"); + }; + match outputs.as_slice() { + [nbformat::v4::Output::Error(error)] => { + assert_eq!(error.ename, "Kernel Error"); + let traceback = error.traceback.join("\n"); + assert!( + traceback.contains("the kernel failed to launch"), + "error output should explain why the cell could not run, got: {traceback}" + ); + } + other => panic!("expected a single error output, got: {other:?}"), + } + }); + } +}