Check that a chromote connection is alive (#136)

* Check that a chromote connection is alive When your computer goes to sleep, the websocket and all debugging sessions are closed. This PR ensures that Chromote and ChromoteSession report this accurately. I distinguished between active and alive for Chromote but not ChromoteSession since the browser process continues to run in the background, even though we have lost our connection to it. There's no way to restore a closed debugging session. However, I'm not sure about the names; maybe I have alive and active around the wrong way? Part of #94 * `devtools::document()` (GitHub Actions) * Proof of concept for auto reconnect * `devtools::document()` (GitHub Actions) * Drop is_active_ private variable * Better browser closing logic * Track session/target separately * Update news * Polish docs * Improve closing logic some more * Use target_closed parameter --------- Co-authored-by: hadley <[email protected]>
rstudio · Feb 2, 2024 · bbc13af · bbc13af
1 parent 454e194
commit bbc13af
Show file tree

Hide file tree

Showing 9 changed files with 161 additions and 47 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,5 +1,9 @@
 # chromote (development version)
 
+* `Chromote` gains a new `is_alive()` method equivalent to the old `is_active()` method; i.e. it reports on if there is an active chrome process running in the background.
+
+* Breaking change: `Chromote$is_active()` method now reports if there is an active connection to the underlying chrome instance, rather than whether or not that instance is alive (#94).
+
 * `--disable-gpu` is no longer included in the default Chrome arguments.
 
 * `ChromoteSession` now records the `targetId`. This eliminates one round-trip to the browser when viewing or closing a session. You can now call the `$respawn()` method if a session terminates and you want to reconnect to the same target (#94).

diff --git a/R/browser.R b/R/browser.R
@@ -23,6 +23,9 @@ Browser <- R6Class("Browser",
     #' @description Browser process
     get_process = function() private$process,
 
+    #' @description Is the process alive?
+    is_alive = function() private$process$is_alive(),
+
     #' @description Browser Host
     get_host = function() private$host,
 

diff --git a/R/chromote.R b/R/chromote.R
@@ -53,7 +53,6 @@ Chromote <- R6Class(
       list2env(self$protocol, self)
 
       private$event_manager <- EventManager$new(self)
-      private$is_active_ <- TRUE
 
       self$wait_for(p)
 
@@ -174,6 +173,7 @@ Chromote <- R6Class(
     #'   `ChromoteSession` object. Otherwise, block during initialization, and
     #'   return a `ChromoteSession` object directly.
     new_session = function(width = 992, height = 1323, targetId = NULL, wait_ = TRUE) {
+      self$check_active()
       create_session(
         chromote = self,
         width = width,
@@ -213,9 +213,7 @@ Chromote <- R6Class(
     #' @param sessionId Determines which [`ChromoteSession`] with the
     #' corresponding to send the command to.
     send_command = function(msg, callback = NULL, error = NULL, timeout = NULL, sessionId = NULL) {
-      if (!private$is_active_) {
-        stop("Chromote object is closed.")
-      }
+      self$check_active()
 
       private$last_msg_id <- private$last_msg_id + 1
       msg$id <- private$last_msg_id
@@ -320,11 +318,40 @@ Chromote <- R6Class(
       paste0("http://", private$browser$get_host(), ":", private$browser$get_port(), path)
     },
 
-    #' @description Retrieve active status
-    #' Once initialized, the value returned is `TRUE`. If `$close()` has been
-    #' called, this value will be `FALSE`.
+    #' @description
+    #' Is there an active websocket connection to the browser process?
     is_active = function() {
-      private$is_active_
+      self$is_alive() && private$ws$readyState() %in% c(0L, 1L)
+    },
+
+    #' @description
+    #' Is the underlying browser process running?
+    is_alive = function() {
+      private$browser$is_alive()
+    },
+
+    #' @description Check that a chromote instance is active and alive.
+    #'  Will automatically reconnect if browser process is alive, but
+    #'  there's no active web socket connection.
+    check_active = function() {
+      if (!self$is_alive()) {
+        stop("Chromote has been closed.")
+      }
+
+      if (!self$is_active()) {
+        inform(c(
+          "!" = "Reconnecting to chrome process.",
+          i = "All active sessions will be need to be respawned.")
+        )
+        self$connect()
+
+        # Mark all sessions as closed
+        for (session in private$sessions) {
+          session$mark_closed(FALSE)
+        }
+        private$sessions <- list()
+      }
+      invisible(self)
     },
 
     #' @description Retrieve [`Browser`]` object
@@ -335,13 +362,24 @@ Chromote <- R6Class(
 
     #' @description Close the [`Browser`] object
     close = function() {
-      if (private$is_active_) {
-        self$Browser$close()
-        private$is_active_ <- FALSE
-        return(TRUE)
-      } else {
-        FALSE
+      # Must be alive to be active so we cache value before closing process
+      is_active <- self$is_active()
+
+      if (self$is_alive()) {
+        if (is_active) {
+          # send a message to the browser requesting that it close
+          self$Browser$close()
+        } else {
+          # terminate the process
+          private$browser$close()
+        }
       }
+
+      if (is_active) {
+        private$ws$close()
+      }
+
+      invisible()
     },
 
     #' @field default_timeout Default timeout in seconds for \pkg{chromote} to
@@ -354,7 +392,6 @@ Chromote <- R6Class(
   private = list(
     browser = NULL,
     ws = NULL,
-    is_active_ = NULL,
 
     # =========================================================================
     # Browser commands
@@ -400,9 +437,7 @@ Chromote <- R6Class(
     event_manager = NULL,
 
     register_event_listener = function(event, callback = NULL, timeout = NULL) {
-      if (!private$is_active_) {
-        stop("Chromote object is closed.")
-      }
+      self$check_active()
       private$event_manager$register_event_listener(event, callback, timeout)
     },
 
@@ -416,7 +451,7 @@ Chromote <- R6Class(
           return()
 
         private$sessions[[sid]] <- NULL
-        session$mark_closed()
+        session$mark_closed(TRUE)
       })
     },
 
@@ -496,7 +531,7 @@ default_chromote_object <- function() {
 #' @rdname default_chromote_object
 #' @export
 has_default_chromote_object <- function() {
-  !is.null(globals$default_chromote) && globals$default_chromote$is_active()
+  !is.null(globals$default_chromote) && globals$default_chromote$is_alive()
 }
 
 #' @param x A \code{\link{Chromote}} object.

diff --git a/R/chromote_session.R b/R/chromote_session.R
@@ -98,7 +98,8 @@ ChromoteSession <- R6Class(
 
       private$auto_events <- auto_events
       private$event_manager <- EventManager$new(self)
-      private$is_active_ <- TRUE
+      private$session_is_active <- TRUE
+      private$target_is_active <- TRUE
 
       # Find pixelRatio for screenshots
       p <- p$
@@ -117,7 +118,7 @@ ChromoteSession <- R6Class(
               warning("Chromote has received a Inspector.targetCrashed event. This means that the ChromoteSession has probably crashed.")
               # Even if no targetId nor sessionId is returned by Inspector.targetCashed
               # mark the session as closed. This will close all sessions..
-              self$mark_closed()
+              self$mark_closed(TRUE)
             })
           })
       }
@@ -184,6 +185,10 @@ ChromoteSession <- R6Class(
     #' when the `ChromoteSession` is closed. Otherwise, block until the
     #' `ChromoteSession` has closed.
     close = function(wait_ = TRUE) {
+      if (!private$target_is_active) {
+        return(invisible())
+      }
+
       # Even if this session calls Target.closeTarget, the response from
       # the browser is sent without a sessionId. In order to wait for the
       # correct browser response, we need to invoke this from the parent's
@@ -192,7 +197,7 @@ ChromoteSession <- R6Class(
 
       p <- p$then(function(value) {
         if (isTRUE(value$success)) {
-          self$mark_closed()
+          self$mark_closed(TRUE)
         }
         invisible(value$success)
       })
@@ -429,7 +434,7 @@ ChromoteSession <- R6Class(
     #' as this session. This is useful if the session has been closed but the target still
     #' exists.
     respawn = function() {
-      if (!private$is_active_) {
+      if (!private$target_is_active) {
         stop("Can't respawn session; target has been closed.")
       }
 
@@ -507,9 +512,7 @@ ChromoteSession <- R6Class(
     #' @param timeout Number of milliseconds for Chrome DevTools Protocol
     #' execute a method.
     send_command = function(msg, callback = NULL, error = NULL, timeout = NULL) {
-      if (!private$is_active_) {
-        stop("Session ", private$session_id, " is closed.")
-      }
+      self$check_active()
       self$parent$send_command(msg, callback, error, timeout, sessionId = private$session_id)
     },
 
@@ -535,19 +538,36 @@ ChromoteSession <- R6Class(
       private$event_manager$invoke_event_callbacks(event, params)
     },
 
-    #' @description
-    #' Disable callbacks for a given session.
-    #'
-    #' For internal use only.
-    mark_closed = function() {
-      private$is_active_ <- FALSE
+    #' @description Mark a session, and optionally, the underlying target,
+    #'   as closed. For internal use only.
+    #' @param target_closed Has the underlying target been closed as well as the
+    #'   active debugging session?
+    mark_closed = function(target_closed) {
+      private$session_is_active <- FALSE
+      private$target_is_active <- target_closed
     },
 
     #' @description Retrieve active status
     #' Once initialized, the value returned is `TRUE`. If `$close()` has been
     #' called, this value will be `FALSE`.
     is_active = function() {
-      private$is_active_
+      private$session_is_active && private$target_is_active && self$parent$is_active()
+    },
+
+    #' @description Check that a session is active, erroring if not.
+    check_active = function() {
+      if (self$is_active()) {
+        return()
+      }
+
+      if (private$target_is_active) {
+        abort(c(
+          "Session has been closed.",
+          i = "Call session$respawn() to create a new session that connects to the same target."
+        ))
+      } else {
+        abort("Session and underlying target have been closed.")
+      }
     },
 
     #' @description Initial promise
@@ -569,16 +589,15 @@ ChromoteSession <- R6Class(
   private = list(
     session_id = NULL,
     target_id = NULL,
-    is_active_ = NULL,
+    session_is_active = NULL,
+    target_is_active = NULL,
     event_manager = NULL,
     pixel_ratio = NULL,
     auto_events = NULL,
     init_promise_ = NULL,
 
     register_event_listener = function(event, callback = NULL, timeout = NULL) {
-      if (!private$is_active_) {
-        stop("Session ", private$session_id, " is closed.")
-      }
+      self$check_active()
       private$event_manager$register_event_listener(event, callback, timeout)
     }
   )

diff --git a/man/Browser.Rd b/man/Browser.Rd
diff --git a/man/Chrome.Rd b/man/Chrome.Rd
diff --git a/man/ChromeRemote.Rd b/man/ChromeRemote.Rd
diff --git a/man/Chromote.Rd b/man/Chromote.Rd