Correctly show quarto error when it fails (#238)

* correctly add quarto-cli error message when reporting error in quarto_run

* Add tests, and update all snapshot transformation

* Add NEWS and update version

Author: cderv

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: quarto
 Title: R Interface to 'Quarto' Markdown Publishing System
-Version: 1.4.4.9004
+Version: 1.4.4.9005
 Authors@R: c(
     person("JJ", "Allaire", , "[email protected]", role = "aut",
            comment = c(ORCID = "0000-0003-0174-9868")),

NEWS.md

@@ -12,6 +12,8 @@
 
 - `QUARTO_R_QUIET` environment variable can be used to set `quarto.quiet` option, which overrides any `quiet = TRUE` argument passed to `quarto_*` functions. This can be useful to debug Quarto rendering inside other packages, like **pkgdown**. Overrides will also now happens for [GHA debug logging](https://docs.github.com/en/actions/monitoring-and-troubleshooting-workflows/troubleshooting-workflows/enabling-debug-logging).
 
+- Correctly report Quarto CLI error when background process call to `quarto` fails (thanks, @salim-b, [#235](https://github.com/quarto-dev/quarto-r/issues/235))
+
 # quarto 1.4.4
 
 - `quarto_preview()` now looks at `quarto preview` log to browse to the correct url when inside RStudio viewer (thanks, @aronatkins, #167).

R/quarto.R

@@ -3,17 +3,22 @@
 #' Determine the path to the quarto binary. Uses `QUARTO_PATH` environment
 #' variable if defined, otherwise uses `Sys.which()`.
 #'
+#' @param normalize If `TRUE` (default), normalize the path using [base::normalizePath()].
+#'
 #' @return Path to quarto binary (or `NULL` if not found)
 #'
 #' @export
-quarto_path <- function() {
+quarto_path <- function(normalize = TRUE) {
   path_env <- get_quarto_path_env()
   quarto_path <- if (is.na(path_env)) {
     path <- unname(Sys.which("quarto"))
     if (nzchar(path)) path else return(NULL)
   } else {
     path_env
   }
+  if (!normalize) {
+    return(quarto_path)
+  }
   normalizePath(quarto_path, winslash = "/", mustWork = FALSE)
 }
 
@@ -69,11 +74,25 @@ quarto_run <- function(
     },
     error = function(e) {
       msg <- c(x = "Error running quarto cli.")
+      # if there is an error message from quarto CLI, add it to the message
+      if (e$stderr != "") {
+        quarto_error_msg <- xfun::split_lines(e$stderr)
+        names(quarto_error_msg) <- rep(" ", length(quarto_error_msg))
+        msg <- c(
+          msg,
+          " " = paste0(rep("-", nchar(msg)), collapse = ""),
+          quarto_error_msg
+        )
+      }
+
+      # if `--quiet` has been set, quarto CLI won't report any error (e$stderr will be empty)
+      # So remind user to run without `--quiet` to see the full error message
       if (cli_arg_quiet() %in% args)
         msg <- c(
           msg,
           "i" = "Rerun with `quiet = FALSE` to see the full error message."
         )
+
       cli::cli_abort(msg, call = .call, parent = e)
     }
   )

man/quarto_path.Rd

@@ -9,6 +9,31 @@
       Caused by error:
       ! System command 'quarto' failed
 
+# quarto_run report full quarto cli error message
+
+    Code
+      quarto_inspect()
+    Condition
+      Error in `quarto_inspect()`:
+      x Error running quarto cli.
+        -------------------------
+        ERROR: Book chapter 'intro.qmd' not found
+        
+        Stack trace:
+        at throwInputNotFound (<quarto.js full path with location>)
+        at findInputs (<quarto.js full path with location>)
+        at eventLoopTick (ext:core/01_core.js:175:7)
+        at async findChapters (<quarto.js full path with location>)
+        at async bookRenderItems (<quarto.js full path with location>)
+        at async Object.bookProjectConfig [as config] (<quarto.js full path with location>)
+        at async projectContext (<quarto.js full path with location>)
+        at async inspectConfig (<quarto.js full path with location>)
+        at async Command.actionHandler (<quarto.js full path with location>)
+        at async Command.execute (<quarto.js full path with location>)
+        
+      Caused by error:
+      ! System command 'quarto' failed
+
 # is_using_quarto correctly check directory
 
     Code

tests/testthat/_snaps/quarto.md

@@ -63,6 +63,34 @@ local_qmd_file <- function(..., .env = parent.frame()) {
   path
 }
 
+local_quarto_project <- function(
+  name = "test-project",
+  type,
+  ...,
+  .env = parent.frame()
+) {
+  skip_if_no_quarto()
+  path_tmp <- withr::local_tempdir(
+    pattern = "quarto-tests-project-",
+    .local_envir = .env
+  )
+  tryCatch(
+    quarto_create_project(
+      name = name,
+      type = type,
+      dir = path_tmp,
+      no_prompt = TRUE,
+      quiet = TRUE,
+      ...
+    ),
+    error = function(e) {
+      stop("Creating temp project for tests failed", call. = FALSE)
+    }
+  )
+  # return the path to the created project
+  return(file.path(path_tmp, name))
+}
+
 .render <- function(input, output_file = NULL, ..., .env = parent.frame()) {
   skip_if_no_quarto()
   skip_if_not_installed("withr")
@@ -105,28 +133,66 @@ expect_snapshot_qmd_output <- function(name, input, output_file = NULL, ...) {
 
 transform_quarto_cli_in_output <- function(
   full_path = FALSE,
-  normalize_path = FALSE,
-  version = FALSE
+  version = FALSE,
+  dir_only = FALSE
 ) {
+  hide_path <- function(lines, real_path) {
+    gsub(
+      real_path,
+      "<quarto full path>",
+      lines,
+      fixed = TRUE
+    )
+  }
+
   return(
     function(lines) {
       if (full_path) {
         quarto_found <- find_quarto()
-        if (normalize_path) {
-          quarto_found <- normalizePath(quarto_found, mustWork = FALSE)
+        if (dir_only) {
+          quarto_found <- dirname(quarto_found)
         }
-        lines <- gsub(quarto_found, "<quarto full path>", lines, fixed = TRUE)
+        quarto_found_normalized <- normalizePath(quarto_found, mustWork = FALSE)
+        # look for non-normalized path
+        lines <- hide_path(lines, quarto_found)
+        # look for normalized path
+        lines <- hide_path(lines, quarto_found_normalized)
+
+        non_normalized_path <- quarto_path(normalize = FALSE)
+        non_normalized_path_slash <- gsub("\\\\", "/", non_normalized_path)
+        lines <- hide_path(lines, non_normalized_path)
+        lines <- hide_path(lines, non_normalized_path_slash)
+
         # seems like there are quotes around path in CI windows
         lines <- gsub(
-          "\"<quarto full path>\"",
-          "<quarto full path>",
-          lines,
-          fixed = TRUE
+          "\"<quarto full path>([^\"]*)\"",
+          "<quarto full path>\\1",
+          lines
         )
+
+        # Handle quarto.js in stackstrace outputs
+        lines <- gsub(
+          "file:[/]{2,3}<quarto full path>[/\\]quarto.js:\\d+:\\d+",
+          "<quarto.js full path with location>",
+          lines
+        )
+        # fixup binary name difference it exists in the output
+        # windows is quarto.exe while quarto on other OS
+        lines <- gsub("quarto.exe", "quarto", lines, fixed = TRUE)
       } else {
         # it will be quarto.exe only on windows
         lines <- gsub("quarto\\.(exe|cmd)", "quarto", lines)
       }
+
+      # fallback: Above can fail on some windows situation, so try a regex match
+      # it should only match windows path with Drive letters
+      lines <- gsub(
+        "file:[/]{2,3}[A-Za-z]:[\\\\/](?:[^:\\n]+[\\\\/])*bin[\\\\/]quarto\\.js:\\d+:\\d+",
+        "<quarto.js full path with location>",
+        lines,
+        perl = TRUE
+      )
+
       if (version) {
         lines <- gsub(quarto_version(), "<quarto version>", lines, fixed = TRUE)
       }

tests/testthat/helper.R

@@ -0,0 +1 @@
+/.quarto

tests/testthat/project/.gitignore

@@ -14,6 +14,26 @@ test_that("quarto_run gives guidance in error", {
   )
 })
 
+test_that("quarto_run report full quarto cli error message", {
+  skip_if_no_quarto()
+  local_reproducible_output(width = 1000)
+  # Ensure we don't have colors in the output for quarto-cli error
+  withr::local_envvar(list(NO_COLOR = 1L))
+  # https://github.com/quarto-dev/quarto-r/issues/235
+  tmp_proj <- local_quarto_project(type = "book")
+  withr::local_dir(tmp_proj)
+  # simulate an error by renaming the intro.qmd
+  file.rename(from = "intro.qmd", to = "no_intro.qmd")
+  expect_snapshot(
+    error = TRUE,
+    quarto_inspect(),
+    transform = transform_quarto_cli_in_output(
+      full_path = TRUE,
+      dir_only = TRUE
+    )
+  )
+})
+
 test_that("is_using_quarto correctly check directory", {
   qmd <- local_qmd_file(c("content"))
   # Only qmd
@@ -63,7 +83,7 @@ test_that("quarto CLI sitrep", {
           lines,
           fixed = TRUE
         )
-        transform_quarto_cli_in_output(full_path = TRUE, normalize_path = TRUE)(
+        transform_quarto_cli_in_output(full_path = TRUE)(
           lines
         )
       }
@@ -75,8 +95,7 @@ test_that("quarto CLI sitrep", {
     expect_snapshot(
       quarto_binary_sitrep(debug = TRUE),
       transform = transform_quarto_cli_in_output(
-        full_path = TRUE,
-        normalize_path = TRUE
+        full_path = TRUE
       )
     )
   )