Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Provide more informative/helpful condition messages in Abort, Warn(, Inform?) #78

Open
7 tasks done
brookslogan opened this issue May 16, 2022 · 4 comments · May be fixed by #102
Open
7 tasks done

Provide more informative/helpful condition messages in Abort, Warn(, Inform?) #78

brookslogan opened this issue May 16, 2022 · 4 comments · May be fixed by #102
Assignees
@brookslogan
Labels
P2 low priority REPL Improved print, errors, etc.

Comments

@brookslogan
Copy link
Contributor

brookslogan commented May 16, 2022 •
edited
Loading

rlang conditions allow us to assign custom classes (allowing selective filtering), pack in relevant information within the condition object, and provide more informative traces than traceback. However, some of this functionality is not at all obvious for abort, and most/all of it is not obvious or activated for warn and inform. Plus, packing information inside the object rather than the message may actually obscure warn and inform and require an extra step for abort without some duplication inside the main message.

Help users unfamiliar with rlang conditions by:

  • Providing a short tutorial periodically when we use rlang conditions, overviewing how to view condition fields, how to get an improved trace, and how to enable these things for warn and inform when they aren't available by default (on some/all R distributions) (requiring global_entrace() first instead). Try this using inform with a .frequency.
  • Making it easy in Abort, Warn, to package in some informative objects that should be printed, and others whose names should be listed, placing them either with appropriate package-name prefixes as condition fields, or as a list condition field.
  • Consider an analogous Inform function.
  • In Abort, provide the caller's function name rather than "Error in Abort", using call parameter.
  • Tweak init to adjust the wrapping based on the actual prefixes used by rlang, e.g., "! " for abort. (Might need to know if global_entrace is active)
  • Make sure there's a way to use recover; look into what happened with this Issue.
  • Make condition class names more readily available for selective recovery? Also, maybe have length 2 or 3 subclasses that will allow to catch all errors or all conditions from this package?
@brookslogan brookslogan self-assigned this May 16, 2022
@brookslogan
Copy link
Contributor Author

It looks like using recover is not currently possible via options(error=recover), but is possible via withCallingHandlers. The interactive session below gives an example.

R version 4.0.5 (2021-03-31) -- "Shake and Throw"
Copyright (C) 2021 The R Foundation for Statistical Computing
Platform: x86_64-redhat-linux-gnu (64-bit)

R is free software and comes with ABSOLUTELY NO WARRANTY.
You are welcome to redistribute it under certain conditions.
Type 'license()' or 'licence()' for distribution details.

  Natural language support but running in an English locale

R is a collaborative project with many contributors.
Type 'contributors()' for more information and
'citation()' on how to cite R or R packages in publications.

Type 'demo()' for some demos, 'help()' for on-line help, or
'help.start()' for an HTML browser interface to help.
Type 'q()' to quit R.

> library(rlang)
> 
> 
> 
> inner_fn_using_stop = function() { result = 42; stop("stop message") }
> outer_fn_using_stop = function() { inner_fn_using_stop() }
> options(error=recover)
> outer_fn_using_stop()
Error in inner_fn_using_stop() : stop message

Enter a frame number, or 0 to exit   

1: outer_fn_using_stop()
2: #1: inner_fn_using_stop()

Selection: 2
Called from: outer_fn_using_stop()
Browse[1]> ls(all.names=TRUE)
[1] "result"
Browse[1]> result
[1] 42
Browse[1]> 

Enter a frame number, or 0 to exit   

1: outer_fn_using_stop()
2: #1: inner_fn_using_stop()

Selection: 0
> 
> 
> 
> options(error=NULL) # (recover wouldn't work automatically below if trying the same thing, at least in current development environment; just deactivate it to avoid confusion)
> 
> inner_fn_using_abort = function() { result = 42; abort("abort message") }
> outer_fn_using_abort = function() { inner_fn_using_abort() }
> withCallingHandlers(outer_fn_using_abort(), error = function(e) { recover() })

Enter a frame number, or 0 to exit   

1: withCallingHandlers(outer_fn_using_abort(), error = function(e) {
    recov
2: outer_fn_using_abort()
3: #1: inner_fn_using_abort()
4: #1: abort("abort message")
5: signal_abort(cnd, .file)
6: signalCondition(cnd)
7: (function (e) 
{
    recover()
})(list(message = "abort message", trace = l

Selection: 3
Called from: signalCondition(cnd)
Browse[1]> ls(all.names=TRUE)
[1] "result"
Browse[1]> result
[1] 42
Browse[1]> 

Enter a frame number, or 0 to exit   

1: withCallingHandlers(outer_fn_using_abort(), error = function(e) {
    recov
2: outer_fn_using_abort()
3: #1: inner_fn_using_abort()
4: #1: abort("abort message")
5: signal_abort(cnd, .file)
6: signalCondition(cnd)
7: (function (e) 
{
    recover()
})(list(message = "abort message", trace = l

Selection: 0
Error in `inner_fn_using_abort()`:
! abort message
Run `rlang::last_error()` to see where the error occurred.
> 
> 
> 
> 
> sessionInfo()
R version 4.0.5 (2021-03-31)
Platform: x86_64-redhat-linux-gnu (64-bit)
Running under: Fedora 34 (Workstation Edition)

Matrix products: default
BLAS/LAPACK: /usr/lib64/libflexiblas.so.3.1

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C              
 [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8   
 [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                 
 [9] LC_ADDRESS=C               LC_TELEPHONE=C            
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] rlang_1.0.2

loaded via a namespace (and not attached):
[1] compiler_4.0.5 cli_3.3.0      glue_1.6.2     crayon_1.5.1

Some more investigation is likely required when dealing with

  • rlang condition chains (using parent args)
  • noninteractive sessions where we may want to dump and use debugger afterward
  • interaction with parallel::mclapply, bettermc::mclapply, furrr::future_*map*

@brookslogan brookslogan added P1 medium priority P0 high priority REPL Improved print, errors, etc. and removed P1 medium priority labels May 24, 2022
@brookslogan
Copy link
Contributor Author

Current work left:

  • Check exactly how we want text wrapping to work, make sure cli interacts reasonably if user has it, make sure tutorial and errors are readable under these settings.
  • Check debug frames functionality
  • Check interaction with parallelism libraries
  • Add tests

@brookslogan brookslogan linked a pull request Jun 16, 2022 that will close this issue
Augment Abort & co.; add rlang condition tutorial, helpers #102
Draft
@brookslogan brookslogan added P2 low priority and removed P0 high priority labels Oct 24, 2023
@brookslogan
Copy link
Contributor Author

(@dsweber2 the call = change might be nested in this ancient PR WIP though it's simple to reimplement... Anything else from here you have been wishing for?)

@dsweber2
Copy link
Contributor

hard to answer, there's a lot in here. I guess we just hadn't gotten to this in the triage? Is the list of todos in the actual PR the only thing that's preventing this from getting merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@brookslogan brookslogan

Labels
P2 low priority REPL Improved print, errors, etc.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Augment Abort & co.; add rlang condition tutorial, helpers cmu-delphi/epiprocess
2 participants
@dsweber2 @brookslogan