Clarify the main profvis loop (#169)

* Ensure that we always terminate `RProf()` even if the expr errors
* Make it a bit more clear how expr works (we regenerate the lazy binding on every iteration)
* Ensure we always set lines, even if the expr errors. (This is what lead to the mystifying lines is not a character vector error)

R/profvis.R

@@ -168,24 +168,24 @@ profvis <- function(expr = NULL,
       filter.callframes = simplify
     ))
 
-    on.exit(Rprof(NULL), add = TRUE)
     if (remove_on_exit) {
       on.exit(unlink(prof_output), add = TRUE)
     }
     repeat {
+      # Work around https://github.com/r-lib/rlang/issues/1749
+      eval(substitute(delayedAssign("expr", expr_q, eval.env = env)))
+
       inject(Rprof(prof_output, !!!rprof_args))
       cnd <- with_profvis_handlers(expr)
-      if (!is.null(cnd)) {
-        break
-      }
       Rprof(NULL)
 
       lines <- readLines(prof_output)
+      if (!is.null(cnd)) {
+        break
+      }
       if (prof_matches(zap_header(lines), rerun)) {
         break
       }
-
-      env_bind_lazy(current_env(), expr = !!expr_q, .eval_env = env)
     }
 
     # Must be in the same handler context as `expr` above to get the

tests/testthat/_snaps/profvis.md

@@ -6,3 +6,12 @@
       Error in `profvis()`:
       ! Exactly one of `expr` or `prof_input` must be supplied.
 
+# can capture profile of code with error
+
+    Code
+      out <- profvis(f(), rerun = "pause")
+    Message
+      profvis: code exited with error:
+      error
+      
+

tests/testthat/test-profvis.R

@@ -28,3 +28,12 @@ test_that("defaults to elapsed timing", {
 test_that("expr and prof_input are mutually exclusive", {
   expect_snapshot(profvis(expr = f(), prof_input = "foo.R"), error = TRUE)
 })
+
+test_that("can capture profile of code with error", {
+  f <- function() {
+    pause(TEST_PAUSE_TIME)
+    stop("error")
+  }
+  expect_snapshot(out <- profvis(f(), rerun = "pause"))
+  expect_equal(profile_mode(out), "pause f")
+})