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.

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

evaluate("sink(...)") crashes the R session #104

Closed
Patrick330 opened this issue Nov 5, 2021 · 4 comments · Fixed by #174
Closed

evaluate("sink(...)") crashes the R session #104

Patrick330 opened this issue Nov 5, 2021 · 4 comments · Fixed by #174
Labels
bug an unexpected problem or unintended behavior

Comments

@Patrick330
Copy link

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
Copy link
Author

Would someone be able to triage this?

@yihui
Copy link
Collaborator

yihui commented Jan 13, 2022

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

@Patrick330
Copy link
Author

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.

@hadley
Copy link
Member

hadley commented Jun 14, 2024

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 added a commit that referenced this issue Jun 23, 2024
I'm not 100% convinced that silently restoring the sink + connection is the correct approach, but it doesn't happen very often and we aren't going to lose much output if we immediately re-open the sink + connection.

Fixes #104 (at least as much as it can be fixed)
@hadley hadley closed this as completed in 1aa6c64 Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants