From 23bfc6892d11217f088e3aa51b711e4ab4a166a6 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 28 Oct 2025 17:24:10 +0100 Subject: [PATCH 1/3] Remove error peeking in top level exec results --- crates/harp/src/exec.rs | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/crates/harp/src/exec.rs b/crates/harp/src/exec.rs index 0bc0e0b86..042a4f80d 100644 --- a/crates/harp/src/exec.rs +++ b/crates/harp/src/exec.rs @@ -348,19 +348,11 @@ where match res { Some(res) => res, - None => { - let mut err_buf = r_peek_error_buffer(); - - if err_buf.len() > 0 { - err_buf = format!("\nLikely caused by:\n{err_buf}"); - } - - Err(Error::TopLevelExecError { - message: format!("Unexpected longjump{err_buf}"), - backtrace: std::backtrace::Backtrace::force_capture(), - span_trace: tracing_error::SpanTrace::capture(), - }) - }, + None => Err(Error::TopLevelExecError { + message: String::from("Unexpected longjump"), + backtrace: std::backtrace::Backtrace::force_capture(), + span_trace: tracing_error::SpanTrace::capture(), + }), } } @@ -606,7 +598,6 @@ mod tests { assert_match!(out, Err(Error::TopLevelExecError { message, backtrace: _ , span_trace: _}) => { assert!(message.contains("Unexpected longjump")); - assert!(message.contains("my message")); }); }) } From 3bbe90e3a2b5dace605714b5131283f4ad2e3f42 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Wed, 29 Oct 2025 09:01:47 +0100 Subject: [PATCH 2/3] Revert "Remove error peeking in top level exec results" This reverts commit 23bfc6892d11217f088e3aa51b711e4ab4a166a6. --- crates/harp/src/exec.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/crates/harp/src/exec.rs b/crates/harp/src/exec.rs index 042a4f80d..0bc0e0b86 100644 --- a/crates/harp/src/exec.rs +++ b/crates/harp/src/exec.rs @@ -348,11 +348,19 @@ where match res { Some(res) => res, - None => Err(Error::TopLevelExecError { - message: String::from("Unexpected longjump"), - backtrace: std::backtrace::Backtrace::force_capture(), - span_trace: tracing_error::SpanTrace::capture(), - }), + None => { + let mut err_buf = r_peek_error_buffer(); + + if err_buf.len() > 0 { + err_buf = format!("\nLikely caused by:\n{err_buf}"); + } + + Err(Error::TopLevelExecError { + message: format!("Unexpected longjump{err_buf}"), + backtrace: std::backtrace::Backtrace::force_capture(), + span_trace: tracing_error::SpanTrace::capture(), + }) + }, } } @@ -598,6 +606,7 @@ mod tests { assert_match!(out, Err(Error::TopLevelExecError { message, backtrace: _ , span_trace: _}) => { assert!(message.contains("Unexpected longjump")); + assert!(message.contains("my message")); }); }) } From f4d3e3f03816c62277a7091b4284654170a72fa6 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Wed, 29 Oct 2025 09:31:50 +0100 Subject: [PATCH 3/3] Still include error message if stack overflow is detected --- crates/ark/src/interface.rs | 4 +--- crates/harp/src/exec.rs | 18 +++++++++++++----- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/crates/ark/src/interface.rs b/crates/ark/src/interface.rs index 8eed05f21..ad70acd04 100644 --- a/crates/ark/src/interface.rs +++ b/crates/ark/src/interface.rs @@ -64,6 +64,7 @@ use harp::exec::r_peek_error_buffer; use harp::exec::r_sandbox; use harp::exec::RFunction; use harp::exec::RFunctionExt; +use harp::exec::RE_STACK_OVERFLOW; use harp::library::RLibraries; use harp::line_ending::convert_line_endings; use harp::line_ending::LineEnding; @@ -2225,9 +2226,6 @@ fn new_incomplete_reply(req: &ExecuteRequest, exec_count: u32) -> amalthea::Resu Err(amalthea::Error::ShellErrorExecuteReply(error, exec_count)) } -static RE_STACK_OVERFLOW: Lazy = - Lazy::new(|| Regex::new(r"C stack usage [ 0-9]+ is too close to the limit\n").unwrap()); - fn new_execute_reply(exec_count: u32) -> amalthea::Result { Ok(ExecuteReply { status: Status::Ok, diff --git a/crates/harp/src/exec.rs b/crates/harp/src/exec.rs index 0bc0e0b86..70ac17195 100644 --- a/crates/harp/src/exec.rs +++ b/crates/harp/src/exec.rs @@ -11,6 +11,8 @@ use std::os::raw::c_void; use anyhow::anyhow; use libr::*; +use once_cell::sync::Lazy; +use regex::Regex; use crate::call::RCall; use crate::environment::R_ENVS; @@ -22,6 +24,9 @@ use crate::object::RObject; use crate::r_symbol; use crate::utils::r_stringify; +pub static RE_STACK_OVERFLOW: Lazy = + Lazy::new(|| Regex::new(r"C stack usage [ 0-9]+ is too close to the limit\n").unwrap()); + pub struct RFunction { pub call: RCall, is_namespaced: bool, @@ -296,10 +301,13 @@ where /// and long jumps. /// /// If a longjump does occur for any reason (including but not limited to R -/// errors), the caller is notified, in this case by an `Err` return value -/// of kind `TopLevelExecError`. The error message contains the contents of -/// the C-level error buffer. It might or might not be related to the cause -/// of the longjump. The error also carries a Rust backtrace. +/// errors), the caller is notified, in this case by an `Err` return value of +/// kind `TopLevelExecError`. +/// +/// If the longjump occurs after a stack overflow error, this is indicated in +/// the error message. Note that it's possible the stackoverflow did not +/// actually cause that particular longjump, it's only indicative that it did +/// happen prior to the longjump. The error also carries a Rust backtrace. /// /// `top_level_exec()` is a low-level operator. Prefer using `try_catch()` /// if possible: @@ -351,7 +359,7 @@ where None => { let mut err_buf = r_peek_error_buffer(); - if err_buf.len() > 0 { + if RE_STACK_OVERFLOW.is_match(&err_buf) { err_buf = format!("\nLikely caused by:\n{err_buf}"); }