From 66e7c693febac66a40e1c05ebeb84eef484e97ea Mon Sep 17 00:00:00 2001 From: ldm0 Date: Sun, 4 Oct 2026 19:04:14 +0800 Subject: [PATCH] fix(navigation): keep ignored response metadata on terminal gate --- moli-protocol/src/conn/runtime_load.rs | 50 +++++++--- .../src/conn/state/navigation_outcome.rs | 4 +- .../network/main_document_progress/mod.rs | 67 +++++++------ .../network/main_document_progress/tests.rs | 98 +++++++++++++++++++ 4 files changed, 172 insertions(+), 47 deletions(-) diff --git a/moli-protocol/src/conn/runtime_load.rs b/moli-protocol/src/conn/runtime_load.rs index a52f864f84..c80b9bc7fe 100644 --- a/moli-protocol/src/conn/runtime_load.rs +++ b/moli-protocol/src/conn/runtime_load.rs @@ -104,6 +104,33 @@ fn response_status_may_use_http_error_page(status: u16) -> bool { (400..600).contains(&status) } +fn no_content_navigation_from_response( + head: ResponseHead, + request_method: String, + request_headers: moli_fetch::RequestHeaders, + network_observation_journal: NetworkObservationJournal, + body_progress_source: &MainDocumentBodyProgressSource, +) -> NavigationLoadOutcome { + let network_extra_info_available = !network_observation_journal.is_empty(); + let network_events = CompletedMainDocumentNetworkEvents::new( + request_method, + request_headers.to_byte_strings(), + head.request_cookie_report, + head.status, + head.headers, + head.cookie_set_reports, + head.redirect_chain.into_iter().map(Into::into).collect(), + network_extra_info_available, + head.from_cache, + ) + .with_negotiated_http_version(head.negotiated_http_version) + .with_network_observation_journal(network_observation_journal); + NavigationLoadOutcome::NoContent(Box::new(NoContentNavigation { + final_url: head.final_url, + network_events: body_progress_source.completed_body_network_events(network_events), + })) +} + #[allow(clippy::too_many_arguments)] async fn prepare_browser_owned_error_page_navigation_with_engine_async( engine: &mut NavigationEngine, @@ -1440,16 +1467,16 @@ async fn build_navigation_from_streaming_raw_response_with_engine_async( elapsed_ms = timing_started.elapsed().as_millis(), ); } - let body_network_progress_state = - body_progress_source.body_network_progress_for_completed_events(network_events); if response_status_has_no_content(response_status) { return Ok(NavigationLoadOutcome::NoContent(Box::new( NoContentNavigation { final_url, - network_progress: body_network_progress_state, + network_events: body_progress_source.completed_body_network_events(network_events), }, ))); } + let body_network_progress_state = + body_progress_source.body_network_progress_for_completed_events(network_events); let body_progress_source_for_body_finish = body_progress_source.clone(); let initial_body_chunk = if response_status_may_use_http_error_page(response_status) { match first_nonempty_response_body_chunk(&mut response).await? { @@ -3451,18 +3478,13 @@ impl CdpConnection { } if response_status_has_no_content(head.status) { - let network_progress = body_progress_source.body_network_progress_for_response_head( - &request_method, - &request_headers, - &head, + return Ok(no_content_navigation_from_response( + head, + request_method, + request_headers, network_observation_journal, - ); - return Ok(NavigationLoadOutcome::NoContent(Box::new( - NoContentNavigation { - final_url: head.final_url, - network_progress, - }, - ))); + &body_progress_source, + )); } if load_inputs.browser_context_id.is_none() { diff --git a/moli-protocol/src/conn/state/navigation_outcome.rs b/moli-protocol/src/conn/state/navigation_outcome.rs index 64c547db22..4fcf2c79c6 100644 --- a/moli-protocol/src/conn/state/navigation_outcome.rs +++ b/moli-protocol/src/conn/state/navigation_outcome.rs @@ -12,7 +12,7 @@ use crate::automation::FrontendProtocol; use crate::conn::{CommandOwnerScope, ResponseCommitReady}; use crate::domains::network::{ CompletedDocumentProgressTransfer, CompletedDownloadProgressTransfer, - MainDocumentBodyNetworkProgress, + CompletedMainDocumentNetworkEvents, }; use super::browser_context::BrowserContext; @@ -259,7 +259,7 @@ pub struct DownloadNavigation { /// A 204/205 response retains the current Document and aborts the navigation. pub struct NoContentNavigation { pub(crate) final_url: Url, - pub(crate) network_progress: MainDocumentBodyNetworkProgress, + pub(crate) network_events: CompletedMainDocumentNetworkEvents, } #[derive(Debug)] diff --git a/moli-protocol/src/domains/network/main_document_progress/mod.rs b/moli-protocol/src/domains/network/main_document_progress/mod.rs index 785ff767e4..3d4d987af3 100644 --- a/moli-protocol/src/domains/network/main_document_progress/mod.rs +++ b/moli-protocol/src/domains/network/main_document_progress/mod.rs @@ -437,33 +437,31 @@ fn materialize_no_content_navigation_progress( navigation: NoContentNavigation, ) -> MaterializedNavigationLoadOutcome { let error_text = moli_fetch::NET_ERR_ABORTED_ERROR_TEXT; - let progress_gate = match navigation.network_progress.into_completed_body_events() { - Some(events) => { - let context = CompletedMainDocumentProgressContext::for_navigation( - conn, - state, - completed_body_main_document_network_request_id( - main_document_network_observed(conn, state.session_id.as_deref()), - state.request_id.clone(), - ), - ); - let batches = MainDocumentNavigationProgressEventBatches::new( - context.request_and_redirect_progress_events(&events), - context.response_received_progress_events(&events, &navigation.final_url, 0), - observed_navigation_failure_event(conn, state, error_text) - .into_iter() - .collect(), - ); - let queue = MainDocumentProgressQueueHandle::from_source( - MainDocumentProgressSource::completed_body(batches), - ); - // No Document commit will advance visibility for this response. - queue - .mark_output_visible_until(MainDocumentProgressOutputBoundary::BodyFinishedVisible); - MainDocumentProgressGate::from_queue(queue) - } - None => failed_navigation_progress_gate(conn, state, error_text), - }; + let context = CompletedMainDocumentProgressContext::for_navigation( + conn, + state, + completed_body_main_document_network_request_id( + main_document_network_observed(conn, state.session_id.as_deref()), + state.request_id.clone(), + ), + ); + let batches = MainDocumentNavigationProgressEventBatches::new( + context.request_and_redirect_progress_events(&navigation.network_events), + context.response_received_progress_events( + &navigation.network_events, + &navigation.final_url, + 0, + ), + observed_navigation_failure_event(conn, state, error_text) + .into_iter() + .collect(), + ); + let queue = MainDocumentProgressQueueHandle::from_source( + MainDocumentProgressSource::completed_body(batches), + ); + // No Document commit will advance visibility for this response. + queue.mark_output_visible_until(MainDocumentProgressOutputBoundary::BodyFinishedVisible); + let progress_gate = MainDocumentProgressGate::from_queue(queue); MaterializedNavigationLoadOutcome::Failed(MaterializedFailedDocumentProgress { error_text: error_text.to_owned(), document_policy: FailedNavigationDocumentPolicy::PreserveCommittedDocument, @@ -896,12 +894,19 @@ impl MainDocumentBodyProgressSource { { MainDocumentBodyNetworkProgress::StreamingBody } else { - let mut completed_events = completed_events; - completed_events.response_stage_metadata_already_emitted = self.response_visibility - == MainDocumentResponseVisibility::AfterResponseStageContinue; - MainDocumentBodyNetworkProgress::CompletedBody(Box::new(completed_events)) + MainDocumentBodyNetworkProgress::CompletedBody(Box::new( + self.completed_body_network_events(completed_events), + )) } } + pub(crate) fn completed_body_network_events( + &self, + mut events: CompletedMainDocumentNetworkEvents, + ) -> CompletedMainDocumentNetworkEvents { + events.response_stage_metadata_already_emitted = + self.response_visibility == MainDocumentResponseVisibility::AfterResponseStageContinue; + events + } } #[derive(Debug, Clone)] diff --git a/moli-protocol/src/domains/network/main_document_progress/tests.rs b/moli-protocol/src/domains/network/main_document_progress/tests.rs index 5901712a9e..f6d13df034 100644 --- a/moli-protocol/src/domains/network/main_document_progress/tests.rs +++ b/moli-protocol/src/domains/network/main_document_progress/tests.rs @@ -1563,3 +1563,101 @@ fn aborted_navigation_progress_marks_loading_failed_canceled() { assert_eq!(out[0]["params"]["errorText"], json!("net::ERR_ABORTED")); assert_eq!(out[0]["params"]["canceled"], json!(true)); } + +#[test] +fn no_content_navigation_retains_response_metadata_with_live_progress() { + use crate::conn::{ + BrowserContext, CommandOwnerScope, NavigationRequestLoadPolicy, NavigationResultProjection, + }; + + for status in [204, 205] { + let mut conn = CdpConnection::new(); + let mut browser_context = BrowserContext::new("BID-1".to_owned()); + browser_context.set_active_target_id("FRAME-1"); + browser_context.attach_active_session("SID-1"); + browser_context + .active_page_target_mut() + .devtools_sessions + .primary_mut() + .network_session_state + .network_enabled = true; + browser_context + .active_page_target_mut() + .runtime_slot + .enable_primary_network_events(); + conn.install_browser_context_fixture_for_test(browser_context); + assert!(main_document_network_observed(&conn, Some("SID-1"))); + let state = NavigationDispatchState { + web_mcp_invocation: None, + navigation_initiator: None, + initial_document_environment: None, + auxiliary_document_response: None, + redirect_chain: Vec::new(), + redirect_headers: None, + navigate_id: Some(1), + owner: CommandOwnerScope::for_session("SID-1"), + result_projection: NavigationResultProjection::Cdp(json!({ + "frameId": "FRAME-1", "loaderId": "LOADER-1", + })), + frame_id: "FRAME-1".to_owned(), + session_id: Some("SID-1".to_owned()), + request_id: Some("REQ-1".to_owned()), + loader_id: "LOADER-1".to_owned(), + request_announced: false, + requested_url: Url::parse("http://example.test/start").unwrap(), + request_method: "GET".to_owned(), + request_body: None, + request_body_bytes: None, + request_headers: Vec::new().into(), + request_load_policy: NavigationRequestLoadPolicy::BrowserInitiated, + timestamp: 12.5, + source_document_security: Default::default(), + }; + let (live_source, _receiver) = live_progress_source(); + let source = MainDocumentBodyProgressSource::from_live_source( + Some(live_source), + MainDocumentResponseVisibility::Immediate, + ); + let mut events = completed_events(); + events.response_status = status; + events.response_headers = vec![("X-Ignored-Response".to_owned(), b"retained".to_vec())]; + let navigation = NoContentNavigation { + final_url: Url::parse("http://example.test/final").unwrap(), + network_events: source.completed_body_network_events(events), + }; + let MaterializedNavigationLoadOutcome::Failed(mut failed) = + materialize_no_content_navigation_progress(&conn, &state, navigation) + else { + panic!("ignored responses must retain main's failed-navigation projection"); + }; + let out = drain_gate_until_body_finished_visible_into_protocol_messages( + &mut failed.progress_gate, + ); + let response = out + .iter() + .find(|event| event["method"] == "Network.responseReceived") + .expect("live ignored navigation must still emit its response metadata"); + assert_eq!(response["params"]["requestId"], json!("REQ-1")); + assert_eq!(response["params"]["response"]["status"], json!(status)); + assert_eq!( + response["params"]["response"]["headers"]["X-Ignored-Response"], + json!("retained") + ); + let failure = out + .iter() + .find(|event| event["method"] == "Network.loadingFailed") + .expect("ignored navigation must terminate without committing a Document"); + assert_eq!( + failure["params"]["errorText"], + json!(moli_fetch::NET_ERR_ABORTED_ERROR_TEXT) + ); + assert_eq!( + failed.document_policy, + FailedNavigationDocumentPolicy::PreserveCommittedDocument + ); + assert!( + !out.iter() + .any(|event| event["method"] == "Network.loadingFinished") + ); + } +}