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

closeAllConnections() causes render and preview failure #5214

Closed
5 tasks done
guslipkin opened this issue Apr 17, 2023 · 7 comments
Closed
5 tasks done

closeAllConnections() causes render and preview failure #5214

guslipkin opened this issue Apr 17, 2023 · 7 comments
Assignees
Labels
bug Something isn't working knitr upstream Bug is in upstream library

Comments

@guslipkin
Copy link

Bug description

I'm not sure how much can be done about this, but some packages use closeAllConnections() internally and therefore kill the connection to the quarto renderer.

With this file:

---
title: "test"
---

```{r}
closeAllConnections()
```

quarto::quarto_preview() produces a slightly more detailed error message than quarto::quarto_render() with:

processing file: Untitled.qmd
  |...................................                 |  67% (unnamed-chunk-1)Quitting from lines 6-7 (Untitled.qmd) 
Error in isIncomplete(con) : invalid connection
Calls: .main ... evaluate_call -> handle_output -> <Anonymous> -> isIncomplete
Quitting from lines 6-7 (Untitled.qmd) 
Error in isOpen(con) : invalid connection
Calls: .main ... <Anonymous> -> evaluate_call -> <Anonymous> -> isOpen
                                                                                                            
Execution halted
Error in run_serve_daemon("preview", file, NULL, args, render, port, host,  : 
  Error starting quarto

Details:

This appears to be cross platform, but my device info is as follows:

  • OS: macOS 13.3 (22E252) (M1)
  • RStudio:
RStudio 2023.03.0+386 "Cherry Blossom" Release (3c53477afb13ab959aeb5b34df1f10c237b256c3, 2023-03-09) for macOS
Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) RStudio/2023.03.0+386 Chrome/108.0.5359.179 Electron/22.0.3 Safari/537.36
  • R:
platform       aarch64-apple-darwin20      
arch           aarch64                     
os             darwin20                    
system         aarch64, darwin20           
status                                     
major          4                           
minor          2.2                         
year           2022                        
month          10                          
day            31                          
svn rev        83211                       
language       R                           
version.string R version 4.2.2 (2022-10-31)
nickname       Innocent and Trusting  
  • Quarto:
quarto check

[] Checking versions of quarto binary dependencies...
      Pandoc version 3.1.1: OK
      Dart Sass version 1.55.0: OK
[] Checking versions of quarto dependencies......OK
[] Checking Quarto installation......OK
      Version: 1.3.326
      Path: /Applications/quarto/bin

[] Checking basic markdown render....OK

[] Checking Python 3 installation....OK
      Version: 3.10.1
      Path: /Library/Frameworks/Python.framework/Versions/3.10/bin/python3
      Jupyter: 4.10.0
      Kernels: julia-1.7, python3

[] Checking Jupyter engine render....OK

[] Checking R installation...........OK
      Version: 4.2.2
      Path: /Library/Frameworks/R.framework/Resources
      LibPaths:
        - /Library/Frameworks/R.framework/Versions/4.2-arm64/Resources/library
      knitr: 1.42
      rmarkdown: 2.21

[] Checking Knitr engine render......OK

Checklist

  • Please include a minimal, fully reproducible example in a single .qmd file? Please provide the whole file rather than the snippet you believe is causing the issue.
  • Please format your issue so it is easier for us to read the bug report.
  • Please document the RStudio IDE version you're running (if applicable), by providing the value displayed in the "About RStudio" main menu dialog?
  • Please document the operating system you're running. If on Linux, please provide the specific distribution.
  • Please provide the output of quarto check so we know which version of quarto and its dependencies you're running.
@guslipkin guslipkin added the bug Something isn't working label Apr 17, 2023
@cderv
Copy link
Collaborator

cderv commented Apr 17, 2023

I'm not sure how much can be done about this, but some packages use closeAllConnections() internally and therefore kill the connection to the quarto renderer.

This will even be an issue with knitr itself and not just Quarto or rmarkdown. It won't render either it you try to knitr::knit() this example document.

I never seen report of this yet. closeAllConnections() is among the function that shouldn't be use in a knitting context. I would like to investigate further based on usage of this. knitr uses evaluate 📦 and it should try to catch such usage and warning or error more gracefully (r-lib/evaluate#23)

Can you point me with some package that does that ?

Thanks

@cderv cderv self-assigned this Apr 17, 2023
@guslipkin
Copy link
Author

Ah. I wonder if jupyter would face a similar issue. I'm not as well versed in python as I would like to know how to test that. I can, however, confirm that a regular rmd knit fails as you suggested it would.

This was noticed in the pullData function from this UT Dallas internal package

I believe a UTD member is planning a PR there to remove the line, but I could see someone using it in their own code as well and not necessarily recognizing the error message and cause.

@cderv
Copy link
Collaborator

cderv commented Apr 17, 2023

but I could see someone using it in their own code as well and not necessarily recognizing the error message and cause.

FWIW I don't find any reference to such issue opened in one of the repo over time. It does not seem that common to use this feature.

Maybe something has changed over time in R, but only thing we can do I think is to just error better like in r-lib/evaluate#23 but in another place. It seems the connection is altered and became invalid before the check was made. isOpen(con) does not return FALSE but just errors.

So overall, closeAllConnections() can't be used in a knitting context. A text connection is open during the knitr process to catch output, warning, etc... If close, then it won't work.

@yihui quite old issue r-lib/evaluate#23 but I think we need to protect better in evaluate, don't we ?

@cderv cderv added upstream Bug is in upstream library knitr labels Apr 17, 2023
@yihui
Copy link
Contributor

yihui commented Apr 17, 2023

isOpen(con) does not return FALSE but just errors.

Perhaps we should try() and detect error there.

@cderv
Copy link
Collaborator

cderv commented Apr 17, 2023

I was thinking of that yes. But the first one happens at
https://github.com/r-lib/evaluate/blob/17918f626659ca3d1e677a6b9dde5b66613c7314/R/watcher.R#L18

processing file: test.Rmd
  |............................................................................................................................| 100% [unnamed-chunk-2]
Quitting from lines 11-12 [unnamed-chunk-2] (test.Rmd)
Error in `isIncomplete()`:
! invalid connection
Backtrace:
  1. knitr::knit("test.Rmd")
  2. knitr:::process_file(text, output)
  6. knitr:::process_group.block(group)
  7. knitr:::call_block(x)
  8. knitr:::block_exec(params)
     ...
 13. evaluate::evaluate(...)
 14. evaluate:::evaluate_call(...)
 15. evaluate (local) handle_output(TRUE)
 16. w$get_new(plot, incomplete_plots, output_handler$text, output_handler$graphics)
 17. base::isIncomplete(con)

Quitting from lines 11-12 [unnamed-chunk-2] (test.Rmd)
Error in `isOpen()`:
! invalid connection
Backtrace:
  1. knitr::knit("test.Rmd")
  2. knitr:::process_file(text, output)
  6. knitr:::process_group.block(group)
  7. knitr:::call_block(x)
  8. knitr:::block_exec(params)
     ...
 12. knitr (local) evaluate(...)
 13. evaluate::evaluate(...)
 14. evaluate:::evaluate_call(...)
 15. w$close()
 16. base::isOpen(con)

Seems like the all connection closing is make the con object invalid, which requires us an extra check maybe

yihui added a commit to r-lib/evaluate that referenced this issue Apr 17, 2023
@yihui
Copy link
Contributor

yihui commented Apr 17, 2023

Seems like the all connection closing is make the con object invalid, which requires us an extra check maybe

Yes, and done: r-lib/evaluate@25c3ceb

@cderv
Copy link
Collaborator

cderv commented Apr 17, 2023

Awesome thanks @yihui !

@guslipkin I don't think we can do much more here in Quarto so I'll close this thread.

Thanks a lot for the report!

@cderv cderv closed this as completed Apr 17, 2023
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Jun 11, 2023
Version 0.21
================================================================================

- `evaluate()` gains `log_echo` and `log_warning` arguments. When set to `TRUE`
  these cause code and warnings (respectively) to be immediately emitted to
  `stderr()`. This is useful for logging in unattended environments (#118).

- Improved the error message when users accidentally called
  `closeAllConnections()` (thanks, @guslipkin,
  quarto-dev/quarto-cli#5214).

Version 0.20
================================================================================

- The arguments `keep_message` and `keep_warning` of `evaluate()` can
  take the value `NA` now, which means `evaluate()` will not capture
  the messages and they will be sent to the console. This is
  equivalent to the `FALSE` value before v0.19 (thanks, @gadenbuie,
  yihui/yihui.org#1458).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working knitr upstream Bug is in upstream library
Projects
None yet
Development

No branches or pull requests

3 participants