extendr / rextendr

An R package that helps scaffolding extendr-enabled packages or compiling Rust code dynamically
https://extendr.github.io/rextendr/
Other
196 stars 28 forks source link

[Feature Proposal]: Produce better errors when `cargo` fails at runtime #115

Closed Ilia-Kosenkov closed 2 years ago

Ilia-Kosenkov commented 3 years ago

Before you read

This is an invitation to discussion. There is no need to implement this feature, but I do believe the end users of {extendr} could benefit from something like this.

Problem outline

We run cargo in two scenarios: when a user uses public API like rust_function and when {knitr} is processing markdown. https://github.com/extendr/rextendr/blob/00ce4650ee044e4b7c66ebeaecaa65c6d5ec7024/R/source.R#L281-L297 As you can see, right now we simply use system2 with some stderr/stdout redirection and check its exit status. If not zero, we throw a super-useful error with absolutely no details.

In a user session, when R has access to stdout and stderr, cargo displays its output in the console (unless quiet = TRUE). This helps to resolve any compilation errors. What happens if no stdout/stderr is available or if quiet = TRUE? Well, nothing is printed and the only useful information is the error message saying "Compilation failed. Aborting". This is especially painful when running R CMD check or rcmdcheck::rcmdcheck, as it shows you errors but not stdout/stderr.

Proposed solution

As suggested by dfalbel in Discord discussion, we use rlang::abort() to produce an rlang::rlang_error when calling rextendr::ui_throw(). rlang::abort() allows attaching additional named data to the thrown error using its ... argument. So, we could augment the invocation of cargo, capture its errors and attach them to our rextendr::rextendr_error. This way, even if we disable verbose output for cargo, we will be able to provide an explanation to the user of what went wrong. Sounds simple? Well, not really, here are the details.

Implementation details

Capturing cargo output

The desired solution is to be able to both display and capture output of cargo, preserving its color scheme and formatting. We would also want to be able to 'switch off' verbose output yet still capture it for the purpose of error formatting, satisfying quiet = TRUE.

cargo is a little bit tricky. It prints to stdout information like passed/failed tests, but sends all compilation info, warnings and errors to stderr (this includes all these fancy 'Updating crates.io', 'Compiling x', 'Finished', etc messages). In our scenario, we run cargo build --lib, so there should be no stdout at all, but for now let us assume there may be stdout during compilation. Just a reminder that we are discussing runtime compilation, where we create Rust crate ourselves, so I expect no build.rs or any other build tricks, just plain compilation of (likely one) Rust file(s).

Printing rextendr_error

In general, {rlang} does not display additional fields and metadata when printed rlang::rlang_error-derived errors. What we would like is to have two display modes: a shorter form which prints only part of errors/warnings, which is used everywhere (especially when the error is uncaught), and a longer form which prints all of the cargo errors/warnings, which should be accessed using summary S3 method.

The best part: reproducible example:

https://github.com/Ilia-Kosenkov/rextendr/tree/callr-cargo Successful CI checks of this branch https://github.com/Ilia-Kosenkov/rextendr/actions/runs/838499744

Before ``` r # Catching error err <- tryCatch(rextendr::rust_function("fn invalid syntax(){}", quiet = TRUE), error = identity) # Short form print(err) #> #> Rust code could not be compiled successfully. Aborting. #> Backtrace: #> 1. base::tryCatch(...) #> 5. rextendr::rust_function("fn invalid syntax(){}", quiet = TRUE) #> 6. rextendr::rust_source(code = code, env = env, ...) #> 7. rextendr:::invoke_cargo(...) #> 8. rextendr:::ui_throw("Rust code could not be compiled successfully. Aborting.") # Long form summary(err) #> #> Rust code could not be compiled successfully. Aborting. #> Backtrace: #> x #> 1. +-base::tryCatch(...) #> 2. | \-base:::tryCatchList(expr, classes, parentenv, handlers) #> 3. | \-base:::tryCatchOne(expr, names, parentenv, handlers[[1L]]) #> 4. | \-base:::doTryCatch(return(expr), name, parentenv, handler) #> 5. \-rextendr::rust_function("fn invalid syntax(){}", quiet = TRUE) #> 6. \-rextendr::rust_source(code = code, env = env, ...) #> 7. \-rextendr:::invoke_cargo(...) #> 8. \-rextendr:::ui_throw("Rust code could not be compiled successfully. Aborting.") ``` Created on 2021-05-13 by the [reprex package](https://reprex.tidyverse.org) (v2.0.0) ![image](https://user-images.githubusercontent.com/8782986/118119261-c39b6580-b3f6-11eb-8039-424e8901e7c3.png)
After ``` r # Catching error err <- tryCatch(rextendr::rust_function("fn invalid syntax(){}", quiet = TRUE), error = identity) # Short form print(err) #> #> #> x `cargo` emitted 3 errors: #> 1. error: expected one of `(` or `<`, found `syntax` #> --> src\lib.rs:3:12 #> |... #> 2. error: aborting due to previous error #> 3. error: could not compile `rextendr1` #> Backtrace: #> 1. base::tryCatch(...) #> 5. rextendr::rust_function("fn invalid syntax(){}", quiet = TRUE) #> 6. rextendr::rust_source(code = code, env = env, ...) #> 7. rextendr:::invoke_cargo(...) #> 8. rextendr:::ui_throw(...) ``` Created on 2021-05-13 by the [reprex package](https://reprex.tidyverse.org) (v2.0.0)