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

How to properly close the Chromote session in a package? #109

Closed
stla opened this issue Jul 1, 2023 · 11 comments · Fixed by #111
Closed

How to properly close the Chromote session in a package? #109

stla opened this issue Jul 1, 2023 · 11 comments · Fixed by #111

Comments

@stla
Copy link
Contributor

stla commented Jul 1, 2023

Hello,

I've made a RStudio addin with chromote (it uses a JavaScript library to reindent some code).

I do as follows to close the Chromote session:

  ......
  result <- session$Runtime$evaluate("editor.getValue();")
  value <- result$result$value
  on.exit(session$close())
  value

Is it ok? I read somewhere that to really shut down the session, one has to do session$parent$close(). But if I do that my package does not work, I get the error Chromote object is closed.

@stla
Copy link
Contributor Author

stla commented Jul 1, 2023

I've just seen this SO thread and then I tried:

  parentBrowser <- session$parent$get_browser()
  on.exit(parentBrowser$close())

Then the package works but I get this message:

[2023-07-01 02:40:26] [error] handle_read_frame error: asio.system:10054 (An existing connection was forcibly closed by the remote host.)

@stla
Copy link
Contributor Author

stla commented Jul 19, 2023

This throws a true R error. And RCMD Check complains that there are connections style open at the end.

@stla
Copy link
Contributor Author

stla commented Jul 19, 2023

I get that when I check my package:

 > cleanEx()
Error: -07-19 09:56:21] [error] handle_read_frame error: websocketpp.transport:7 (End of File)
  Error: connections left open:
  	/tmp/RtmpI2u7Qc/working_dir/RtmpSVIMYD/supervisor_stdin1da34ccd933f (fifo)
  	/tmp/RtmpI2u7Qc/working_dir/RtmpSVIMYD/supervisor_stdout1da37a2efa39 (fifo)
  Execution halted

@stla
Copy link
Contributor Author

stla commented Jul 19, 2023

I read somewhere that to really shut down the session, one has to do session$parent$close(). But if I do that my package does not work, I get the error Chromote object is closed.

I know why this error occurs.

The session$parent$close function is:

function () {
    private$is_active_ <- FALSE
    self$Browser$close()
}

And the session$Browser$close function calls self$send_command. But in the send_command function, there is:

    if (!private$is_active_) {
        stop("Chromote object is closed.")
    }

Hence the error.

@wch
Copy link
Collaborator

wch commented Jul 19, 2023

Thanks for the investigation into the problem! I've just pushed a fix for it.

@stla
Copy link
Contributor Author

stla commented Jul 19, 2023

Thanks! Does this fix solves the RCMD Check problem with the unclosed connection ?

Oddly, I don't have this problem when I RCMD Check on my Windows laptop, but it occurs on Ubuntu in the Github action.

@wch
Copy link
Collaborator

wch commented Jul 19, 2023

Hm, I'm not sure about R CMD check. It looks like the workflow on this repo was disabled automatically due to inactivity. I just re-enabled it, but I'm not sure if it's possible to kick off another run without pushing a new commit.

Update: I just pushed a commit to trigger the GHA workflow.

@stla
Copy link
Contributor Author

stla commented Jul 19, 2023

I've just tested your fix and session$parent$close() returns FALSE.

@wch
Copy link
Collaborator

wch commented Jul 19, 2023

I've just tested your fix and session$parent$close() returns FALSE.

That just happens to be the value of the last expression in the function. Do you think it should return something else?

@stla
Copy link
Contributor Author

stla commented Jul 19, 2023

session$close() returns TRUE. I thought this means "ok, the session has been closed". The FALSE is misleading for me.

@wch
Copy link
Collaborator

wch commented Jul 19, 2023

OK, I just pushed a change so that it returns TRUE when it closes. Subsequent calls to $close() return FALSE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants