-
Notifications
You must be signed in to change notification settings - Fork 20
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
Check that a chromote connection is alive #136
Changes from 11 commits
acc89fc
e2a7a95
2fd9552
6e0cf52
f5a7850
c029dce
6df996a
add90d3
e8656a5
b1cffad
2f39a89
ef173a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 <- FALSE | ||
}, | ||
|
||
#' @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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't mind changing the name, but calling it So while I agree that "active" isn't the best name, and has some potential for confusion, I think changing it to "alive" is a net neutral because it just introduces a different source confusion. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, let's keep it as "active" then. |
||
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) | ||
} | ||
) | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like
target_closed
is unused?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doh. I realised that I forgot to call
spawn()
before closing in my script test. Here's the new output: