r-lib / processx

Execute and Control Subprocesses from R
https://processx.r-lib.org/
Other
234 stars 43 forks source link

Error printout has too much duplication #307

Closed gaborcsardi closed 2 years ago

gaborcsardi commented 3 years ago

In particular, if the stack is printed, then we don't need to print the errors up front:

❯ R -q -e 'callr::r(function() library(sdfsdfsd))'
> callr::r(function() library(sdfsdfsd))

Error: <callr_status_error: callr subprocess failed: there is no package called ‘sdfsdfsd’>
-->
<callr_remote_error in library(sdfsdfsd):
 there is no package called ‘sdfsdfsd’>
 in process 17258

 Stack trace:

 Process 17246:
 1. callr::r(function() library(sdfsdfsd))
 2. callr:::get_result(output = out, options)
 3. throw(newerr, parent = remerr[[2]])

 x callr subprocess failed: there is no package called ‘sdfsdfsd’

 Process 17258:
 15. (function ()  ...
 16. base:::library(sdfsdfsd)
 17. base:::stop(packageNotFoundError(package, lib.loc, sys.call()))
 18. (function (e)  ...

 x there is no package called ‘sdfsdfsd’

Execution halted

It gets quite annoying when stdout/stderr is embedded into the error message.

gaborcsardi commented 2 years ago

With the current feature/errors-v3 branch this is:

❯ R -q -e 'callr::r(function() library(sdfsdfsd))'
> callr::r(function() library(sdfsdfsd))
Error: :
! error in callr subprocess
Caused by error in `library(sdfsdfsd)`:
! there is no package called ‘sdfsdfsd’
---
Backtrace:
1. callr::r(function() library(sdfsdfsd))
2. callr:::get_result(output = out, options) at eval.R:189:3
3. callr:::throw(callr_remote_error(remerr)) at result.R:67:5
4. callr:::callr_remote_error(remerr) at result.R:67:5
5. callr:::throw(err, parent = remerr[[3]]) at error.R:44:3
---
Subprocess trace:
1. base::library(sdfsdfsd)
2. base::stop(packageNotFoundError(package, lib.loc, sys.call()))
3. global (function (e)
Execution halted

and with cli it is even nicer:

❯ R -q -e 'library(cli); callr::r(function() library(sdfsdfsd))'
Screen Shot 2022-06-08 at 4 07 48 PM
gaborcsardi commented 2 years ago

This has been fixed by #335.