Skip to content

Commit 463a788

Browse files
committed
Prevent calling readline() or menu() from error handler
From R's perspective, the handler runs _after_ the error was emitted. That's why the user is able to see the error message before the recover prompt. From our perspective though, we have to run the handler _before_ emitting the error, because all executed code needs to nested in an execution request so that we can properly match output to a prompt on the frontend side. The Jupyter protocol does not really support orphan side effects (streams, input requests).
1 parent 2be99fa commit 463a788

File tree

2 files changed

+64
-1
lines changed

2 files changed

+64
-1
lines changed

crates/ark/src/interface.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -906,6 +906,20 @@ impl RMain {
906906
}
907907

908908
if let Some(exception) = self.take_exception() {
909+
// We might get an input request if `readline()` or `menu()` is
910+
// called in `options(error = )`. We respond to this with an error
911+
// as this is not supported by Ark.
912+
if matches!(info.kind, PromptKind::InputRequest) {
913+
// Reset error so we can handle it when we recurse here after
914+
// the error aborts the readline. Note it's better to first emit
915+
// the R invalid input request error, and then handle
916+
// `exception` within the context of a new `ReadConsole`
917+
// instance, so that we emit the proper execution prompts as
918+
// part of the response, and not the readline prompt.
919+
self.last_error = Some(exception);
920+
return self.handle_invalid_input_request_after_error();
921+
}
922+
909923
// Clear any pending inputs, if any
910924
self.pending_inputs = None;
911925

@@ -1482,6 +1496,18 @@ impl RMain {
14821496
return ConsoleResult::Error(message);
14831497
}
14841498

1499+
fn handle_invalid_input_request_after_error(&self) -> ConsoleResult {
1500+
log::warn!("Detected invalid `input_request` after error (probably from `getOption('error')`). Preparing to throw an R error.");
1501+
1502+
let message = vec![
1503+
"Can't request input from the user at this time.",
1504+
"Are you calling `readline()` or `menu()` from `options(error = )`?",
1505+
]
1506+
.join("\n");
1507+
1508+
return ConsoleResult::Error(message);
1509+
}
1510+
14851511
fn start_debug(&mut self, debug_preserve_focus: bool) {
14861512
match self.dap.stack_info() {
14871513
Ok(stack) => {

crates/ark/tests/kernel.rs

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -824,7 +824,6 @@ fn test_env_vars() {
824824
fn test_execute_request_error_handler_failure() {
825825
let frontend = DummyArkFrontend::lock();
826826

827-
// Define the functions and set the error handler
828827
let code = r#"
829828
f <- function() g()
830829
g <- function() h()
@@ -855,6 +854,44 @@ ouch
855854
);
856855
}
857856

857+
#[test]
858+
fn test_execute_request_error_handler_readline() {
859+
let frontend = DummyArkFrontend::lock();
860+
861+
let code = r#"
862+
f <- function() g()
863+
g <- function() h()
864+
h <- function() stop("foo")
865+
options(error = function() menu("ouch"))
866+
"#;
867+
frontend.execute_request_invisibly(code);
868+
869+
frontend.send_execute_request("f()", ExecuteRequestOptions::default());
870+
frontend.recv_iopub_busy();
871+
let input = frontend.recv_iopub_execute_input();
872+
assert_eq!(input.code, "f()");
873+
874+
frontend.recv_iopub_stream_stdout("Enter an item from the menu, or 0 to exit\n");
875+
876+
frontend.recv_iopub_stream_stderr(
877+
r#"The `getOption("error")` handler failed.
878+
This option was unset to avoid cascading errors.
879+
Caused by:
880+
Can't request input from the user at this time.
881+
Are you calling `readline()` or `menu()` from `options(error = )`?
882+
"#,
883+
);
884+
885+
assert!(frontend.recv_iopub_execute_error().contains("foo"));
886+
frontend.recv_iopub_idle();
887+
888+
assert_eq!(
889+
frontend.recv_shell_execute_reply_exception(),
890+
input.execution_count
891+
);
892+
}
893+
894+
#[test]
858895
/// Install a SIGINT handler for shutdown tests. This overrides the test runner
859896
/// handler so it doesn't cancel our test.
860897
fn install_sigint_handler() {

0 commit comments

Comments
 (0)