yihui / knitr

A general-purpose tool for dynamic report generation in R
https://yihui.org/knitr/
2.37k stars 874 forks source link

Improve error report when chunk fails #2218

Closed cderv closed 1 year ago

cderv commented 1 year ago

This closes #2215

Created as a first draft for discussion, and to improve dealing with possible edge cases when we agree on #2215

cc @yihui @hadley

cderv commented 1 year ago

@lionel- I made the change to use withCallingHandlers(error = function(cnd) rlang::entrace(cnd)). Is doing that within the existing withCallingHandlers ok ? Is this done conform as you suggested ?

@hadley asking for another review as you may want to check this on your end

(Oh I can do only one reviewer at a time... sorry for the notification noise)

@yihui I believe this ok for you.

thanks

cderv commented 1 year ago

Thanks !

(In a future PR it'd be cool to use cli.r-lib.org/reference/links.html#files, so that the error message was clickable)

Yes this is tracked in #2153 - I added the doc there so that I can follow the recommandations

lionel- commented 1 year ago

@cderv

I made the change to use withCallingHandlers(error = function(cnd) rlang::entrace(cnd)). Is doing that within the existing withCallingHandlers ok ?

yup perfect!

yihui commented 1 year ago

I think this PR broke some learnr tests such as https://github.com/rstudio/learnr/blob/main/tests/testthat/test-exercise.R#L128

── Failure ('test-exercise.R:128:3'): render_exercise() returns envir_result up to error ──
exercise_result$parent inherits from 'rlang_error'/'error'/'condition' not 'simpleError'.
── Failure ('test-exercise.R:184:3'): evaluate_exercise() errors from setup chunks aren't checked by error checker ──
exercise_result$feedback$error inherits from 'rlang_error'/'error'/'condition' not 'simpleError'.
── Failure ('test-exercise.R:201:3'): evaluate_exercise() errors from user code are checked by error_checker ──
exercise_result$feedback$checker_args$last_value inherits from 'rlang_error'/'error'/'condition' not 'simpleError'.
── Failure ('test-exercise.R:222:3'): evaluate_exercise() errors from user code are checked by default error checker as a fallback ──
exercise_result$feedback$checker_args$last_value inherits from 'rlang_error'/'error'/'condition' not 'simpleError'.
── Failure ('test-exercise.R:461:3'): evaluate_exercise() handles default vs. explicit error check code ──
res$feedback$checker_args$last_value inherits from 'rlang_error'/'error'/'condition' not 'simpleError'.

I'm not sure what we should do about it (cc @gadenbuie).