diff --git a/moli-cdp-smoke/README.md b/moli-cdp-smoke/README.md index 65f076798..afaafbc43 100644 --- a/moli-cdp-smoke/README.md +++ b/moli-cdp-smoke/README.md @@ -148,15 +148,16 @@ The current suite is a strong core smoke gate, not a complete Playwright compati Covered well: - The default raw `debugger-breakpoints`, `runtime-exception`, and - `file-chooser` groups preserve six raw-CDP contracts covering seven Lexbench - task regressions at the public process boundary, and add two multi-attachment - Runtime exception contracts. They dispatch `Debugger.getPossibleBreakpoints`, + `file-chooser` groups preserve focused Lexbench regressions at the public + process boundary, including multi-attachment Runtime exception contracts. + They dispatch `Debugger.getPossibleBreakpoints`, `setBreakpoint`, `removeBreakpoint`, and `setBreakpointByUrl` while the Page is normally running, require an uncaught timer error to publish `Runtime.exceptionThrown` without a follow-up command, verify that each Runtime-enabled attachment receives the target-owned exception while a - disabled peer does not, and require a user-gesture file-input activation to - publish the session-scoped `Page.fileChooserOpened` event. + disabled peer does not, keep `Runtime.enable` from making Error stack cost + track JavaScript stack depth, and require a user-gesture file-input activation + to publish the session-scoped `Page.fileChooserOpened` event. - The default raw `url-policy` group holds the hosted local-file boundary at the public process edge. It requires an exact session-routed `Page.navigate` `-32000` error with no lifecycle or document replacement, verifies page diff --git a/moli-cdp-smoke/moli_cdp_smoke/groups/protocol_regressions.py b/moli-cdp-smoke/moli_cdp_smoke/groups/protocol_regressions.py index d0db66a31..8fc7e054a 100644 --- a/moli-cdp-smoke/moli_cdp_smoke/groups/protocol_regressions.py +++ b/moli-cdp-smoke/moli_cdp_smoke/groups/protocol_regressions.py @@ -151,6 +151,8 @@ async def run_runtime_exception_group( # asynchronous exception. enable_id = await client.send("Runtime.enable", session_id=session_id) await client.recv_until_id(enable_id, timeout=5) + stack_cost = await _runtime_enable_stack_cost_probe(client, session_id) + record(results, "raw_cdp_runtime_enable_stack_cost", stack_cost) marker = "moli-smoke-async-exception" exceptions = await _schedule_async_exception_and_collect( client, @@ -209,6 +211,90 @@ async def run_runtime_exception_group( await _close_client(client) +async def _runtime_enable_stack_cost_probe( + client: RawCdpClient, + session_id: str, +) -> dict[str, Any]: + evaluate_id = await client.send( + "Runtime.evaluate", + { + "expression": r""" +(() => { + const measureOnce = () => { + let sink = 0; + const sample = depth => { + if (depth > 0) return sample(depth - 1); + const started = performance.now(); + for (let index = 0; index < 3; index += 1) { + try { + throw new Error('runtime-stack-cost-probe'); + } catch (error) { + sink += error.stack.length; + } + } + return (performance.now() - started) / 3; + }; + + for (let index = 0; index < 50; index += 1) sample(50); + const depths = []; + const costs = []; + for (let depth = 10; depth <= 200; depth += 10) { + let cost = 0; + for (let repeat = 0; repeat < 4; repeat += 1) cost += sample(depth); + depths.push(depth); + costs.push(cost / 4); + } + + const mean = values => + values.reduce((total, value) => total + value, 0) / values.length; + const meanDepth = mean(depths); + const meanCost = mean(costs); + let covariance = 0; + let depthVariance = 0; + let costVariance = 0; + for (let index = 0; index < depths.length; index += 1) { + const depthDelta = depths[index] - meanDepth; + const costDelta = costs[index] - meanCost; + covariance += depthDelta * costDelta; + depthVariance += depthDelta * depthDelta; + costVariance += costDelta * costDelta; + } + const slope = depthVariance === 0 ? 0 : covariance / depthVariance; + const rSquared = depthVariance === 0 || costVariance === 0 + ? 0 + : (covariance * covariance) / (depthVariance * costVariance); + return {rSquared, slope, sink}; + }; + return [measureOnce(), measureOnce(), measureOnce()]; +})() +""", + "returnByValue": True, + }, + session_id=session_id, + ) + response, _messages = await client.recv_until_id(evaluate_id, timeout=5) + if "error" in response: + raise SmokeError(f"Runtime stack-cost probe failed: {response}") + probes = response.get("result", {}).get("result", {}).get("value") + if not isinstance(probes, list) or len(probes) != 3: + raise SmokeError(f"Runtime stack-cost probe returned invalid samples: {response}") + try: + ordered_r_squared = sorted(float(probe["rSquared"]) for probe in probes) + except (KeyError, TypeError, ValueError) as error: + raise SmokeError( + f"Runtime stack-cost probe returned invalid regression data: {probes}" + ) from error + if ordered_r_squared[1] >= 0.5: + raise SmokeError( + "Runtime.enable exposed a repeatable Error stack-depth timing slope: " + f"{probes}" + ) + return { + "probes": probes, + "medianRSquared": ordered_r_squared[1], + } + + async def run_file_chooser_group( endpoint: str, fixture: str, diff --git a/moli-cdp-smoke/moli_cdp_smoke/runner.py b/moli-cdp-smoke/moli_cdp_smoke/runner.py index ffdca0463..42fdebd1a 100644 --- a/moli-cdp-smoke/moli_cdp_smoke/runner.py +++ b/moli-cdp-smoke/moli_cdp_smoke/runner.py @@ -111,7 +111,7 @@ async def _await_group(group: SmokeGroup, awaitable: Awaitable[None]) -> None: ), SmokeGroup( "runtime-exception", - "Raw asynchronous Runtime.exceptionThrown delivery without a follow-up command.", + "Raw Runtime.enable stack-cost privacy and asynchronous Runtime.exceptionThrown delivery.", "raw", run_runtime_exception_group, ), diff --git a/moli-renderer-v8/src/inspector_session.rs b/moli-renderer-v8/src/inspector_session.rs new file mode 100644 index 000000000..01db1d5c7 --- /dev/null +++ b/moli-renderer-v8/src/inspector_session.rs @@ -0,0 +1,80 @@ +//! Synchronous Inspector session commands shared by Page and Worker owners. +//! +//! V8 enables Runtime with a 200-frame exception capture limit. Use a ten-frame +//! default to bound the extra cost of constructing Error objects, while leaving +//! explicit frontend limits and restored session settings under V8's control. + +use serde::Deserialize; +use serde_json::{Value, json}; + +const STACK_CAPTURE_CALL_ID: i32 = -1; +const DEFAULT_RUNTIME_STACK_CAPTURE_DEPTH: i32 = 10; + +#[cfg(test)] +pub(crate) mod tests; + +/// Adapts the owner's response routing without changing its notification stream. +pub(crate) trait InspectorSessionOutput { + /// Only for synchronous, non-reentrant Inspector settings. Capture this + /// dispatch's response before frontend routing, even if a frontend uses the + /// same ID. Preserve queued responses, callbacks, and notifications. + fn capture_internal_response(&self, call_id: i32, dispatch: impl FnOnce()) -> Option; +} + +/// The caller establishes its usual Inspector isolate/microtask scope. +pub(crate) fn dispatch_with_runtime_defaults( + session: &v8::inspector::V8InspectorSession, + raw_json: &str, + output: &impl InspectorSessionOutput, +) -> Result<(), String> { + let enabling_runtime = serde_json::from_str::(raw_json).is_ok_and(|message| { + message.get("method").and_then(Value::as_str) == Some("Runtime.enable") + }); + let runtime_was_disabled = enabling_runtime && !runtime_enabled(session)?; + session.dispatch_protocol_message(v8::inspector::StringView::from(raw_json.as_bytes())); + + // V8 owns enable/restore state. Repeated or failed enables must not reset + // explicit frontend limits. The setter runs synchronously without JS or + // microtasks, so a scoped response capture can safely reuse a fixed ID. + if runtime_was_disabled && runtime_enabled(session)? { + let request = json!({ + "id": STACK_CAPTURE_CALL_ID, + "method": "Runtime.setMaxCallStackSizeToCapture", + "params": {"size": DEFAULT_RUNTIME_STACK_CAPTURE_DEPTH} + }) + .to_string(); + let response = output + .capture_internal_response(STACK_CAPTURE_CALL_ID, || { + session + .dispatch_protocol_message(v8::inspector::StringView::from(request.as_bytes())); + }) + .ok_or("Runtime stack-capture default produced no Inspector response")?; + if response != json!({"id": STACK_CAPTURE_CALL_ID, "result": {}}) { + return Err(format!( + "Runtime stack-capture default returned an unexpected response: {response}" + )); + } + } + Ok(()) +} + +fn runtime_enabled(session: &v8::inspector::V8InspectorSession) -> Result { + // V8 owns this serialized state and uses these same fields on reconnect. + // Decode it only for Runtime.enable, never on the Runtime.evaluate hot path. + #[derive(Deserialize)] + struct SessionState { + #[serde(rename = "Runtime")] + runtime: RuntimeState, + } + #[derive(Deserialize)] + struct RuntimeState { + #[serde(default, rename = "runtimeEnabled")] + enabled: bool, + } + + let state = v8::crdtp::cbor_to_json(&session.state()) + .ok_or("Inspector session state is not valid CBOR")?; + let state: SessionState = serde_json::from_slice(&state) + .map_err(|error| format!("invalid Inspector Runtime state: {error}"))?; + Ok(state.runtime.enabled) +} diff --git a/moli-renderer-v8/src/inspector_session/tests.rs b/moli-renderer-v8/src/inspector_session/tests.rs new file mode 100644 index 000000000..ca90bee38 --- /dev/null +++ b/moli-renderer-v8/src/inspector_session/tests.rs @@ -0,0 +1,103 @@ +use serde_json::{Value, json}; + +fn stack_probe() -> String { + json!({ + "id": 100, + "method": "Runtime.evaluate", + "params": { + "expression": "(function recurse(n) { if (n) return recurse(n - 1); throw new Error('stack depth'); })(80)" + } + }) + .to_string() +} + +fn assert_frontend_response(messages: &[Value], request: &Value) { + let responses = messages + .iter() + .filter(|message| message.get("id").is_some()) + .collect::>(); + assert_eq!( + responses, + vec![&json!({"id": request["id"], "result": {}})], + "only the frontend response may escape, including when its ID is negative: {request}" + ); +} + +fn assert_stack_depth(messages: &[Value], expected: usize, request: &Value) { + let response = messages + .iter() + .find(|message| message["id"] == json!(100)) + .expect("stack probe response"); + let exception = &response["result"]["exceptionDetails"]; + assert!(exception.is_object(), "probe must throw: {response}"); + let depth = exception["stackTrace"]["callFrames"] + .as_array() + .map_or(0, Vec::len); + assert_eq!( + depth, expected, + "Inspector exception stack depth after {request}: {response}" + ); +} + +pub(crate) fn assert_session_lifecycle(mut dispatch: impl FnMut(&str) -> Vec) { + for request in [ + json!({"id": 9, "method": "Runtime.enable", "params": "invalid"}), + json!({"id": 9, "method": "Runtime.setMaxCallStackSizeToCapture", "params": {"size": 50}}), + ] { + let messages = dispatch(&request.to_string()); + assert!( + messages + .iter() + .any(|message| message["id"] == json!(9) && message.get("error").is_some()), + "failed requests must retain their protocol error and must not enable Runtime: {messages:?}" + ); + } + let probe = stack_probe(); + for (request, expected_depth) in [ + (json!({"id": -1, "method": "Runtime.enable"}), Some(10)), + ( + json!({"id": 2, "method": "Runtime.setMaxCallStackSizeToCapture", "params": {"size": 50}}), + Some(50), + ), + (json!({"id": -1, "method": "Runtime.enable"}), Some(50)), + ( + json!({"id": 3, "method": "Runtime.setMaxCallStackSizeToCapture", "params": {"size": 0}}), + Some(0), + ), + (json!({"id": -1, "method": "Runtime.enable"}), Some(0)), + (json!({"id": 4, "method": "Runtime.disable"}), None), + (json!({"id": -1, "method": "Runtime.enable"}), Some(10)), + ] { + assert_frontend_response(&dispatch(&request.to_string()), &request); + if let Some(expected_depth) = expected_depth { + assert_stack_depth(&dispatch(&probe), expected_depth, &request); + } + } +} + +pub(crate) fn assert_multiple_sessions(mut dispatch: impl FnMut(&str, &str) -> Vec) { + let probe = stack_probe(); + for (session, request, expected_depth) in [ + ("A", json!({"id": -1, "method": "Runtime.enable"}), Some(10)), + ( + "A", + json!({"id": 2, "method": "Runtime.setMaxCallStackSizeToCapture", "params": {"size": 50}}), + Some(50), + ), + ("B", json!({"id": -1, "method": "Runtime.enable"}), Some(50)), + ("A", json!({"id": -1, "method": "Runtime.enable"}), Some(50)), + ("A", json!({"id": 3, "method": "Runtime.disable"}), Some(10)), + ( + "B", + json!({"id": 4, "method": "Runtime.setMaxCallStackSizeToCapture", "params": {"size": 0}}), + Some(0), + ), + ("B", json!({"id": -1, "method": "Runtime.enable"}), Some(0)), + ("B", json!({"id": 5, "method": "Runtime.disable"}), None), + ] { + assert_frontend_response(&dispatch(session, &request.to_string()), &request); + if let Some(expected_depth) = expected_depth { + assert_stack_depth(&dispatch(session, &probe), expected_depth, &request); + } + } +} diff --git a/moli-renderer-v8/src/lib.rs b/moli-renderer-v8/src/lib.rs index 591029f76..a641ad333 100644 --- a/moli-renderer-v8/src/lib.rs +++ b/moli-renderer-v8/src/lib.rs @@ -57,6 +57,7 @@ mod frame_owner_model; mod host; mod host_bindings; mod inspector_microtasks; +mod inspector_session; mod javascript_url; mod layout_renderer; mod link_as; diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/lifecycle.rs b/moli-renderer-v8/src/runtime/page_vm/tests/lifecycle.rs index f4800607e..50b171827 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/lifecycle.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/lifecycle.rs @@ -10,7 +10,7 @@ use crate::{ RendererDocumentLifecycleEventKind, RendererDocumentLifecycleIdentity, RendererDocumentLifecycleMilestone, RendererDocumentLifecycleWaitOutcome, RendererDocumentTerminationReason, RendererLifecycleStartReason, RendererPageReply, - RendererPageState, + RendererPageState, RendererRuntimeInspectorMessage, page_task_queue::PostParseLifecycleWork, runtime::document_lifecycle_turn::DocumentLifecycleNavigationTiming, script_vm::{ @@ -20,6 +20,7 @@ use crate::{ MainDocumentLifecycleTargetRejection, }, }; +use serde_json::Value; use std::sync::Arc; #[test] @@ -3654,6 +3655,69 @@ fn runtime_evaluate_without_enable_uses_inspector_default_context() { ); } +#[test] +fn runtime_stack_capture_follows_session_lifecycle() { + let mut page_vm = test_page_vm(); + crate::inspector_session::tests::assert_session_lifecycle(|request| { + page_vm + .vm_mut() + .dispatch_inspector_protocol_message(request) + .expect("Page Inspector dispatch") + }); +} + +#[test] +fn runtime_stack_capture_preserves_independent_session_limits() { + let mut page_vm = test_page_vm(); + crate::inspector_session::tests::assert_multiple_sessions(|session, request| { + page_vm + .vm_mut() + .dispatch_inspector_protocol_message_for_session(Some(session), request) + .expect("Page Inspector session dispatch") + .into_iter() + .map(RendererRuntimeInspectorMessage::into_v8_inspector_message) + .collect() + }); +} + +#[test] +fn runtime_enable_hides_the_internal_stack_capture_limit_response() { + let mut page_vm = test_page_vm(); + + let messages = page_vm + .vm_mut() + .dispatch_inspector_protocol_message(r#"{"id":41,"method":"Runtime.enable"}"#) + .expect("Runtime.enable dispatch"); + assert!( + messages + .iter() + .any(|message| message["id"] == json!(41) && message["result"] == json!({})), + "Runtime.enable should retain its frontend response: {messages:#?}" + ); + assert!( + messages.iter().all(|message| { + message + .get("id") + .and_then(Value::as_i64) + .is_none_or(|call_id| call_id >= 0) + }), + "the private stack-capture command response must not enter Page CDP output: {messages:#?}" + ); + + let messages = page_vm + .vm_mut() + .dispatch_inspector_protocol_message( + r#"{"id":42,"method":"Runtime.setMaxCallStackSizeToCapture","params":{"size":8}}"#, + ) + .expect("frontend Page stack-capture command"); + assert!( + messages + .iter() + .any(|message| message["id"] == json!(42) && message["result"] == json!({})), + "the private default must not prevent a frontend override: {messages:#?}" + ); +} + #[test] fn page_diagnostics_snapshot_uses_rust_runtime_observable_console_source_queue() { let mut page_vm = test_page_vm(); diff --git a/moli-renderer-v8/src/script_vm.rs b/moli-renderer-v8/src/script_vm.rs index d5cdc2fb2..dc139c2a7 100644 --- a/moli-renderer-v8/src/script_vm.rs +++ b/moli-renderer-v8/src/script_vm.rs @@ -18,6 +18,7 @@ use crate::{ FrameRealmId, FrameScriptJob, FrameScriptJobKind, }, inspector_microtasks::with_scoped_inspector_microtasks, + inspector_session::dispatch_with_runtime_defaults, network::{ResourceRequestClient, context::DocumentResourceLoader}, page_task_queue::{ PageRuntimeWakeSender, PageTask, PageTaskSender, PostParseLifecycleWork, @@ -1535,10 +1536,9 @@ impl ScriptVm { let _dispatch_response_capture = outbound.capture_dispatch_responses(); with_scoped_inspector_microtasks(scope, || { - session.dispatch_protocol_message( - v8::inspector::StringView::from(raw_json.as_bytes()), - ); - }); + dispatch_with_runtime_defaults(session, raw_json, &outbound) + }) + .map_err(anyhow::Error::msg)?; } let response = outbound .take_response_for_call_id_after( @@ -1762,11 +1762,12 @@ impl ScriptVm { // callback even if they settle in this same owner turn. let dispatch_response_capture = outbound.capture_dispatch_responses(); let dispatch_started = timing_started.map(|_| Instant::now()); - with_scoped_inspector_microtasks(scope, || { - session.dispatch_protocol_message(v8::inspector::StringView::from( - raw_json.as_bytes(), - )); - }); + if let Err(error) = with_scoped_inspector_microtasks(scope, || { + dispatch_with_runtime_defaults(session, raw_json, &outbound) + }) { + outbound.discard_messages_after(snap); + return Err(anyhow::Error::msg(error)); + } if let (Some(total_started), Some(started)) = (timing_started, dispatch_started) { diff --git a/moli-renderer-v8/src/script_vm/inspector/agent_sessions.rs b/moli-renderer-v8/src/script_vm/inspector/agent_sessions.rs index 46c0435d6..b8f966139 100644 --- a/moli-renderer-v8/src/script_vm/inspector/agent_sessions.rs +++ b/moli-renderer-v8/src/script_vm/inspector/agent_sessions.rs @@ -410,6 +410,7 @@ pub(super) fn inspector_session_key(inspector_session_id: Option<&str>) -> DevTo #[cfg(test)] mod tests { use super::*; + use crate::inspector_session::dispatch_with_runtime_defaults; use crate::script_vm::inspector::context_registry::DocumentInspectorContextRegistrationId; use serde_json::Value; use std::pin::pin; @@ -423,9 +424,8 @@ mod tests { fn dispatch(session: &RendererDevToolsSession, request: &str) -> Vec { let snapshot = session.outbound.len(); let _response_capture = session.outbound.capture_dispatch_responses(); - session - .session - .dispatch_protocol_message(v8::inspector::StringView::from(request.as_bytes())); + dispatch_with_runtime_defaults(&session.session, request, &session.outbound) + .expect("Inspector session dispatch"); session.outbound.take_messages_after(snapshot) } @@ -587,6 +587,14 @@ mod tests { 4, ); + assert_successful_response( + &dispatch( + &first_attach, + r#"{"id":6,"method":"Runtime.setMaxCallStackSizeToCapture","params":{"size":50}}"#, + ), + 6, + ); + let state = first_attach.v8_state(); assert!( !state.is_empty(), @@ -658,6 +666,15 @@ mod tests { "V8 should apply the restored custom formatter setting: {formatter_response:?}" ); let restored_state = restored.v8_state(); + assert_successful_response( + &dispatch(&restored, r#"{"id":7,"method":"Runtime.enable"}"#), + 7, + ); + assert_eq!( + state_json(&restored.v8_state()), + state_json(&restored_state), + "repeated Runtime.enable must preserve the restored stack-capture override" + ); assert_eq!( state_json(&restored_state), initial_state_json, diff --git a/moli-renderer-v8/src/script_vm/inspector/outbound.rs b/moli-renderer-v8/src/script_vm/inspector/outbound.rs index 28cef9383..97a519c78 100644 --- a/moli-renderer-v8/src/script_vm/inspector/outbound.rs +++ b/moli-renderer-v8/src/script_vm/inspector/outbound.rs @@ -2,6 +2,7 @@ use crate::devtools::pause::{ RendererInspectorPauseNotificationRoute, RendererInspectorPausePrefaceGuard, RendererInspectorSessionOutboundRoute, }; +use crate::inspector_session::InspectorSessionOutput; use crate::runtime::{ PendingRendererOutputRecord, RendererCommandTurnOutputRecorder, RendererDevToolsSessionOutputHost, RendererProtocolObservation, @@ -76,6 +77,18 @@ impl Default for InspectorOutbound { } } +impl InspectorSessionOutput for InspectorOutbound { + fn capture_internal_response(&self, call_id: i32, dispatch: impl FnOnce()) -> Option { + let snapshot = self.len(); + { + let _internal_capture = self.capture_internal_dispatch_response(call_id); + let _dispatch_capture = self.capture_dispatch_responses(); + dispatch(); + } + self.take_response_for_call_id_after(snapshot, i64::from(call_id)) + } +} + impl InspectorOutbound { pub(super) fn for_agent(agent_token: RendererDevToolsAgentToken) -> Self { Self { @@ -337,9 +350,6 @@ impl InspectorOutbound { } pub(super) fn push_response_value(&self, call_id: i32, value: Value) { - if let Some(session_route) = self.session_route.borrow().as_ref() { - session_route.mark_command_response(call_id, value.get("error").is_none()); - } let mut guard = self.response_routing.borrow_mut(); if guard .internal_dispatch_response_call_ids @@ -350,6 +360,9 @@ impl InspectorOutbound { self.push_local_value(value); return; } + if let Some(session_route) = self.session_route.borrow().as_ref() { + session_route.mark_command_response(call_id, value.get("error").is_none()); + } let callback = guard.pending_response_callbacks.remove(&call_id); if let Some(callback) = callback { let recorder = (guard.runtime_command_output_suppression_depth == 0) @@ -548,6 +561,8 @@ impl InspectorOutbound { .map(|message| message.value) } + // DOMDebugger's internal Runtime.callFunctionOn can execute JS, unlike the + // synchronous stack-capture setter, so it still needs an unoccupied ID. pub(in crate::script_vm) fn internal_dispatch_call_id_is_available( &self, call_id: i32, @@ -751,14 +766,21 @@ impl Drop for InspectorInternalDispatchResponseCapture { #[cfg(test)] mod tests { use super::*; - use crate::runtime::{PageId, RendererOutputItem, RendererOutputStreamIdentity}; - - #[test] - fn instrumentation_pause_prefix_publishes_context_created_with_bound_origin() { + use crate::devtools::pause::RendererInspectorPauseBridge; + use crate::runtime::{ + PageId, RendererInspectorCommandRoute, RendererInspectorIngressTicket, + RendererInspectorPauseCommandEffect, RendererOutputItem, RendererOutputStreamIdentity, + }; + + fn routed_outbound() -> ( + InspectorOutbound, + RendererInspectorPauseBridge, + RendererTurnOutputJournal, + ) { let journal = RendererTurnOutputJournal::new( RendererOutputStreamIdentity::new_page_for_protocol_test(PageId::new_for_testing(41)), ); - let pause_bridge = crate::devtools::pause::RendererInspectorPauseBridge::default(); + let pause_bridge = RendererInspectorPauseBridge::default(); pause_bridge.configure_page_route(journal.clone()); let agent_token = RendererDevToolsAgentToken::allocate(); let session = DevToolsSessionKey::Primary; @@ -781,9 +803,63 @@ mod tests { .outbound_route(agent_token, session.clone()), Some(journal.clone()), ); + (outbound, pause_bridge, journal) + } + + #[test] + fn internal_response_does_not_complete_a_same_id_frontend_pause_command() { + let (outbound, bridge, _) = routed_outbound(); + let route = outbound + .session_route + .borrow() + .clone() + .expect("frontend route"); + route.route_notification(&serde_json::json!({ + "method": "Debugger.paused", "params": {"callFrames": []} + })); + assert!(bridge.enter_pause().is_some()); + let dispatch = bridge.begin_command_dispatch( + 1, + &RendererInspectorIngressTicket::new(None, None, RendererInspectorCommandRoute::Io), + RendererInspectorPauseCommandEffect::Resume, + Some(-1), + ); + let recorder = RendererCommandTurnOutputRecorder::default(); + outbound + .begin_command_turn_output(DevToolsSessionKey::Primary, recorder.clone()) + .expect("command output scope"); + let response = serde_json::json!({"id": -1, "result": {}}); + assert_eq!( + outbound.capture_internal_response(-1, || { + outbound.push_response_value(-1, response.clone()); + }), + Some(response) + ); + outbound.end_command_turn_output(&recorder); + assert!(recorder.finish().is_empty()); + assert!(outbound.take_pending_messages().is_empty()); + drop(dispatch); + bridge.leave_pause(); + assert!( + matches!( + route.route_notification( + &serde_json::json!({"method": "Debugger.resumed", "params": {}}) + ), + RendererInspectorPauseNotificationRoute::PublishImmediately { + command_output: None, + .. + } + ), + "the internal response must not mark the frontend resume command successful" + ); + } + + #[test] + fn instrumentation_pause_prefix_publishes_context_created_with_bound_origin() { + let (outbound, pause_bridge, journal) = routed_outbound(); let recorder = RendererCommandTurnOutputRecorder::default(); outbound - .begin_command_turn_output(session, recorder) + .begin_command_turn_output(DevToolsSessionKey::Primary, recorder) .expect("command output scope"); outbound.push_value(serde_json::json!({ diff --git a/moli-renderer-v8/src/script_vm/inspector/tests.rs b/moli-renderer-v8/src/script_vm/inspector/tests.rs index 3bba50d23..aff847ffd 100644 --- a/moli-renderer-v8/src/script_vm/inspector/tests.rs +++ b/moli-renderer-v8/src/script_vm/inspector/tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::inspector_session::InspectorSessionOutput; #[test] fn inspector_outbound_queue_reports_its_exact_length_until_flushed() { @@ -325,6 +326,31 @@ fn internal_dispatch_call_id_avoids_queued_and_active_frontend_ids() { assert!(outbound.internal_dispatch_call_id_is_available(-2)); } +#[test] +fn internal_response_capture_preserves_queued_responses_and_canceled_callbacks() { + let outbound = InspectorOutbound::default(); + let queued = json!({"id": -1, "result": "queued"}); + let notification = json!({"method": "Runtime.consoleAPICalled", "params": {}}); + outbound.push_value(queued.clone()); + let (tx, _rx) = tokio::sync::oneshot::channel(); + outbound.register_response_callback(RendererRuntimeInspectorResponseSender::new(-1, tx)); + outbound.cancel_response_callback(-1); + + let response = outbound.capture_internal_response(-1, || { + outbound.push_response_value(-1, json!({"id": -1, "result": "outer"})); + let nested = outbound.capture_internal_response(-1, || { + outbound.push_value(notification.clone()); + outbound.push_response_value(-1, json!({"id": -1, "result": "inner"})); + }); + assert_eq!(nested, Some(json!({"id": -1, "result": "inner"}))); + }); + assert_eq!(response, Some(json!({"id": -1, "result": "outer"}))); + assert_eq!(outbound.response_callback_counts(), (0, 1)); + outbound.push_response_value(-1, json!({"id": -1, "result": "canceled"})); + assert_eq!(outbound.response_callback_counts(), (0, 0)); + assert_eq!(outbound.take_pending_messages(), vec![queued, notification]); +} + #[test] fn document_inspector_context_group_ids_are_unique() { let first = DocumentInspectorContextGroupId::next(); diff --git a/moli-renderer-v8/src/worker/thread/runtime_inspector.rs b/moli-renderer-v8/src/worker/thread/runtime_inspector.rs index b53cbcbba..398dfe57b 100644 --- a/moli-renderer-v8/src/worker/thread/runtime_inspector.rs +++ b/moli-renderer-v8/src/worker/thread/runtime_inspector.rs @@ -7,6 +7,7 @@ use std::{ use serde_json::{Value, json}; use crate::inspector_microtasks::with_scoped_inspector_microtasks; +use crate::inspector_session::{InspectorSessionOutput, dispatch_with_runtime_defaults}; use crate::runtime::{RendererRuntimeInspectorMessage, RendererRuntimeInspectorResponseSender}; use crate::worker::{ handle::{WorkerRuntimeInspectorMessageBatch, WorkerToParentMessage}, @@ -34,9 +35,34 @@ struct WorkerInspectorOutboundState { #[derive(Clone, Default)] struct WorkerInspectorOutbound(Rc>); +struct WorkerInspectorSessionOutput<'a> { + outbound: &'a WorkerInspectorOutbound, + session_key: &'a str, +} + +impl InspectorSessionOutput for WorkerInspectorSessionOutput<'_> { + fn capture_internal_response(&self, call_id: i32, dispatch: impl FnOnce()) -> Option { + let scope = self + .outbound + .push_dispatch_scope(self.session_key, Some(call_id)); + dispatch(); + let mut response = None; + for message in scope.finish() { + let value = message.into_v8_inspector_message(); + if value.get("id").and_then(Value::as_i64) == Some(i64::from(call_id)) { + response = Some(value); + } else { + self.outbound.push_value(self.session_key, value); + } + } + response + } +} + #[derive(Default)] struct WorkerInspectorDispatchScope { session_key: String, + internal_call_id: Option, messages: Vec, } @@ -158,13 +184,22 @@ impl WorkerInspectorOutbound { } fn push_response_value(&self, session_key: &str, call_id: i32, value: Value) { - let callback = { - self.0 - .borrow_mut() - .pending_response_callbacks - .remove(&(session_key.to_owned(), call_id)) - }; - if let Some(callback) = callback { + let mut guard = self.0.borrow_mut(); + if let Some(scope) = guard.active_dispatch_scopes.last_mut().filter(|scope| { + scope.session_key == session_key && scope.internal_call_id == Some(call_id) + }) { + scope + .messages + .push(RendererRuntimeInspectorMessage::from_v8_inspector_message( + value, + )); + return; + } + if let Some(callback) = guard + .pending_response_callbacks + .remove(&(session_key.to_owned(), call_id)) + { + drop(guard); if let Err(message) = callback.send(value) { tracing::debug!( call_id, @@ -174,7 +209,6 @@ impl WorkerInspectorOutbound { } return; } - let mut guard = self.0.borrow_mut(); if let Some(scope) = guard .active_dispatch_scopes .last_mut() @@ -245,12 +279,17 @@ impl WorkerInspectorOutbound { coalesce_worker_inspector_messages(messages) } - fn push_dispatch_scope(&self, session_key: &str) -> WorkerInspectorDispatchScopeGuard { + fn push_dispatch_scope( + &self, + session_key: &str, + internal_call_id: Option, + ) -> WorkerInspectorDispatchScopeGuard { self.0 .borrow_mut() .active_dispatch_scopes .push(WorkerInspectorDispatchScope { session_key: session_key.to_owned(), + internal_call_id, messages: Vec::new(), }); WorkerInspectorDispatchScopeGuard { @@ -510,9 +549,16 @@ impl WorkerRuntimeInspector { self.outbound .register_response_callback(&session_key, callback); } - let dispatch_scope = self.outbound.push_dispatch_scope(&session_key); + let dispatch_scope = self.outbound.push_dispatch_scope(&session_key, None); let session = self.ensure_session(&session_key); - session.dispatch_protocol_message(v8::inspector::StringView::from(raw_json.as_bytes())); + dispatch_with_runtime_defaults( + &session, + raw_json, + &WorkerInspectorSessionOutput { + outbound: &self.outbound, + session_key: &session_key, + }, + )?; let messages = dispatch_scope.finish(); self.record_execution_context_state(&messages); Ok(messages) @@ -866,7 +912,7 @@ mod tests { fn worker_inspector_outbound_captures_dispatch_and_drops_stale_late_response() { let outbound = WorkerInspectorOutbound::default(); - let dispatch_scope = outbound.push_dispatch_scope("SID-1"); + let dispatch_scope = outbound.push_dispatch_scope("SID-1", None); outbound.push_response_value("SID-1", 1, json!({"id": 1, "result": "command-tail"})); let current = dispatch_scope.finish(); outbound.push_response_value("SID-1", 2, json!({"id": 2, "result": "late"})); @@ -884,6 +930,192 @@ mod tests { ); } + #[test] + fn worker_runtime_enable_hides_the_internal_stack_capture_limit_response() { + crate::ensure_v8_for_test(); + let mut isolate = v8::Isolate::new(Default::default()); + let inspector = WorkerRuntimeInspector::new_for_test(&mut isolate); + let context = { + let scope = pin!(v8::HandleScope::new(&mut isolate)); + let scope = &mut scope.init(); + let context = v8::Context::new(scope, Default::default()); + inspector.attach_context( + context, + v8::Global::new(scope, context), + "https://worker-runtime-limit.test/worker.js", + ); + v8::Global::new(scope, context) + }; + let scope = pin!(v8::HandleScope::new(&mut isolate)); + let scope = &mut scope.init(); + let context = v8::Local::new(scope, &context); + let scope = &mut v8::ContextScope::new(scope, context); + + let (tx, mut rx) = tokio::sync::oneshot::channel(); + inspector + .dispatch_protocol_message_with_optional_deferred_response( + scope, + None, + r#"{"id":-1,"method":"Runtime.evaluate","params":{"expression":"new Promise(resolve => globalThis.resolveStackCaptureProbe = resolve)","awaitPromise":true}}"#, + Some(RendererRuntimeInspectorResponseSender::new(-1, tx)), + ) + .expect("pending frontend evaluation before Runtime.enable"); + let messages = inspector + .dispatch_protocol_message(scope, None, r#"{"id":41,"method":"Runtime.enable"}"#) + .expect("worker Runtime.enable dispatch"); + assert_eq!(protocol_response(&messages, 41)["result"], json!({})); + assert!( + messages.iter().all(|message| match message { + RendererRuntimeInspectorMessage::Protocol(message) => message + .value() + .get("id") + .and_then(Value::as_i64) + .is_none_or(|call_id| call_id >= 0), + RendererRuntimeInspectorMessage::RuntimeContext(_) => true, + }), + "the private stack-capture command response must not enter worker CDP output: {messages:#?}" + ); + assert!( + matches!( + rx.try_recv(), + Err(tokio::sync::oneshot::error::TryRecvError::Empty) + ), + "the internal -1 response must not settle the pending frontend -1 command" + ); + inspector.dispatch_protocol_message( + scope, + None, + r#"{"id":40,"method":"Runtime.evaluate","params":{"expression":"resolveStackCaptureProbe(7)"}}"#, + ).expect("resolve frontend evaluation"); + let completion = rx.try_recv().expect("frontend promise response"); + assert_eq!( + completion + .output + .protocol_response(-1) + .expect("response ID -1")["result"]["result"]["value"], + json!(7) + ); + + let messages = inspector + .dispatch_protocol_message( + scope, + None, + r#"{"id":42,"method":"Runtime.setMaxCallStackSizeToCapture","params":{"size":8}}"#, + ) + .expect("frontend worker stack-capture command"); + assert_eq!( + protocol_response(&messages, 42)["result"], + json!({}), + "the private default must not prevent a frontend override" + ); + + inspector + .dispatch_protocol_message(scope, None, r#"{"id":43,"method":"Runtime.disable"}"#) + .expect("disable before checking the session lifecycle"); + crate::inspector_session::tests::assert_session_lifecycle(|request| { + inspector + .dispatch_protocol_message(scope, None, request) + .expect("Worker Inspector dispatch") + .into_iter() + .map(RendererRuntimeInspectorMessage::into_v8_inspector_message) + .collect() + }); + inspector + .dispatch_protocol_message(scope, None, r#"{"id":44,"method":"Runtime.disable"}"#) + .expect("disable the default session before testing independent limits"); + crate::inspector_session::tests::assert_multiple_sessions(|session, request| { + inspector + .dispatch_protocol_message(scope, Some(session), request) + .expect("Worker Inspector session dispatch") + .into_iter() + .map(RendererRuntimeInspectorMessage::into_v8_inspector_message) + .collect() + }); + } + + #[test] + fn worker_internal_response_capture_preserves_frontend_output() { + let outbound = WorkerInspectorOutbound::default(); + let scope = outbound.push_dispatch_scope("SID-1", None); + let queued = json!({"id": -1, "result": "queued"}); + let notification = json!({"method": "Runtime.consoleAPICalled", "params": {}}); + outbound.push_response_value("SID-1", -1, queued.clone()); + let (tx, mut rx) = tokio::sync::oneshot::channel(); + outbound.register_response_callback( + "SID-1", + RendererRuntimeInspectorResponseSender::new(-1, tx), + ); + let (peer_tx, mut peer_rx) = tokio::sync::oneshot::channel(); + outbound.register_response_callback( + "SID-2", + RendererRuntimeInspectorResponseSender::new(-1, peer_tx), + ); + let output = WorkerInspectorSessionOutput { + outbound: &outbound, + session_key: "SID-1", + }; + + let response = output.capture_internal_response(-1, || { + outbound.push_response_value("SID-1", -1, json!({"id": -1, "result": "outer"})); + let nested = output.capture_internal_response(-1, || { + outbound.push_response_value("SID-2", -1, json!({"id": -1, "result": "peer"})); + outbound.push_value("SID-1", notification.clone()); + outbound.push_response_value("SID-1", -1, json!({"id": -1, "result": "inner"})); + }); + assert_eq!(nested, Some(json!({"id": -1, "result": "inner"}))); + }); + assert_eq!(response, Some(json!({"id": -1, "result": "outer"}))); + assert_eq!( + peer_rx + .try_recv() + .expect("peer callback must not be intercepted") + .output + .protocol_response(-1), + Some(&json!({"id": -1, "result": "peer"})) + ); + assert!(matches!( + rx.try_recv(), + Err(tokio::sync::oneshot::error::TryRecvError::Empty) + )); + outbound.push_response_value("SID-1", -1, json!({"id": -1, "result": "frontend"})); + let completion = rx + .try_recv() + .expect("frontend callback must remain registered"); + assert_eq!( + completion.output.protocol_response(-1), + Some(&json!({"id": -1, "result": "frontend"})) + ); + assert_eq!( + scope.finish(), + vec![inspector_message(queued), inspector_message(notification)] + ); + assert!(outbound.take_all().is_empty()); + } + + #[test] + fn worker_internal_response_capture_restores_routing_after_unwind() { + let outbound = WorkerInspectorOutbound::default(); + let scope = outbound.push_dispatch_scope("SID-1", None); + let (tx, mut rx) = tokio::sync::oneshot::channel(); + outbound.register_response_callback( + "SID-1", + RendererRuntimeInspectorResponseSender::new(-1, tx), + ); + let output = WorkerInspectorSessionOutput { + outbound: &outbound, + session_key: "SID-1", + }; + assert!( + std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + output.capture_internal_response(-1, || panic!("test dispatch unwind")); + })) + .is_err() + ); + outbound.push_response_value("SID-1", -1, json!({"id": -1, "result": {}})); + assert!(rx.try_recv().is_ok()); + assert!(scope.finish().is_empty()); + } + #[test] fn worker_inspector_outbound_duplicate_response_callback_keeps_existing_callback() { let outbound = WorkerInspectorOutbound::default(); @@ -952,7 +1184,7 @@ mod tests { fn worker_inspector_outbound_tags_peer_session_pending_messages() { let outbound = WorkerInspectorOutbound::default(); - let dispatch_scope = outbound.push_dispatch_scope("SID-1"); + let dispatch_scope = outbound.push_dispatch_scope("SID-1", None); outbound.push_value( "SID-2", json!({"method": "Runtime.executionContextCreated"}), @@ -978,7 +1210,7 @@ mod tests { #[test] fn worker_pause_flushes_active_notifications_but_retains_command_response() { let outbound = WorkerInspectorOutbound::default(); - let dispatch_scope = outbound.push_dispatch_scope("SID-1"); + let dispatch_scope = outbound.push_dispatch_scope("SID-1", None); outbound.push_value("SID-1", json!({"method": "Debugger.paused", "params": {}})); outbound.push_response_value("SID-1", 7, json!({"id": 7, "result": {}}));