Better handle errors in options(error = )

Author: lionel-

crates/ark/src/interface.rs

@@ -1465,14 +1465,14 @@ impl RMain {
     /// https://github.com/rstudio/renv/blob/5d0d52c395e569f7f24df4288d949cef95efca4e/inst/resources/activate.R#L85-L87
     fn handle_invalid_input_request(&self, buf: *mut c_uchar, buflen: c_int) -> ConsoleResult {
         if let Some(input) = Self::renv_autoloader_reply() {
-            log::info!("Detected `readline()` call in renv autoloader. Returning `'{input}'`.");
+            log::warn!("Detected `readline()` call in renv autoloader. Returning `'{input}'`.");
             match Self::on_console_input(buf, buflen, input) {
                 Ok(()) => return ConsoleResult::NewInput,
                 Err(err) => return ConsoleResult::Error(format!("{err}")),
             }
         }
 
-        log::info!("Detected invalid `input_request` outside an `execute_request`. Preparing to throw an R error.");
+        log::warn!("Detected invalid `input_request` outside an `execute_request`. Preparing to throw an R error.");
 
         let message = vec![
             "Can't request input from the user at this time.",

crates/ark/src/modules/positron/errors.R

@@ -201,7 +201,33 @@ invoke_option_error_handler <- function() {
     }
 
     for (hnd in handler) {
-        eval(hnd, globalenv())
+        err <- tryCatch(
+            {
+                eval(hnd, globalenv())
+                NULL
+            },
+            error = identity
+        )
+
+        if (!is.null(err)) {
+            # Disable error handler to avoid cascading errors
+            options(error = NULL)
+
+            # We don't let the error propagate to avoid a confusing sequence of
+            # error messages from R, such as "Error during wrapup"
+            writeLines(
+                c(
+                    "The `getOption(\"error\")` handler failed.",
+                    "This option was unset to avoid cascading errors.",
+                    "Caused by:",
+                    conditionMessage(err)
+                ),
+                con = stderr()
+            )
+
+            # Bail early
+            return()
+        }
     }
 }
 

crates/ark/tests/kernel.rs

@@ -820,6 +820,41 @@ fn test_env_vars() {
     assert_eq!(frontend.recv_shell_execute_reply(), input.execution_count);
 }
 
+#[test]
+fn test_execute_request_error_handler_failure() {
+    let frontend = DummyArkFrontend::lock();
+
+    // Define the functions and set the error handler
+    let code = r#"
+f <- function() g()
+g <- function() h()
+h <- function() stop("foo")
+options(error = function() stop("ouch"))
+"#;
+    frontend.execute_request_invisibly(code);
+
+    frontend.send_execute_request("f()", ExecuteRequestOptions::default());
+    frontend.recv_iopub_busy();
+    let input = frontend.recv_iopub_execute_input();
+    assert_eq!(input.code, "f()");
+
+    frontend.recv_iopub_stream_stderr(
+        r#"The `getOption("error")` handler failed.
+This option was unset to avoid cascading errors.
+Caused by:
+ouch
+"#,
+    );
+
+    assert!(frontend.recv_iopub_execute_error().contains("foo"));
+
+    frontend.recv_iopub_idle();
+    assert_eq!(
+        frontend.recv_shell_execute_reply_exception(),
+        input.execution_count
+    );
+}
+
 /// Install a SIGINT handler for shutdown tests. This overrides the test runner
 /// handler so it doesn't cancel our test.
 fn install_sigint_handler() {