Move error checking from render_exercise() to evaluate_exercise()

R/exercise.R

@@ -294,12 +294,6 @@ evaluate_exercise <- function(exercise, envir, evaluate_global_setup = FALSE) {
     eval(parse(text = exercise$global_setup), envir = envir)
   }
 
-
-  # Setup a temporary directory for rendering the exercise
-  exercise_dir <- tempfile(pattern = "lnr-ex")
-  dir.create(exercise_dir)
-  on.exit(unlink(exercise_dir), add = TRUE)
-
   checker_feedback <- NULL
   # Run the checker pre-evaluation _if_ there is code checking to do
   if (length(exercise$code_check)) {
@@ -317,31 +311,34 @@ evaluate_exercise <- function(exercise, envir, evaluate_global_setup = FALSE) {
     }
   }
 
+  # Setup a temporary directory for rendering the exercise
+  exercise_dir <- tempfile(pattern = "lnr-ex")
+  dir.create(exercise_dir)
+  withr::local_dir(exercise_dir)
+  on.exit(unlink(exercise_dir), add = TRUE)
+
   # Resolve knitr options for the exercise and setup chunks
-  rmd_results <- withr::with_dir(
-    exercise_dir,
-    tryCatch(
-      render_exercise(exercise, envir),
-      error = function(e) {
-        if (length(exercise$error_check)) {
-          # Run the condition through an error checker
-          # (the exercise could be to throw an error!)
-          checker_feedback <- try_checker(
-            exercise, "exercise.checker",
-            check_code = exercise$error_check,
-            envir_result = e$envir_result,
-            evaluate_result = e$evaluate_result,
-            envir_prep = e$envir_prep,
-            last_value = e$e,
-            engine = exercise$engine
-          )
-          if (is_exercise_result(checker_feedback)) {
-            return(checker_feedback)
-          }
+  rmd_results <- tryCatch(
+    render_exercise(exercise, envir),
+    error = function(e) {
+      if (length(exercise$error_check)) {
+        # Run the condition through an error checker
+        # (the exercise could be to throw an error!)
+        checker_feedback <- try_checker(
+          exercise, "exercise.checker",
+          check_code = exercise$error_check,
+          envir_result = e$envir_result,
+          evaluate_result = e$evaluate_result,
+          envir_prep = e$envir_prep,
+          last_value = e$e,
+          engine = exercise$engine
+        )
+        if (is_exercise_result(checker_feedback)) {
+          return(checker_feedback)
         }
-        exercise_result_error(e$msg)
       }
-    )
+      exercise_result_error(e$msg)
+    }
   )
 
   if (is_exercise_result(rmd_results)) {
@@ -570,7 +567,7 @@ render_exercise <- function(exercise, envir) {
       return(exercise_result_timeout())
     }
     rlang::abort(
-      class = "learnr_exercise_result",
+      class = "learnr_render_exercise_error",
       envir_result = envir_result,
       evaluate_result = evaluate_result,
       envir_prep = envir_prep,

tests/testthat/test-exercise.R

@@ -141,7 +141,9 @@ test_that("render_exercise() returns identical envir_prep and envir_result if an
   )
 
   exercise_result <- withr::with_tempdir(
-    tryCatch(render_exercise(exercise, new.env()), error = identity)
+    rlang::catch_cnd(
+      render_exercise(exercise, new.env()), "learnr_render_exercise_error"
+    )
   )
 
   expect_s3_class(exercise_result$last_value, "simpleError")
@@ -161,7 +163,9 @@ test_that("render_exercise() returns envir_result up to error", {
   )
 
   exercise_result <- withr::with_tempdir(
-    tryCatch(render_exercise(exercise, new.env()), error = identity)
+    rlang::catch_cnd(
+      render_exercise(exercise, new.env()), "learnr_render_exercise_error"
+    )
   )
 
   expect_s3_class(exercise_result$last_value, "simpleError")
@@ -181,17 +185,21 @@ test_that("render_exercise() with errors and no checker returns exercise result
   )
 
   exercise_result <- withr::with_tempdir(
-    tryCatch(render_exercise(exercise, new.env()), error = identity)
+    rlang::catch_cnd(
+      render_exercise(exercise, new.env()), "learnr_render_exercise_error"
+    )
   )
-  expect_s3_class(exercise_result, "learnr_exercise_result")
+  expect_s3_class(exercise_result, "learnr_render_exercise_error")
   expect_identical(exercise_result$error_message, "setup")
   expect_null(exercise_result$feedback)
 
   exercise <- mock_exercise(user_code = "stop('user')")
   exercise_result <- withr::with_tempdir(
-    tryCatch(render_exercise(exercise, new.env()), error = identity)
+    rlang::catch_cnd(
+      render_exercise(exercise, new.env()), "learnr_render_exercise_error"
+    )
   )
-  expect_s3_class(exercise_result, "learnr_exercise_result")
+  expect_s3_class(exercise_result, "learnr_render_exercise_error")
   expect_identical(exercise_result$error_message, "user")
   expect_null(exercise_result$feedback)
 })
@@ -229,22 +237,14 @@ test_that("render_exercise() cleans up exercise_prep files even when setup fails
   files <- expect_message(
     withr::with_tempdir({
       before <- dir()
-      e <- tryCatch(render_exercise(exercise, new.env()), error = identity)
-      res <- try_checker(
-        exercise, "exercise.checker",
-        check_code = exercise$error_check,
-        envir_result = e$envir_result,
-        evaluate_result = e$evaluate_result,
-        envir_prep = e$envir_prep,
-        last_value = e$e,
-        engine = exercise$engine
+      e      <- rlang::catch_cnd(
+        render_exercise(exercise, new.env()), "learnr_render_exercise_error"
       )
 
       list(
         before = before,
-        before_error = get("dir_setup", res$feedback$checker_args$envir_prep),
-        during = res$feedback$checker_result,
-        after = dir()
+        during = get("dir_setup", e$envir_prep),
+        after  = dir()
       )
     }),
     "exercise_prep.Rmd"
@@ -253,9 +253,7 @@ test_that("render_exercise() cleans up exercise_prep files even when setup fails
   # start with nothing
   expect_identical(files$before, character(0))
   # prep file is present while evaluating prep
-  expect_identical(files$before_error, "exercise_prep.Rmd")
-  # prep files are cleaned up after error
-  expect_identical(files$during, character(0))
+  expect_identical(files$during, "exercise_prep.Rmd")
   # nothing in directory after render_exercise() because user code didn't evaluate
   expect_identical(files$after, character(0))
 })