repl: Show kernel error on notebook cell instead of hanging (#59137)

Closes AI-394

When the kernel isn't running — e.g. Python isn't installed, ipykernel
is missing, or the kernel failed to launch — clicking run on a notebook
cell would unconditionally enter the executing state and silently drop
the execute request. The cell then spun forever with no logs and no way
to cancel (the stop button only works on a running kernel).

This change makes `execute_cell` attempt to send the execute request
first, and only mark the cell as executing when the send succeeds. On
failure it:

- Renders the failure (including the original kernel launch error) as an
error output on the cell, reusing the same `ErrorView` used for Python
tracebacks
- Logs the reason at the moment run is clicked, instead of only once at
notebook open
- No longer swallows `try_send` errors when the kernel process has died

Also adds the first unit test for the notebook editor, which exercises
the production path end to end: a real `.ipynb` parsed via
`NotebookItem::try_open`, a kernelspec pointing at a nonexistent
interpreter selected through `ReplStore::set_active_kernelspec`, a real
failed process spawn, and assertions through the production save path
(`to_nbformat_cell`). Verified red/green: the test fails against the
previous behavior.

Note for future notebook tests: `NotebookEditor` renders a continuously
animating kernel status icon, so rendering it as a root view in a GPUI
test makes `run_until_parked` spin forever. The test creates the editor
inside an empty window instead.

Release Notes:

- N/A
This commit is contained in:
MartinYe1234 authored and GitHub committed 2026-06-12 14:57:52 +00:00
1 parent 89d002100f
commit eaa996bbec
2 files changed
+237 -24

No files matched your search

+19
View File
@@ -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>,
) {
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<Duration> {
self.execution_duration
}
+218 -24
View File
@@ -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<Self>) {
fn execute_cell(&mut self, cell_id: CellId, window: &mut Window, cx: &mut Context<Self>) {
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<Self>) {
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:?}"),
}
});
}
}