HenrikBengtsson / Wishlist-for-R

Features and tweaks to R that I and others would love to see - feel free to add yours!
https://github.com/HenrikBengtsson/Wishlist-for-R/issues
GNU Lesser General Public License v3.0
134 stars 4 forks source link

WISH: Support for modifying base::.Traceback #86

Open HenrikBengtsson opened 5 years ago

HenrikBengtsson commented 5 years ago

Background

It's currently possible to edit .Traceback of the base package, e.g.

> foo <- function() { 
  on.exit(.BaseNamespaceEnv$.Traceback <- c("some", "other", "call", "stack"))
  stop("whoops")
}

> foo()
Error in foo() : whoops

> traceback()
4: some
3: other
2: call
1: stack

However, there is a risk that this is a "hack" and not officially supported by R, e.g.

  1. ?traceback says: "Warning: It is undocumented where .Traceback is stored nor that it is visible, and this is subject to change."

  2. ?bindenv says: "… lockEnvironment locks … a normal environment (not base). (Locking the base environment and namespace may be supported later.)"

Wish

Officially support editing of .Traceback, for instance via a new base::setTraceback() function.

References

brodieG commented 5 years ago

The particular use case that I have in mind is a REPL that runs in an interactive R session, as browser() does. The REPL simulates the standard R REPL, but with additional features. However, unlike browser, in order to support evaluation of expressions that cause errors it must resort to tryCatch/withCallingHandlers, which means the traceback is not set.

More generally, there will be many cases where R software that evaluates R expressions and handles errors wishes to report back the traceback of the handled errors for users to access.

While these may seem like edge cases, the cost of providing this feature is low since the stored value .Traceback is inherently volatile, like .Last.value. No one should expect that those values will survive any subsequent evaluation so allowing write access to .Traceback seems harmless.

gaborcsardi commented 5 years ago

You don't have to use .Traceback and traceback(), just write your own code to trap errors and copy the traceback from the worker node.

brodieG commented 5 years ago

I guess in my case the user sees an error and types traceback() in the mock REPL expecting to see the traceback. I could also intercept the traceback() call in the mock REPL, but that doesn't feel very satisfying.

And to be clear, I want the user of my code to be able to type traceback() and get the traceback. Obviously my code can get it. It just needs to have an approved way of writing so that the user can get it with traceback().

HenrikBengtsson commented 5 years ago

... just write your own code to trap errors and copy the traceback from the worker node.

This is my current approach that take with future::backtrace(). What I and @brodieG (and probably several others) wish is to avoid introducing yet-another-implementation of something that should really be in core R. The traceback() function is well established so I hope we can build on that.

gaborcsardi commented 5 years ago

The traceback() output is not the best, imo. E.g. rlang's traceback trees are much better.

lionel- commented 5 years ago

But Henrik is right this should be part of R core. Ideally the structured and chained backtraces we've been experimenting with would make their way into R. My only concern about setTraceback() is that it might get in the way of changes to the baseenv()$.Traceback data structure.

HenrikBengtsson commented 5 years ago

[...] My only concern about setTraceback() is that it might get in the way of changes to the baseenv()$.Traceback data structure.

This is a valid concern. Right now the format is not really documented and it looks like it's just a character vector + source-reference attributes. As you guys say, there's room for improvement/enrichment of what can be conveyed.

Then, how can current traceback() be improved with enhanced features while still allowing it to work as it does right now? First of all, if we look at base::traceback(), we see that it's effectively doing:

traceback <- function(x = NULL, max.lines = getOption("deparse.max.lines"))
{
    x <- .traceback(x)
    printTraceback(x, max.lines = max.lines)
    invisible(x)
}

In other words, a first step to introduce flexibility to the traceback framework is to go down the S3 method path, e.g.

https://github.com/HenrikBengtsson/Wishlist-for-R/blob/af9ef5173c6c98bf05a8304a2577e72aa8364996/R/traceback.R#L29-L67

This would open up for extending this new traceback class with new formats that each have their own print() method (and possible other methods). E.g. does your rlang work fit into this minimal enhancement?

lionel- commented 5 years ago

Interesting suggestion.

does your rlang work fit into this minimal enhancement?

It does, the entrace() error handler could just store a backtrace object there, and the print.rlang_trace() method would kick in when the user calls traceback().