mirror of
https://github.com/lexmount/moli.git
synced 2026-09-23 16:01:25 +00:00
fix(webdriver): terminate timed-out script execution
This commit is contained in:
@@ -42,6 +42,7 @@ pub(super) fn devtools_command_uses_interleaved_runtime_dispatch(
|
||||
DevToolsCommand::GetRealms(_)
|
||||
| DevToolsCommand::EvaluateScript(_)
|
||||
| DevToolsCommand::CallFunction(_)
|
||||
| DevToolsCommand::TerminateExecution(_)
|
||||
| DevToolsCommand::LocateNodes(_)
|
||||
| DevToolsCommand::ReleaseObjects(_)
|
||||
)
|
||||
|
||||
@@ -10962,6 +10962,69 @@ async fn webdriver_classic_execute_sync_honors_script_timeout() {
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn webdriver_classic_execute_sync_timeout_interrupts_non_yielding_script() {
|
||||
let app = build_router(test_state());
|
||||
|
||||
let session = classic_request_json(app.clone(), Method::POST, "/session").await;
|
||||
let session_id = session["value"]["sessionId"]
|
||||
.as_str()
|
||||
.expect("classic session id");
|
||||
|
||||
let set_timeouts = classic_request_json_with_body(
|
||||
app.clone(),
|
||||
Method::POST,
|
||||
&format!("/session/{session_id}/timeouts"),
|
||||
json!({ "script": 100 }),
|
||||
)
|
||||
.await;
|
||||
assert_eq!(set_timeouts, json!({ "value": null }));
|
||||
|
||||
let (timeout_status, timeout_response) = tokio::time::timeout(
|
||||
Duration::from_secs(10),
|
||||
classic_request_status_and_json_with_body(
|
||||
app.clone(),
|
||||
Method::POST,
|
||||
&format!("/session/{session_id}/execute/sync"),
|
||||
json!({
|
||||
"script": "for (;;) {}",
|
||||
"args": []
|
||||
}),
|
||||
),
|
||||
)
|
||||
.await
|
||||
.expect("non-yielding script timeout must interrupt V8 and return");
|
||||
assert_eq!(timeout_status, StatusCode::REQUEST_TIMEOUT);
|
||||
assert_eq!(timeout_response["value"]["error"], json!("script timeout"));
|
||||
|
||||
let reset_timeouts = classic_request_json_with_body(
|
||||
app.clone(),
|
||||
Method::POST,
|
||||
&format!("/session/{session_id}/timeouts"),
|
||||
json!({ "script": 1000 }),
|
||||
)
|
||||
.await;
|
||||
assert_eq!(reset_timeouts, json!({ "value": null }));
|
||||
|
||||
let recovered = tokio::time::timeout(
|
||||
Duration::from_secs(5),
|
||||
classic_request_json_with_body(
|
||||
app.clone(),
|
||||
Method::POST,
|
||||
&format!("/session/{session_id}/execute/sync"),
|
||||
json!({
|
||||
"script": "return 42;",
|
||||
"args": []
|
||||
}),
|
||||
),
|
||||
)
|
||||
.await
|
||||
.expect("renderer must accept another script after timeout termination");
|
||||
assert_eq!(recovered, json!({ "value": 42 }));
|
||||
|
||||
let _ = classic_request_json(app, Method::DELETE, &format!("/session/{session_id}")).await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn webdriver_classic_execute_async_honors_script_timeout() {
|
||||
let app = build_router(test_state());
|
||||
|
||||
@@ -2523,7 +2523,7 @@ pub(super) async fn webdriver_classic_execute_sync(
|
||||
apply_classic_script_argument_handles(&mut command, &script_argument_handles);
|
||||
let result = binding
|
||||
.runtime
|
||||
.execute_inner(command, binding.timeouts.script.map(Duration::from_millis))
|
||||
.execute_script(command, binding.timeouts.script.map(Duration::from_millis))
|
||||
.await;
|
||||
release_classic_remote_objects(
|
||||
&binding,
|
||||
@@ -2594,7 +2594,7 @@ pub(super) async fn webdriver_classic_execute_async(
|
||||
apply_classic_script_argument_handles(&mut command, &script_argument_handles);
|
||||
let result = binding
|
||||
.runtime
|
||||
.execute_inner(command, binding.timeouts.script.map(Duration::from_millis))
|
||||
.execute_script(command, binding.timeouts.script.map(Duration::from_millis))
|
||||
.await;
|
||||
release_classic_remote_objects(
|
||||
&binding,
|
||||
@@ -3866,7 +3866,7 @@ pub(super) async fn webdriver_classic_get_element_property(
|
||||
apply_classic_script_argument_handles(&mut command, &script_argument_handles);
|
||||
let result = binding
|
||||
.runtime
|
||||
.execute_inner(command, binding.timeouts.script.map(Duration::from_millis))
|
||||
.execute_script(command, binding.timeouts.script.map(Duration::from_millis))
|
||||
.await;
|
||||
release_classic_remote_objects(
|
||||
&binding,
|
||||
|
||||
@@ -15,7 +15,7 @@ use moli_protocol::{
|
||||
DevToolsCommand, DevToolsCommandContext, DevToolsCommandResult, DevToolsDomNodeReference,
|
||||
DevToolsError, DevToolsErrorKind, DevToolsFrameId, DevToolsGetFrameOwnerCommand,
|
||||
DevToolsGetFrameOwnerResult, DevToolsGetFrameTreeCommand, DevToolsProtocol,
|
||||
DevToolsSessionId, DevToolsTargetId,
|
||||
DevToolsSessionId, DevToolsTargetId, DevToolsTerminateExecutionCommand,
|
||||
},
|
||||
};
|
||||
use moli_protocol_webdriver_classic::{
|
||||
@@ -38,6 +38,8 @@ use super::super::webdriver_bidi::{
|
||||
};
|
||||
use super::super::{CookieProfileCommit, protocol_local_executor::spawn_protocol_local_task};
|
||||
|
||||
const CLASSIC_SCRIPT_TERMINATION_TIMEOUT: Duration = Duration::from_secs(5);
|
||||
|
||||
#[derive(Debug, Clone, Default)]
|
||||
pub(in crate::protocol_server) struct SharedClassicSessionRegistry {
|
||||
inner: Arc<Mutex<ClassicSessionManager>>,
|
||||
@@ -536,7 +538,17 @@ impl ClassicSessionRuntimeHandle {
|
||||
command: DevToolsCommand,
|
||||
timeout: Option<Duration>,
|
||||
) -> Result<DevToolsCommandResult, DevToolsError> {
|
||||
self.execute_with_options(command, timeout, None).await
|
||||
self.execute_with_options(command, timeout, None, false)
|
||||
.await
|
||||
}
|
||||
|
||||
pub(super) async fn execute_script(
|
||||
&self,
|
||||
command: DevToolsCommand,
|
||||
timeout: Option<Duration>,
|
||||
) -> Result<DevToolsCommandResult, DevToolsError> {
|
||||
self.execute_with_options(command, timeout, None, true)
|
||||
.await
|
||||
}
|
||||
|
||||
pub(super) async fn execute_with_pending_navigation_wait(
|
||||
@@ -545,7 +557,7 @@ impl ClassicSessionRuntimeHandle {
|
||||
timeout: Option<Duration>,
|
||||
pending_navigation_timeout: Option<Duration>,
|
||||
) -> Result<DevToolsCommandResult, DevToolsError> {
|
||||
self.execute_with_options(command, timeout, pending_navigation_timeout)
|
||||
self.execute_with_options(command, timeout, pending_navigation_timeout, false)
|
||||
.await
|
||||
}
|
||||
|
||||
@@ -582,6 +594,7 @@ impl ClassicSessionRuntimeHandle {
|
||||
command: DevToolsCommand,
|
||||
timeout: Option<Duration>,
|
||||
pending_navigation_timeout: Option<Duration>,
|
||||
terminate_execution_on_timeout: bool,
|
||||
) -> Result<DevToolsCommandResult, DevToolsError> {
|
||||
let (response_tx, response_rx) = oneshot::channel();
|
||||
self.tx
|
||||
@@ -589,6 +602,7 @@ impl ClassicSessionRuntimeHandle {
|
||||
command: Box::new(command),
|
||||
timeout,
|
||||
pending_navigation_timeout,
|
||||
terminate_execution_on_timeout,
|
||||
response_tx,
|
||||
})
|
||||
.map_err(|_| {
|
||||
@@ -793,6 +807,7 @@ enum ClassicSessionRuntimeRequest {
|
||||
command: Box<DevToolsCommand>,
|
||||
timeout: Option<Duration>,
|
||||
pending_navigation_timeout: Option<Duration>,
|
||||
terminate_execution_on_timeout: bool,
|
||||
response_tx: oneshot::Sender<Result<DevToolsCommandResult, DevToolsError>>,
|
||||
},
|
||||
WaitForDocumentLifecycle {
|
||||
@@ -880,9 +895,11 @@ async fn handle_classic_session_runtime_request(
|
||||
command,
|
||||
timeout,
|
||||
pending_navigation_timeout,
|
||||
terminate_execution_on_timeout,
|
||||
response_tx,
|
||||
} => {
|
||||
let execution = execute_classic_devtools_command_with_pending_navigation_retry(
|
||||
let termination_context = command.context().clone();
|
||||
let mut execution = execute_classic_devtools_command_with_pending_navigation_retry(
|
||||
scheduler,
|
||||
receivers,
|
||||
*command,
|
||||
@@ -890,6 +907,34 @@ async fn handle_classic_session_runtime_request(
|
||||
pending_navigation_timeout,
|
||||
)
|
||||
.await;
|
||||
if terminate_execution_on_timeout
|
||||
&& matches!(
|
||||
execution.result,
|
||||
Err(ref error) if error.kind == DevToolsErrorKind::Timeout
|
||||
)
|
||||
{
|
||||
// Finish the IO-side termination before the HTTP handler
|
||||
// releases argument handles or admits the next Classic
|
||||
// command on this session.
|
||||
let termination = execute_classic_devtools_command_once(
|
||||
scheduler,
|
||||
receivers,
|
||||
DevToolsCommand::TerminateExecution(DevToolsTerminateExecutionCommand {
|
||||
context: termination_context,
|
||||
}),
|
||||
Some(CLASSIC_SCRIPT_TERMINATION_TIMEOUT),
|
||||
)
|
||||
.await;
|
||||
if let Err(error) = &termination.result {
|
||||
tracing::warn!(
|
||||
?error,
|
||||
"failed to terminate timed-out WebDriver Classic script execution"
|
||||
);
|
||||
}
|
||||
execution
|
||||
.protocol_output
|
||||
.append(termination.protocol_output);
|
||||
}
|
||||
let result = execution.result;
|
||||
let keep_attached = if let Some(attached) = attached_bidi.as_mut() {
|
||||
attached
|
||||
|
||||
@@ -256,6 +256,7 @@ impl CdpConnection {
|
||||
command @ (DevToolsCommand::GetRealms(_)
|
||||
| DevToolsCommand::EvaluateScript(_)
|
||||
| DevToolsCommand::CallFunction(_)
|
||||
| DevToolsCommand::TerminateExecution(_)
|
||||
| DevToolsCommand::LocateNodes(_)
|
||||
| DevToolsCommand::ReleaseObjects(_)) => {
|
||||
let output = Box::pin(
|
||||
|
||||
@@ -216,6 +216,7 @@ pub enum DevToolsCommand {
|
||||
GetRealms(DevToolsGetRealmsCommand),
|
||||
EvaluateScript(DevToolsEvaluateScriptCommand),
|
||||
CallFunction(DevToolsCallFunctionCommand),
|
||||
TerminateExecution(DevToolsTerminateExecutionCommand),
|
||||
ReleaseObjects(DevToolsReleaseObjectsCommand),
|
||||
CreateTarget(DevToolsCreateTargetCommand),
|
||||
CloseTarget(DevToolsCloseTargetCommand),
|
||||
@@ -301,6 +302,7 @@ impl DevToolsCommand {
|
||||
DevToolsCommand::GetRealms(command) => &command.context,
|
||||
DevToolsCommand::EvaluateScript(command) => &command.context,
|
||||
DevToolsCommand::CallFunction(command) => &command.context,
|
||||
DevToolsCommand::TerminateExecution(command) => &command.context,
|
||||
DevToolsCommand::ReleaseObjects(command) => &command.context,
|
||||
DevToolsCommand::CreateTarget(command) => &command.context,
|
||||
DevToolsCommand::CloseTarget(command) => &command.context,
|
||||
@@ -468,6 +470,11 @@ pub struct DevToolsCallFunctionCommand {
|
||||
pub materialize_bidi_script_result: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct DevToolsTerminateExecutionCommand {
|
||||
pub context: DevToolsCommandContext,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct DevToolsSerializationOptions {
|
||||
pub max_object_depth: Option<u64>,
|
||||
|
||||
@@ -121,12 +121,19 @@ struct DevToolsRuntimeTarget {
|
||||
struct DevToolsRuntimeCommandDispatchState {
|
||||
internal_command_id: u64,
|
||||
command_context: DevToolsCommandContext,
|
||||
result_kind: DevToolsRuntimeCommandResultKind,
|
||||
result_ownership: DevToolsResultOwnership,
|
||||
serialization_options: Option<DevToolsSerializationOptions>,
|
||||
target: DevToolsRuntimeTarget,
|
||||
target_realm: Option<DevToolsRealmId>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
enum DevToolsRuntimeCommandResultKind {
|
||||
Script,
|
||||
Empty,
|
||||
}
|
||||
|
||||
pub struct PendingDevToolsRuntimeCommandDispatch {
|
||||
state: DevToolsRuntimeCommandDispatchState,
|
||||
pending: PendingRuntimeCommandDispatch,
|
||||
@@ -2143,13 +2150,22 @@ pub(crate) async fn execute_devtools_runtime_command_async_with_protocol_events(
|
||||
if let DevToolsCommand::LocateNodes(command) = command {
|
||||
return execute_devtools_locate_nodes_command_async(conn, command).await;
|
||||
}
|
||||
let result_kind = devtools_runtime_command_result_kind(&command);
|
||||
let result_ownership = devtools_runtime_result_ownership(&command);
|
||||
let serialization_options = devtools_runtime_serialization_options(&command);
|
||||
let target = match devtools_runtime_target_async(conn, &command).await {
|
||||
Ok(target) => target,
|
||||
Err(error) => return DevToolsCommandExecutionOutput::new(Err(error)),
|
||||
};
|
||||
let target_realm = devtools_realm_id_for_runtime_target_async(conn, &target).await;
|
||||
// Control commands must reach their Inspector route without first asking
|
||||
// the Page owner for realm inventory. That owner can be the JavaScript
|
||||
// execution this command exists to interrupt.
|
||||
let target_realm = match result_kind {
|
||||
DevToolsRuntimeCommandResultKind::Script => {
|
||||
devtools_realm_id_for_runtime_target_async(conn, &target).await
|
||||
}
|
||||
DevToolsRuntimeCommandResultKind::Empty => None,
|
||||
};
|
||||
let mut route_scope = conn.scoped_none_session_owner_route_override(target.route.clone());
|
||||
if let DevToolsCommand::CallFunction(call_function) = &mut command
|
||||
&& matches!(
|
||||
@@ -2198,6 +2214,13 @@ pub(crate) async fn execute_devtools_runtime_command_async_with_protocol_events(
|
||||
renderer_output_predecessor,
|
||||
);
|
||||
};
|
||||
if result_kind == DevToolsRuntimeCommandResultKind::Empty {
|
||||
return DevToolsCommandExecutionOutput::from_parts(
|
||||
devtools_empty_result_from_response(response),
|
||||
protocol_events,
|
||||
renderer_output_predecessor,
|
||||
);
|
||||
}
|
||||
let mut result = match devtools_script_result_from_response(
|
||||
response,
|
||||
result_ownership,
|
||||
@@ -2316,6 +2339,7 @@ impl CdpConnection {
|
||||
.await;
|
||||
}
|
||||
|
||||
let result_kind = devtools_runtime_command_result_kind(&command);
|
||||
let result_ownership = devtools_runtime_result_ownership(&command);
|
||||
let serialization_options = devtools_runtime_serialization_options(&command);
|
||||
let target = match devtools_runtime_target_async(self, &command).await {
|
||||
@@ -2331,7 +2355,15 @@ impl CdpConnection {
|
||||
.await;
|
||||
}
|
||||
};
|
||||
let target_realm = devtools_realm_id_for_runtime_target_async(self, &target).await;
|
||||
// Keep the interrupt path free of Page-owner realm lookups. In
|
||||
// particular, Runtime.terminateExecution must be able to enter its IO
|
||||
// envelope while a MainThread script is not yielding.
|
||||
let target_realm = match result_kind {
|
||||
DevToolsRuntimeCommandResultKind::Script => {
|
||||
devtools_realm_id_for_runtime_target_async(self, &target).await
|
||||
}
|
||||
DevToolsRuntimeCommandResultKind::Empty => None,
|
||||
};
|
||||
let mut route_scope = self.scoped_none_session_owner_route_override(target.route.clone());
|
||||
if let DevToolsCommand::CallFunction(call_function) = &mut command
|
||||
&& matches!(
|
||||
@@ -2377,6 +2409,7 @@ impl CdpConnection {
|
||||
let state = DevToolsRuntimeCommandDispatchState {
|
||||
internal_command_id,
|
||||
command_context,
|
||||
result_kind,
|
||||
result_ownership,
|
||||
serialization_options,
|
||||
target: target.clone(),
|
||||
@@ -2456,6 +2489,16 @@ impl CdpConnection {
|
||||
)
|
||||
.await;
|
||||
};
|
||||
if state.result_kind == DevToolsRuntimeCommandResultKind::Empty {
|
||||
return self
|
||||
.complete_devtools_runtime_direct_result(
|
||||
state.command_context,
|
||||
devtools_empty_result_from_response(response),
|
||||
protocol_events,
|
||||
renderer_output_predecessor,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
let mut result = match devtools_script_result_from_response(
|
||||
response,
|
||||
state.result_ownership,
|
||||
@@ -3493,6 +3536,15 @@ fn devtools_runtime_result_ownership(command: &DevToolsCommand) -> DevToolsResul
|
||||
}
|
||||
}
|
||||
|
||||
fn devtools_runtime_command_result_kind(
|
||||
command: &DevToolsCommand,
|
||||
) -> DevToolsRuntimeCommandResultKind {
|
||||
match command {
|
||||
DevToolsCommand::TerminateExecution(_) => DevToolsRuntimeCommandResultKind::Empty,
|
||||
_ => DevToolsRuntimeCommandResultKind::Script,
|
||||
}
|
||||
}
|
||||
|
||||
fn devtools_runtime_serialization_options(
|
||||
command: &DevToolsCommand,
|
||||
) -> Option<DevToolsSerializationOptions> {
|
||||
@@ -3547,6 +3599,13 @@ async fn devtools_runtime_target_async(
|
||||
conn: &mut CdpConnection,
|
||||
command: &DevToolsCommand,
|
||||
) -> Result<DevToolsRuntimeTarget, DevToolsError> {
|
||||
if let DevToolsCommand::TerminateExecution(command) = command {
|
||||
let target_id =
|
||||
command.context.target_id.as_ref().ok_or_else(|| {
|
||||
DevToolsError::new(DevToolsErrorKind::NoSuchTarget, "NoSuchTarget")
|
||||
})?;
|
||||
return devtools_runtime_control_target(conn, target_id);
|
||||
}
|
||||
let (target_id, realm_id, world_name) = match command {
|
||||
DevToolsCommand::EvaluateScript(command) => (
|
||||
command.context.target_id.as_ref(),
|
||||
@@ -3578,6 +3637,23 @@ async fn devtools_runtime_target_async(
|
||||
))
|
||||
}
|
||||
|
||||
fn devtools_runtime_control_target(
|
||||
conn: &CdpConnection,
|
||||
target_id: &DevToolsTargetId,
|
||||
) -> Result<DevToolsRuntimeTarget, DevToolsError> {
|
||||
// Do not fall back to realm discovery here: it is a MainThread operation
|
||||
// and would put the escape hatch behind the work it needs to interrupt.
|
||||
let route = conn
|
||||
.target_session_route_for_target_id(target_id.as_str())
|
||||
.or_else(|| conn.target_session_route_for_child_frame_id(target_id.as_str()))
|
||||
.ok_or_else(|| DevToolsError::new(DevToolsErrorKind::NoSuchTarget, "NoSuchTarget"))?;
|
||||
Ok(DevToolsRuntimeTarget {
|
||||
route,
|
||||
execution_context_id: None,
|
||||
window_context_id: Some(target_id.clone()),
|
||||
})
|
||||
}
|
||||
|
||||
async fn devtools_runtime_context_target_async(
|
||||
conn: &mut CdpConnection,
|
||||
target_id: &DevToolsTargetId,
|
||||
@@ -4574,6 +4650,30 @@ async fn start_protocol_neutral_runtime_command(
|
||||
)),
|
||||
}
|
||||
}
|
||||
DevToolsCommand::TerminateExecution(_) => {
|
||||
let json = runtime_inspector_command_json(
|
||||
internal_command_id,
|
||||
"Runtime.terminateExecution",
|
||||
&json!({}),
|
||||
);
|
||||
let parsed = match parse_synthesized_runtime_command(json) {
|
||||
Ok(command) => command,
|
||||
Err(message) => {
|
||||
return RuntimeCommandTaskStep::Complete(runtime_inspector_error_plan(
|
||||
Some(internal_command_id),
|
||||
message,
|
||||
));
|
||||
}
|
||||
};
|
||||
let cmd = Cmd::from_parsed(&parsed)
|
||||
.expect("synthesized Runtime command must contain a domain separator");
|
||||
let MainRuntimeCommand::Inspector(command) =
|
||||
MainRuntimeCommand::classify(RuntimeAction::TerminateExecution)
|
||||
else {
|
||||
unreachable!("Runtime.terminateExecution must use an Inspector command route")
|
||||
};
|
||||
start_main_runtime_inspector_command(route_scope.conn_mut(), &cmd, command)
|
||||
}
|
||||
_ => RuntimeCommandTaskStep::Complete(CommandOutputPlan::from_devtools_error(
|
||||
DevToolsError::new(DevToolsErrorKind::Unsupported, "UnsupportedDevToolsCommand"),
|
||||
)),
|
||||
@@ -5516,6 +5616,15 @@ fn devtools_script_result_from_response(
|
||||
)))
|
||||
}
|
||||
|
||||
fn devtools_empty_result_from_response(
|
||||
response: Value,
|
||||
) -> Result<DevToolsCommandResult, DevToolsError> {
|
||||
if let Some(error) = response.get("error") {
|
||||
return Err(devtools_error_from_cdp_error_value(error));
|
||||
}
|
||||
Ok(DevToolsCommandResult::Empty)
|
||||
}
|
||||
|
||||
fn validate_protocol_neutral_runtime_handle_realms(
|
||||
conn: &CdpConnection,
|
||||
command: &DevToolsCommand,
|
||||
|
||||
Reference in New Issue
Block a user