poissonconsulting / chk

An R package for developers to check user-supplied function arguments
https://poissonconsulting.github.io/chk/
Other
48 stars 4 forks source link

Prevent call from appearing in error message #112

Closed ngreifer closed 2 years ago

ngreifer commented 2 years ago

When an error is thrown (i.e., as the result of err() or chk_ function), the error thrown always says

Error in `err()`:

The user should not see err() and likely doesn't know what it is, so that part of the error message is confusing and unnecessary. It doesn't actually tell the user where the error is; it appears to be in a function err() that they did not call. It would be great if err() would not display the function call unless requested, similar to rlang::abort() doesn't or stop(., call. = FALSE). This is due to using rlang::exec() in the err() call rather than calling rlang::abort() directly; for some reason using exec() triggers call to be found and supplied. Thanks!

joethorley commented 2 years ago

Thanks @ngreifer - we'll look into

joethorley commented 2 years ago

This was a good idea - I've switched to rlang::abort() and set the call value so that now the code tells the user where the error is conditional on it being informative

library(chk)
chk_string(1)
#> Error:
#> ! `1` must be a string (non-missing character scalar).

fun <- function(x) chk_string(x)
fun(1)
#> Error in `fun()`:
#> ! `x` must be a string (non-missing character scalar).

Created on 2022-08-25 by the reprex package (v2.0.1)

ngreifer commented 2 years ago

This is still a bit of a problem with all the chk_ functions, which still just display the location of the error, even if that is in a non-user-visible function. Also, in err(), the default to call is caller_call(3) which seems arbitrary (i.e., is err() supposed to be used 3 function calls deep?). Not sure what a sensible default is, but I don't know if 3 is it.

github-actions[bot] commented 4 months ago

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue and link to this old issue if necessary.