r-lib / pkgdown

Generate static html documentation for an R package
https://pkgdown.r-lib.org/
Other
721 stars 336 forks source link

When building articles with duplicate chunk names, error stack trace does not include the actual Rmarkdown parsing error #2786

Closed fwimp closed 2 months ago

fwimp commented 2 months ago

When trying to build a vignette that contains duplicate chunk labels, build_site() does not give any real indication of where the problem originated.

── Building articles ───────────────────────────────────────────────────────────
Reading vignettes/duplicate-chunk.Rmd
Error: 
! in callr subprocess.
Caused by error in `.f(.x[[i]], ...)`:
! Failed to render vignettes/duplicate-chunk.Rmd.
ℹ See `$stdout` and `$stderr` for standard output and error.
Type .Last.error to see the more details.

The full .Last.error

> .Last.error
<callr_error/rlib_error_3_0/rlib_error/error>
Error: 
! in callr subprocess.
Caused by error in `.f(.x[[i]], ...)`:
! Failed to render vignettes/duplicate-chunk.Rmd.
ℹ See `$stdout` and `$stderr` for standard output and error.
---
Backtrace:
1. pkgdown::build_site(preview = FALSE)
2. pkgdown:::build_site_external(pkg = pkg, examples = examples, run_dont_run = run_dont_run, …
3. callr::r(function(..., cli_colors, hyperlinks, pkgdown_internet) { …
4. callr:::get_result(output = out, options)
5. callr:::throw(callr_remote_error(remerr, output), parent = fix_msg(remerr[[3]]))
---
Subprocess backtrace:
 1. pkgdown::build_site(...)
 2. pkgdown:::build_site_local(pkg = pkg, examples = examples, run_dont_run = run_dont_run, …
 3. pkgdown::build_articles(pkg, lazy = lazy, override = override, preview = FALSE)
 4. pkgdown:::unwrap_purrr_error(purrr::walk(pkg$vignettes$name[pkg$vignettes$type == …
 5. base::withCallingHandlers(code, purrr_error_indexed = function(err) { …
 6. purrr::walk(pkg$vignettes$name[pkg$vignettes$type == "rmd"], …
 7. purrr::map(.x, .f, ..., .progress = .progress)
 8. purrr:::map_("list", .x, .f, ..., .progress = .progress)
 9. purrr:::with_indexed_errors(i = i, names = names, error_call = .purrr_error_call, …
10. base::withCallingHandlers(expr, error = function(cnd) { …
11. purrr:::call_with_cleanup(map_impl, environment(), .type, .progress, …
12. local .f(.x[[i]], ...)
13. pkgdown:::build_rmarkdown_article(pkg = pkg, input_file = input, input_path = input_path, …
14. base::withCallingHandlers(callr::r_safe(rmarkdown_render_with_seed, …
15. callr::r_safe(rmarkdown_render_with_seed, args = args, show = !quiet)
16. callr:::get_result(output = out, options)
17. base::throw(callr_remote_error(remerr, output), parent = fix_msg(remerr[[3]]))
18. | base::signalCondition(cond)
19. (function (cnd) …
20. pkgdown:::wrap_rmarkdown_error(cnd, input_file, call)
21. cli::cli_abort(c(`!` = "Failed to render {.path {input}}.", set_names(lines, …
22. | rlang::abort(message, ..., call = call, use_cli_format = TRUE, …
23. | rlang:::signal_abort(cnd, .file)
24. | base::signalCondition(cnd)
25. (function (cnd) …
26. cli::cli_abort(message, location = i, name = name, parent = cnd, …
27. | rlang::abort(message, ..., call = call, use_cli_format = TRUE, …
28. | rlang:::signal_abort(cnd, .file)
29. | base::signalCondition(cnd)
30. (function (err) …
31. rlang::cnd_signal(err$parent)
32. rlang:::signal_abort(cnd)
33. base::signalCondition(cnd)
34. global (function (e) …

There is also nothing in .Last.error$stdout or .Last.error$stderr to indicate where this might be going wrong.

The data that is required is however present in .Last.error$parent$parent$message

> .Last.error$parent$parent$message
[1] "Duplicate chunk label 'label', which has been used for the chunk:\nNULL"

Whilst not essential, including this data in the error output would make it much easier to track down these sorts of bugs in situations where the vignette is hard to get to knit standalone.


Reprex vignette file

---
title: "Duplicate chunk vignette"
output: rmarkdown::html_vignette
vignette: >
  %\VignetteIndexEntry{Duplicate chunk vignette}
  %\VignetteEngine{knitr::rmarkdown}
  %\VignetteEncoding{UTF-8}
---

Note: ends of chunk fences have been escaped to preserve github's code environment. Remove these before trying to reproduce.

```{r label}
NULL
NULL
jayhesselberth commented 2 months ago

Agree we should surface this error (and wondering what other similar errors we are not surfacing) but I'm not sure how to do that when the call is wrapped in callr::r() so @hadley may need to weigh in.

hadley commented 2 months ago

Hmmmm, when I add that vignette to pkgdown and call build_site(), I (eventually) see:

Error:
! ! in callr subprocess.
Caused by error in `.f(.x[[i]], ...)`:
! Failed to render vignettes/problem.Rmd.
Caused by error:
! Duplicate chunk label 'label', which has been used for the chunk:
NULL
Show Traceback
fwimp commented 2 months ago

Hmm that's strange.

I am on R version 4.3.0 Current pkgdown (2.1.1) dependencies on my end are package pkgdown min installed latest
bslib ≥0.5.1 0.6.0 0.8.0
callr ≥ 3.7.3 3.7.3 3.7.6
cli ≥ 3.6.1 3.6.3 -
desc ≥ 1.4.0 1.4.2 1.4.3
digest - 0.6.33 0.6.37
downlit ≥ 0.4.4 0.4.4 -
fontawesome - 0.5.2 -
fs - 1.6.3 1.6.4
httr2 ≥ 1.0.2 1.0.3 1.0.4
jsonlite - 1.8.8 1.8.9
openssl - 2.2.0 2.2.2
purrr ≥ 1.0.0 1.0.2 -
ragg - 1.2.6 1.3.3
rlang ≥ 1.1.0 1.1.2 1.1.4
rmarkdown ≥ 2.27 2.27 2.28
tibble - 3.2.1 -
whisker - 0.4.1 -
withr ≥ 2.4.3 2.5.2 3.0.1
xml2 ≥ 1.3.1 1.3.5 1.3.6
yaml - 2.3.8 2.3.10

If the issue is rooted in an "outdated" version of one of those, that implies that potentially a bump in the required version of the package at fault would fix it.

Just looking at the options alongside the stack trace and source code for wrap_rmarkdown_error() in build-article.R, I'm suspicious that if a package version is to blame it's either callr or rlang (as the other outdated packages don't seem to be called when handling this error).

I can push my latest package version to its repo (https://github.com/fwimp/ohvbd/) and provide instructions here on how to reproduce the exact issue as I originally discovered it (by renaming a chunk in a vignette), just in case the issue stems somehow from my particular package (though I'm not doing anything particularly obscure with my vignettes).

Would that be helpful?

fwimp commented 2 months ago

Right, I've confirmed that updating rlang to v1.1.4 causes the error to appropriately be propagated up to the user:

Error: 
! in callr subprocess.
Caused by error in `.f(.x[[i]], ...)`:
! Failed to render vignettes/duplicate-chunk.Rmd.
Caused by error:
! Duplicate chunk label 'label', which has been used for the chunk:
NULL
ℹ See `$stdout` and `$stderr` for standard output and error.
Type .Last.error to see the more details.

I expect this was fixed in this commit https://github.com/r-lib/rlang/commit/84deb877d1ce4ba50fbaafea246c1eb918805631, as I can't see anything else in the commits making up 1.1.3 and 1.1.4 for rlang that would potentially intersect with this issue. Doesn't really matter in any case as it seems like updating rlang solves the problem.

I guess if this is something that shouldn't happen for users, then bumping the version requirement for pkgdown to rlang ≥1.1.4 should mean that others don't hit this particular frustration. I'm afraid I didn't test through 1.1.3 so it could be that it gets fixed there, in which case the dependency could be relaxed to be rlang ≥1.1.3.

hadley commented 2 months ago

Yeah, we should just bump the required rlang version just to make sure that no one else experiences this. Thanks for the investigation!