evaluate icon indicating copy to clipboard operation
evaluate copied to clipboard

`evaluate("sink(...)")` crashes the R session

Open Patrick330 opened this issue 4 years ago • 3 comments

Context

I am writing a package that implements a standardized logging function using sink(...). I would like to be able to use RMarkdown to write the vignettes for the package. However, my functions do not execute correctly when rendering the Rmd files due to evaluate's treatment of the sink commands passed in by rmarkdown::render.

Issue

The use of sink(...) within evaluate() crashes the R session. The function should provide similar behavior to eval(parse(text='sink(...)')) which, correctly, executes the sink() command and returns no other output. Alternately, because this is difficult to resolve, evaluate() could return an error or a warning which could be handled by other consumers of evaluate, particularly rmarkdown::render. An easy way to catch this issue would be to observe if new connections were added or if sink.number() increased within the w$close function returned by watchout().

Reprex

evaluate("sink('temp.txt')") produces: Error: invalid connection, and no commands will execute until the R session is restarted.

sessionInfo()

R version 4.1.2 (2021-11-01) Platform: x86_64-w64-mingw32/x64 (64-bit) Running under: Windows 10 x64 (build 19043), RStudio 2021.9.0.351

Locale: LC_COLLATE=English_United States.1252 LC_CTYPE=English_United States.1252 LC_MONETARY=English_United States.1252 LC_NUMERIC=C LC_TIME=English_United States.1252

Package version: compiler_4.1.2 evaluate_0.14 graphics_4.1.2 grDevices_4.1.2 methods_4.1.2 stats_4.1.2 tools_4.1.2
utils_4.1.2 xfun_0.27

Patrick330 avatar Nov 05 '21 05:11 Patrick330

Would someone be able to triage this?

Patrick330 avatar Jan 06 '22 20:01 Patrick330

We can definitely throw an error if sink.number() has changed. Would you be okay with that?

yihui avatar Jan 13 '22 23:01 yihui

In my specific use case we would also need to modify rmarkdown::render() to handle this case, perhaps by using eval() rather than evaluate(). But in any case, I think providing an error specific to the issue is an improvement.

Patrick330 avatar Jan 14 '22 03:01 Patrick330

Another option would be to override sink() in the environment in which evaluate runs code. We could also provide a similar error for closeAllConnections().

There's already an existing mechanism for this in inject_funs()

hadley avatar Jun 14 '24 19:06 hadley