diff --git a/moli-fetch/src/error.rs b/moli-fetch/src/error.rs index 7266e6e278..5be395cee5 100644 --- a/moli-fetch/src/error.rs +++ b/moli-fetch/src/error.rs @@ -3,9 +3,31 @@ use http::StatusCode; pub const NET_ERR_ABORTED_ERROR_TEXT: &str = "net::ERR_ABORTED"; +/// Explicit request cancellation, independent of diagnostic context or wording. +#[derive(Debug, Clone, Copy)] +pub struct FetchCancelled; + +impl std::fmt::Display for FetchCancelled { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter.write_str(NET_ERR_ABORTED_ERROR_TEXT) + } +} + +impl std::error::Error for FetchCancelled {} + +/// Recognizes cancellation through typed causes, including shared body errors. +pub fn is_fetch_cancelled(error: &anyhow::Error) -> bool { + error.is::() + || error.chain().any(|cause| { + cause.is::() + || cause + .downcast_ref::() + .is_some_and(curl::Error::is_aborted_by_callback) + }) +} + pub(crate) fn browser_network_error_text(error: &anyhow::Error) -> &'static str { - let error_chain = format!("{error:#}"); - if error_chain.contains("request cancelled") || error_chain.contains("Callback aborted") { + if is_fetch_cancelled(error) { return NET_ERR_ABORTED_ERROR_TEXT; } @@ -60,6 +82,42 @@ pub fn ensure_http_status_success( mod tests { use super::*; + #[test] + fn cancellation_survives_context_without_matching_diagnostic_text() { + for error in [ + anyhow::Error::new(FetchCancelled), + anyhow::Error::new(FetchCancelled) + .context("transport stopped") + .context("failed to prepare page"), + anyhow::anyhow!("inner detail").context(FetchCancelled), + anyhow::Error::new(curl::Error::new(curl_sys::CURLE_ABORTED_BY_CALLBACK)) + .context("arbitrary diagnostic"), + ] { + assert!(is_fetch_cancelled(&error), "{error:#}"); + assert_eq!( + browser_network_error_text(&error), + NET_ERR_ABORTED_ERROR_TEXT + ); + } + } + + #[test] + fn cancellation_words_cannot_change_a_non_cancelled_failure() { + for message in ["net::ERR_ABORTED", "request cancelled", "Callback aborted"] { + let error = anyhow::anyhow!(message).context("failed to fetch"); + assert!(!is_fetch_cancelled(&error)); + assert_eq!(browser_network_error_text(&error), "net::ERR_FAILED"); + + let error = + anyhow::Error::new(curl::Error::new(curl_sys::CURLE_RECV_ERROR)).context(message); + assert!(!is_fetch_cancelled(&error)); + assert_eq!( + browser_network_error_text(&error), + "net::ERR_CONNECTION_RESET" + ); + } + } + #[test] fn curl_receive_failure_maps_to_browser_connection_reset() { let error = anyhow::Error::new(curl::Error::new(curl_sys::CURLE_RECV_ERROR)) diff --git a/moli-fetch/src/lib.rs b/moli-fetch/src/lib.rs index 943bc1d381..16dc6e1142 100644 --- a/moli-fetch/src/lib.rs +++ b/moli-fetch/src/lib.rs @@ -40,7 +40,9 @@ pub use cancellation::FetchCancelHandle; pub use client::{FetchClient, FetchClientHandle}; pub use config::FetchConfig; pub use cors::validate_cors_response_for_origin; -pub use error::{NET_ERR_ABORTED_ERROR_TEXT, ensure_http_status_success}; +pub use error::{ + FetchCancelled, NET_ERR_ABORTED_ERROR_TEXT, ensure_http_status_success, is_fetch_cancelled, +}; pub use fetch_url_list::FetchUrlList; pub use headers::{ cors_unsafe_request_header_names, is_cors_safelisted_method, diff --git a/moli-fetch/src/network_fetch_result.rs b/moli-fetch/src/network_fetch_result.rs index ded5de0963..dd09d8b1f1 100644 --- a/moli-fetch/src/network_fetch_result.rs +++ b/moli-fetch/src/network_fetch_result.rs @@ -590,6 +590,11 @@ mod tests { #[test] fn network_failure_reason_preserves_curl_code_and_details() { for (code, detail, network_error_text) in [ + ( + curl_sys::CURLE_ABORTED_BY_CALLBACK, + "transfer cancelled", + crate::NET_ERR_ABORTED_ERROR_TEXT, + ), ( curl_sys::CURLE_PEER_FAILED_VERIFICATION, "SSL certificate problem: unable to get local issuer certificate", diff --git a/moli-fetch/src/runtime.rs b/moli-fetch/src/runtime.rs index 1e2968fa25..726ea16105 100644 --- a/moli-fetch/src/runtime.rs +++ b/moli-fetch/src/runtime.rs @@ -864,7 +864,11 @@ impl RuntimeOwner { mut job: RuntimeJob, ) -> std::result::Result { if job.cancel_handle.is_cancelled() { - return Err((job.response_tx, anyhow!("fetch runtime request cancelled"))); + return Err(( + job.response_tx, + anyhow::Error::new(crate::FetchCancelled) + .context("fetch runtime request cancelled"), + )); } if let Err(error) = job.request.validate_request_mode_for_url(&job.current_url) { return Err((job.response_tx, error)); @@ -1428,12 +1432,17 @@ impl RuntimeOwner { if self.shutdown_requested.load(Ordering::SeqCst) { return Err(( job.response_tx, - anyhow!("fetch runtime request cancelled during shutdown"), + anyhow::Error::new(crate::FetchCancelled) + .context("fetch runtime request cancelled during shutdown"), )); } if let Err(error) = result { if job.cancel_handle.is_cancelled() { - return Err((job.response_tx, anyhow!("fetch runtime request cancelled"))); + return Err(( + job.response_tx, + anyhow::Error::new(crate::FetchCancelled) + .context("fetch runtime request cancelled"), + )); } if let Some(response) = easy.as_mut().and_then(take_failed_proxy_connect_response) { let response = proxy_connect_raw_response( @@ -1648,7 +1657,8 @@ impl RuntimeOwner { fail_streaming_job_with_easy( job, Some(easy), - anyhow!("fetch runtime streaming request cancelled during shutdown"), + anyhow::Error::new(crate::FetchCancelled) + .context("fetch runtime streaming request cancelled during shutdown"), ); return; } @@ -1959,7 +1969,8 @@ impl RuntimeOwner { fail_raw_streaming_job_with_easy( job, Some(easy), - anyhow!("fetch runtime raw streaming request cancelled during shutdown"), + anyhow::Error::new(crate::FetchCancelled) + .context("fetch runtime raw streaming request cancelled during shutdown"), ); return; } diff --git a/moli-protocol-webdriver-bidi/src/commands.rs b/moli-protocol-webdriver-bidi/src/commands.rs index efd42674aa..5763ebf3c0 100644 --- a/moli-protocol-webdriver-bidi/src/commands.rs +++ b/moli-protocol-webdriver-bidi/src/commands.rs @@ -20,9 +20,9 @@ use moli_protocol::devtools_runtime::{ DevToolsPreloadScriptSource, DevToolsPrintToPdfCommand, DevToolsPrintToPdfTransferMode, DevToolsRealmId, DevToolsReleaseObjectsCommand, DevToolsReloadCommand, DevToolsRemoteHandleId, DevToolsRemoveBrowserContextCommand, DevToolsRemoveNetworkDataCollectorCommand, - DevToolsRemoveNetworkInterceptCommand, DevToolsRemovePreloadScriptCommand, DevToolsRequestId, - DevToolsResultOwnership, DevToolsScreenshotClip, DevToolsScreenshotElementClip, - DevToolsSerializationOptions, DevToolsSetCacheBehaviorCommand, + DevToolsRemoveNetworkInterceptCommand, DevToolsRemovePreloadScriptCommand, + DevToolsRequestFailure, DevToolsRequestId, DevToolsResultOwnership, DevToolsScreenshotClip, + DevToolsScreenshotElementClip, DevToolsSerializationOptions, DevToolsSetCacheBehaviorCommand, DevToolsSetClientWindowStateCommand, DevToolsSetDownloadBehaviorCommand, DevToolsSetExtraHeadersCommand, DevToolsSetGeolocationOverrideCommand, DevToolsSetLocaleOverrideCommand, DevToolsSetNetworkConditionsCommand, @@ -1127,7 +1127,7 @@ fn bidi_network_fail_request_command( Ok(DevToolsFailInterceptedRequestCommand { context: context.command_context(None), request_id: DevToolsRequestId::from(required_network_request_id(&command.params)?), - error_text: "Failed".to_owned(), + failure: DevToolsRequestFailure::Failed("Failed".to_owned()), }) } diff --git a/moli-protocol-webdriver-bidi/src/tests/session_and_network_commands.rs b/moli-protocol-webdriver-bidi/src/tests/session_and_network_commands.rs index 2f2093d236..fc32de74e3 100644 --- a/moli-protocol-webdriver-bidi/src/tests/session_and_network_commands.rs +++ b/moli-protocol-webdriver-bidi/src/tests/session_and_network_commands.rs @@ -817,7 +817,10 @@ fn maps_network_response_controls_to_shared_fetch_commands() { panic!("expected FailInterceptedRequest command"); }; assert_eq!(command.request_id.as_str(), "REQ-10"); - assert_eq!(command.error_text, "Failed"); + assert_eq!( + command.failure, + moli_protocol::devtools_runtime::DevToolsRequestFailure::Failed("Failed".to_owned()) + ); } #[test] diff --git a/moli-protocol/src/conn/dispatch_tests/bidi.rs b/moli-protocol/src/conn/dispatch_tests/bidi.rs index 58c54de385..c426fb63b3 100644 --- a/moli-protocol/src/conn/dispatch_tests/bidi.rs +++ b/moli-protocol/src/conn/dispatch_tests/bidi.rs @@ -63,7 +63,9 @@ async fn bidi_fetch_control_resolves_background_request_owner() { DevToolsFailInterceptedRequestCommand { context, request_id: DevToolsRequestId::from("FETCH-background"), - error_text: "Failed".to_owned(), + failure: crate::devtools_runtime::DevToolsRequestFailure::Failed( + "Failed".to_owned(), + ), }, )) .await; diff --git a/moli-protocol/src/conn/dispatch_tests/fetch.rs b/moli-protocol/src/conn/dispatch_tests/fetch.rs index 3f5cb7823f..80959b3d46 100644 --- a/moli-protocol/src/conn/dispatch_tests/fetch.rs +++ b/moli-protocol/src/conn/dispatch_tests/fetch.rs @@ -19,7 +19,9 @@ async fn devtools_fetch_control_command_routes_through_fetch_owner() { DevToolsFailInterceptedRequestCommand { context: context.clone(), request_id: DevToolsRequestId::from("INT-99"), - error_text: "Failed".to_owned(), + failure: crate::devtools_runtime::DevToolsRequestFailure::Failed( + "Failed".to_owned(), + ), }, )) .await; diff --git a/moli-protocol/src/conn/fetch_support.rs b/moli-protocol/src/conn/fetch_support.rs index 9aa4adc0ef..5599cb284f 100644 --- a/moli-protocol/src/conn/fetch_support.rs +++ b/moli-protocol/src/conn/fetch_support.rs @@ -1214,7 +1214,7 @@ impl PausedDocumentTransfer { pub(crate) fn fail( self, - error_text: String, + error: anyhow::Error, ) -> ( Option, NavigationDispatchState, @@ -1227,13 +1227,9 @@ impl PausedDocumentTransfer { body, } => { let _ = body; - ( - document_navigation_token, - navigation, - Err(anyhow::Error::msg(error_text)), - ) + (document_navigation_token, navigation, Err(error)) } - PausedDocumentTransferState::ActiveBodyStream { stream, .. } => stream.fail(error_text), + PausedDocumentTransferState::ActiveBodyStream { stream, .. } => stream.fail(error), } } } @@ -1339,17 +1335,13 @@ impl ActiveDocumentBodyStreamState { fn fail( self, - error_text: String, + error: anyhow::Error, ) -> ( Option, NavigationDispatchState, anyhow::Result, ) { - ( - self.document_navigation_token, - self.navigation, - Err(anyhow::Error::msg(error_text)), - ) + (self.document_navigation_token, self.navigation, Err(error)) } } diff --git a/moli-protocol/src/conn/runtime_load.rs b/moli-protocol/src/conn/runtime_load.rs index f6279e20df..0f9674b253 100644 --- a/moli-protocol/src/conn/runtime_load.rs +++ b/moli-protocol/src/conn/runtime_load.rs @@ -871,6 +871,9 @@ impl BackgroundNavigationLoadJob { failure.observation_journal(), ); } + if moli_fetch::is_fetch_cancelled(&error) { + return Err(error); + } tracing::debug!( url = %self.raw_url, error = ?error, @@ -1528,7 +1531,7 @@ impl CdpConnection { && token.loader_id == navigation.loader_id && self.accepts_pending_document_navigation_for_owner(&navigation.owner, token) }) - .ok_or_else(|| anyhow::anyhow!(moli_fetch::NET_ERR_ABORTED_ERROR_TEXT))?; + .ok_or(moli_fetch::FetchCancelled)?; let mut inputs = self.navigation_request_load_inputs(navigation); inputs.document_replacement = self .runtime_session_owner_slot_for_owner(&navigation.owner) @@ -3902,6 +3905,21 @@ mod tests { }; use serde_json::json; + #[tokio::test] + async fn shared_body_cancellation_keeps_a_typed_cause_for_both_consumers() { + let (completion_tx, completion_rx) = tokio::sync::oneshot::channel(); + let error = anyhow::Error::new(moli_fetch::FetchCancelled) + .context("failed to read page body from stream"); + let navigation_error = super::complete_body_capture(Err(error), completion_tx) + .expect_err("cancelled body capture must fail navigation"); + let renderer_error = completion_rx.await.unwrap().unwrap_err(); + for error in [navigation_error, renderer_error] { + let error = error.context("consumer context"); + assert!(moli_fetch::is_fetch_cancelled(&error)); + assert!(error.root_cause().is::()); + } + } + #[tokio::test] async fn streaming_body_failure_preserves_source_for_renderer_and_navigation() { let (chunks_tx, chunks_rx) = tokio::sync::mpsc::unbounded_channel(); diff --git a/moli-protocol/src/conn/tests/navigation_error.rs b/moli-protocol/src/conn/tests/navigation_error.rs index ceec764b99..04df734e15 100644 --- a/moli-protocol/src/conn/tests/navigation_error.rs +++ b/moli-protocol/src/conn/tests/navigation_error.rs @@ -8,6 +8,153 @@ use moli_core::page::{SubresourceAuthCredentials, SubresourceAuthScheme, Subreso const OFFLINE_ERROR_TEXT: &str = "net::ERR_INTERNET_DISCONNECTED"; +#[tokio::test] +async fn cancelled_background_transport_does_not_prepare_a_network_error_document() { + use tokio::io::AsyncReadExt; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let (request_tx, request_rx) = tokio::sync::oneshot::channel(); + let server = tokio::spawn(async move { + let (mut stream, _) = listener.accept().await.unwrap(); + let mut buffer = [0; 4096]; + assert!(stream.read(&mut buffer).await.unwrap() > 0); + request_tx.send(()).unwrap(); + // Hold response metadata until cancellation disconnects the transport. + while stream.read(&mut buffer).await.unwrap_or(0) != 0 {} + }); + + let (mut ctx, token, mut navigation) = navigation_fixture(); + navigation.requested_url = Url::parse(&format!("http://{address}/pending-head")).unwrap(); + let cancellation = ctx + .conn + .document_navigation_cancellation_handle(&token) + .unwrap(); + let job = ctx + .conn + .background_navigation_load_job_for_navigation( + &token, + &navigation, + Default::default(), + None, + ) + .unwrap(); + let result = tokio::time::timeout(std::time::Duration::from_secs(30), async { + let (result, ()) = tokio::join!(job.run(None), async { + request_rx.await.unwrap(); + cancellation.cancel(); + }); + result + }) + .await; + server.abort(); + let (result, _) = result.expect("cancellation must release the pending transport"); + let error = result.expect_err("cancellation must not prepare a browser-owned error page"); + assert!( + error + .downcast_ref::() + .is_some() + ); + assert!(moli_fetch::is_fetch_cancelled(&error)); + let outcome = crate::domains::network::materialize_navigation_load_result( + &mut ctx.conn, + &navigation, + Err(error.context("background navigation")), + ); + let MaterializedNavigationLoadOutcome::Failed(failure) = outcome else { + panic!("cancelled transport must remain a failure, not a replacement document"); + }; + assert_eq!( + failure.document_policy, + FailedNavigationDocumentPolicy::PreserveCommittedDocument + ); +} + +#[tokio::test] +async fn context_wrapped_cancellation_preserves_the_committed_page_and_releases_navigation() { + let (mut ctx, fixture_token, navigation) = navigation_fixture(); + // This fixture starts an uncommitted request; retire it before installing + // the old document so it cannot keep the channel suspended during the test. + ctx.conn + .finish_renderer_document_navigation_for_owner(&navigation.owner, &fixture_token) + .unwrap(); + ctx.conn + .clear_pending_document_navigation_for_owner_if_loader_matches( + &navigation.owner, + &fixture_token.loader_id, + ); + ctx.install_navigation_fixture_for_session_owner("about:blank", Some("SID-1")) + .await; + let old_page = ctx + .conn + .runtime_session_owner_slot(Some("SID-1")) + .unwrap() + .loaded_page() + .unwrap() + .page_id(); + + for error in [ + anyhow::Error::new(moli_fetch::FetchCancelled), + anyhow::Error::new(crate::devtools_runtime::DevToolsRequestFailure::Aborted), + ] { + let token = ctx + .conn + .start_document_navigation_for_owner(&navigation.owner, navigation.loader_id.clone()) + .unwrap(); + let error = error + .context("failed to prepare page") + .context("failed to continue intercepted navigation"); + let expected_text = format!("{error:#}"); + let materialized = crate::domains::network::materialize_navigation_load_result( + &mut ctx.conn, + &navigation, + Err(error), + ); + let MaterializedNavigationLoadOutcome::Failed(failure) = &materialized else { + panic!("cancellation must produce a terminal failure"); + }; + assert_eq!( + failure.document_policy, + FailedNavigationDocumentPolicy::PreserveCommittedDocument + ); + assert_eq!(failure.error_text, expected_text); + let mut out = Vec::new(); + ctx.conn + .drain_materialized_navigation_completion_into( + &mut out, + MaterializedNavigationCompletion::new(token, navigation.clone(), materialized), + &mut CommandDispatchContext::default(), + ) + .await; + assert_eq!( + ctx.conn + .runtime_session_owner_slot(Some("SID-1")) + .unwrap() + .loaded_page() + .expect("cancel must retain the old page") + .page_id(), + old_page + ); + assert!( + !ctx.conn + .renderer_document_navigation_is_suspended_for_session_owner(Some("SID-1")) + ); + assert!( + out.iter() + .any(|message| message["error"]["message"] == expected_text) + ); + } + ctx.process_async(json!({ + "id": 700, "sessionId": "SID-1", "method": "Runtime.evaluate", + "params": { "expression": "6 * 7" } + })) + .await; + assert_eq!( + ctx.take_response_by_id(700)["result"]["result"]["value"], + json!(42) + ); +} + fn failing_streamed_document( navigation: &NavigationDispatchState, ) -> crate::conn::DocumentBodySource { diff --git a/moli-protocol/src/devtools_runtime.rs b/moli-protocol/src/devtools_runtime.rs index ae87ce3a2c..037d771542 100644 --- a/moli-protocol/src/devtools_runtime.rs +++ b/moli-protocol/src/devtools_runtime.rs @@ -1362,11 +1362,29 @@ pub struct DevToolsContinueWithAuthCommand { pub password: Option, } +/// A protocol-requested failure. Cancellation is a cause, not a display string. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum DevToolsRequestFailure { + Aborted, + Failed(String), +} + +impl std::fmt::Display for DevToolsRequestFailure { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Aborted => formatter.write_str("Aborted"), + Self::Failed(message) => formatter.write_str(message), + } + } +} + +impl std::error::Error for DevToolsRequestFailure {} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct DevToolsFailInterceptedRequestCommand { pub context: DevToolsCommandContext, pub request_id: DevToolsRequestId, - pub error_text: String, + pub failure: DevToolsRequestFailure, } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/moli-protocol/src/domains/fetch.rs b/moli-protocol/src/domains/fetch.rs index b866f03a02..869bb4475b 100644 --- a/moli-protocol/src/domains/fetch.rs +++ b/moli-protocol/src/domains/fetch.rs @@ -1007,7 +1007,8 @@ async fn complete_disable_command_async( .await; } for pending in pending_response_navigations { - let (token, navigation, result) = pending.fail("Fetch interception disabled".to_owned()); + let (token, navigation, result) = + pending.fail(anyhow::anyhow!("Fetch interception disabled")); let result = network::materialize_navigation_load_result(conn, &navigation, result); navigation::complete_tokened_materialized_navigation_as_background_events_async( conn, out, token, navigation, result, diff --git a/moli-protocol/src/domains/fetch/commands.rs b/moli-protocol/src/domains/fetch/commands.rs index 74698f38c5..af6ed05cfc 100644 --- a/moli-protocol/src/domains/fetch/commands.rs +++ b/moli-protocol/src/domains/fetch/commands.rs @@ -7,7 +7,7 @@ use crate::devtools_runtime::{ DevToolsAuthChallengeAction, DevToolsCommand, DevToolsContinueInterceptedRequestCommand, DevToolsContinueInterceptedResponseCommand, DevToolsContinueWithAuthCommand, DevToolsFailInterceptedRequestCommand, DevToolsFulfillInterceptedRequestCommand, - DevToolsProtocol, DevToolsRequestId, + DevToolsProtocol, DevToolsRequestFailure, DevToolsRequestId, }; use crate::domains::command_output::CommandOutputPlan; use crate::domains::{activity, network, page}; @@ -27,7 +27,7 @@ use super::navigation::{ }; use super::params::{ CloseWebSocketParams, ContinueRequestParams, ContinueResponseParams, - DispatchWebSocketMessageParams, FailRequestParams, FulfillRequestParams, + DispatchWebSocketMessageParams, ErrorReason, FailRequestParams, FulfillRequestParams, WebSocketMessageOpcode, }; use super::state::{ @@ -92,11 +92,14 @@ pub(super) fn start_devtools_fetch_command_for_owner( } } -fn navigation_fail_request_error_text(error_reason: Option) -> String { - let error_text = error_reason.unwrap_or_else(|| "Fetch request failed".to_owned()); - match error_text.as_str() { - "BlockedByClient" => BLOCKED_BY_CLIENT_ERROR_TEXT.to_owned(), - _ => error_text, +fn navigation_fail_request_failure(error_reason: Option) -> DevToolsRequestFailure { + match error_reason { + Some(ErrorReason::Aborted) => DevToolsRequestFailure::Aborted, + Some(ErrorReason::BlockedByClient) => { + DevToolsRequestFailure::Failed(BLOCKED_BY_CLIENT_ERROR_TEXT.to_owned()) + } + Some(reason) => DevToolsRequestFailure::Failed(reason.as_ref().to_owned()), + None => DevToolsRequestFailure::Failed("Fetch request failed".to_owned()), } } @@ -460,7 +463,7 @@ fn finish_continue_subresource_request( pub(super) enum PendingFailRequestState { Navigation { pending: Box, - error_text: String, + failure: DevToolsRequestFailure, }, SubresourceFetch { pending: Box, @@ -470,7 +473,7 @@ pub(super) enum PendingFailRequestState { }, ResponseTransfer { transfer: Box, - error_text: String, + failure: DevToolsRequestFailure, }, } @@ -487,9 +490,8 @@ pub(super) fn start_fail_request_command( )); } }; - let error_text = navigation_fail_request_error_text(params.error_reason); - let command = - build_cdp_fail_intercepted_request_command(conn, cmd, params.request_id, error_text); + let failure = navigation_fail_request_failure(params.error_reason); + let command = build_cdp_fail_intercepted_request_command(conn, cmd, params.request_id, failure); start_devtools_fetch_command( conn, cmd.id, @@ -502,14 +504,14 @@ fn build_cdp_fail_intercepted_request_command( conn: &CdpConnection, cmd: &Cmd<'_>, request_id: String, - error_text: String, + failure: DevToolsRequestFailure, ) -> DevToolsFailInterceptedRequestCommand { let (browser_context_id, target_id) = devtools_fetch_owner_identity_for_session(conn, cmd.session_id); DevToolsFailInterceptedRequestCommand { context: cmd.devtools_command_context(target_id.as_deref(), browser_context_id.as_deref()), request_id: DevToolsRequestId::from(request_id), - error_text, + failure, } } @@ -522,7 +524,8 @@ fn start_devtools_fail_intercepted_request_command( let command_session_id = owner.session_id(); let validate_request_id = command.context.protocol == DevToolsProtocol::Cdp; let request_id = command.request_id.into_string(); - let error_text = command.error_text; + let failure = command.failure; + let error_text = failure.to_string(); let action_session_id = action_session_id_for_devtools_context( command_session_id, command.context.protocol, @@ -536,7 +539,7 @@ fn start_devtools_fail_intercepted_request_command( PendingFetchCommandKind::FailRequest { state: Box::new(PendingFailRequestState::Navigation { pending: Box::new(pending), - error_text, + failure, }), }, PendingFetchCommandOperation::Ready, @@ -660,7 +663,7 @@ fn start_devtools_fail_intercepted_request_command( PendingFetchCommandKind::FailRequest { state: Box::new(PendingFailRequestState::ResponseTransfer { transfer: Box::new(transfer), - error_text, + failure, }), }, PendingFetchCommandOperation::Ready, @@ -681,10 +684,7 @@ pub(super) async fn complete_fail_request_command_async( out: &mut FetchCommandOutput, ) { match state { - PendingFailRequestState::Navigation { - pending, - error_text, - } => { + PendingFailRequestState::Navigation { pending, failure } => { let pending = *pending; emit_devtools_empty_success(out); let token = pending.document_navigation_token; @@ -692,7 +692,7 @@ pub(super) async fn complete_fail_request_command_async( let navigation = network::materialize_navigation_load_result( conn, &navigation_state, - Err(anyhow::Error::msg(error_text)), + Err(failure.into()), ); complete_tokened_materialized_navigation_as_background_events_async( conn, @@ -784,12 +784,9 @@ pub(super) async fn complete_fail_request_command_async( .await; out.extend_background_events(events); } - PendingFailRequestState::ResponseTransfer { - transfer, - error_text, - } => { + PendingFailRequestState::ResponseTransfer { transfer, failure } => { let transfer = *transfer; - let (token, navigation_state, navigation) = transfer.fail(error_text); + let (token, navigation_state, navigation) = transfer.fail(failure.into()); let navigation = network::materialize_navigation_load_result(conn, &navigation_state, navigation); emit_devtools_empty_success(out); @@ -1890,7 +1887,7 @@ fn continue_streaming_document_response_in_background( let _ = sender.send(page::BackgroundNavigationCompletion::new( document_navigation_token, navigation, - Err(anyhow::anyhow!(moli_fetch::NET_ERR_ABORTED_ERROR_TEXT)), + Err(moli_fetch::FetchCancelled.into()), )); return; } @@ -1912,6 +1909,23 @@ fn continue_streaming_document_response_in_background( #[cfg(test)] mod protocol_neutral_tests { + use crate::devtools_runtime::DevToolsRequestFailure; + + #[test] + fn cdp_abort_reason_is_preserved_as_a_typed_cause() { + use super::{ErrorReason, navigation_fail_request_failure}; + let failure = navigation_fail_request_failure(Some(ErrorReason::Aborted)); + let error = anyhow::Error::new(failure).context("interception continuation"); + assert!(matches!( + error.downcast_ref::(), + Some(DevToolsRequestFailure::Aborted) + )); + assert_eq!( + navigation_fail_request_failure(Some(ErrorReason::ConnectionAborted)), + DevToolsRequestFailure::Failed("ConnectionAborted".to_owned()), + ); + } + use crate::devtools_runtime::{AutomationEvent, DevToolsCommand, DevToolsProtocol}; use moli_core::page::SubresourceResourceType; use serde_json::{Value, json}; @@ -2473,7 +2487,7 @@ mod protocol_neutral_tests { &conn, &cmd, "interception-job-2".to_owned(), - "net::ERR_BLOCKED_BY_CLIENT".to_owned(), + DevToolsRequestFailure::Failed("net::ERR_BLOCKED_BY_CLIENT".to_owned()), ); assert_eq!(command.context.protocol, DevToolsProtocol::Cdp); @@ -2484,7 +2498,10 @@ mod protocol_neutral_tests { assert_eq!(command.context.target_id, None); assert_eq!(command.context.browser_context_id, None); assert_eq!(command.request_id.as_str(), "interception-job-2"); - assert_eq!(command.error_text, "net::ERR_BLOCKED_BY_CLIENT"); + assert_eq!( + command.failure, + DevToolsRequestFailure::Failed("net::ERR_BLOCKED_BY_CLIENT".to_owned()) + ); } #[test] diff --git a/moli-protocol/src/domains/fetch/params.rs b/moli-protocol/src/domains/fetch/params.rs index a45fe5c6c2..ac99852706 100644 --- a/moli-protocol/src/domains/fetch/params.rs +++ b/moli-protocol/src/domains/fetch/params.rs @@ -5,6 +5,7 @@ pub(super) use chromiumoxide_cdp::cdp::browser_protocol::fetch::AuthChallengeRes pub(super) use chromiumoxide_cdp::cdp::browser_protocol::fetch::{ ContinueRequestParams, ContinueResponseParams, ContinueWithAuthParams, HeaderEntry, }; +pub(super) use chromiumoxide_cdp::cdp::browser_protocol::network::ErrorReason; #[derive(Deserialize, Default)] #[serde(rename_all = "camelCase")] @@ -37,7 +38,7 @@ pub(super) struct RequestIdParam { pub(super) struct FailRequestParams { pub(super) request_id: String, #[serde(default)] - pub(super) error_reason: Option, + pub(super) error_reason: Option, } #[derive(Deserialize)] @@ -103,7 +104,24 @@ fn default_websocket_opcode() -> String { #[cfg(test)] mod tests { - use super::WebSocketMessageOpcode; + use super::{ErrorReason, FailRequestParams, WebSocketMessageOpcode}; + + #[test] + fn fail_request_decodes_a_typed_reason_not_an_error_message() { + let params: FailRequestParams = serde_json::from_value(serde_json::json!({ + "requestId": "request", "errorReason": "Aborted" + })) + .unwrap(); + assert_eq!(params.error_reason, Some(ErrorReason::Aborted)); + for error_text in ["net::ERR_ABORTED", "failed to prepare page: Aborted"] { + assert!( + serde_json::from_value::(serde_json::json!({ + "requestId": "request", "errorReason": error_text + })) + .is_err() + ); + } + } #[test] fn websocket_message_opcode_parses_supported_cdp_tokens() { 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 3502b9b72b..5f8df3d8af 100644 --- a/moli-protocol/src/domains/network/main_document_progress/mod.rs +++ b/moli-protocol/src/domains/network/main_document_progress/mod.rs @@ -21,6 +21,7 @@ use crate::conn::{ CompletedDownloadBodyArtifact, DownloadNavigation, LoadedNavigation, NavigationDispatchState, NavigationLoadOutcome, NavigationRequestBlocked, ResponseCommitReady, TargetRuntimeSlot, }; +use crate::devtools_runtime::DevToolsRequestFailure; #[cfg(test)] use gate::MainDocumentProgressDrain; @@ -71,13 +72,24 @@ pub(crate) enum FailedNavigationDocumentPolicy { } impl FailedNavigationDocumentPolicy { - fn for_navigation_error(error_text: &str) -> Self { - // Fetch.failRequest supplies the CDP reason "Aborted", while the - // transport supplies net::ERR_ABORTED. Both cancel the provisional - // load without discarding the currently committed document. - match error_text { - "Aborted" | moli_fetch::NET_ERR_ABORTED_ERROR_TEXT => Self::PreserveCommittedDocument, - _ => Self::InvalidateCommittedDocument, + fn for_navigation_error(error: &anyhow::Error) -> Self { + // Decide from the cause before projecting diagnostic text. Context and + // shared body-capture wrappers must not turn cancellation into failure. + if moli_fetch::is_fetch_cancelled(error) + || matches!( + error.downcast_ref::(), + Some(DevToolsRequestFailure::Aborted) + ) + || error.chain().any(|cause| { + matches!( + cause.downcast_ref::(), + Some(DevToolsRequestFailure::Aborted) + ) + }) + { + Self::PreserveCommittedDocument + } else { + Self::InvalidateCommittedDocument } } @@ -360,12 +372,11 @@ fn materialize_navigation_load_outcome( materialize_download_navigation_progress(conn, state, *navigation), ), NavigationLoadOutcome::NetworkFailure(error_text) => { - let document_policy = FailedNavigationDocumentPolicy::for_navigation_error(&error_text); MaterializedNavigationLoadOutcome::Failed(materialize_failed_navigation_progress( conn, state, error_text, - document_policy, + FailedNavigationDocumentPolicy::InvalidateCommittedDocument, FailedNavigationResponseMode::CdpErrorTextResult, )) } @@ -380,6 +391,7 @@ pub(crate) fn materialize_navigation_load_result( match navigation { Ok(navigation) => materialize_navigation_load_outcome(conn, state, navigation), Err(error) => { + let document_policy = FailedNavigationDocumentPolicy::for_navigation_error(&error); tracing::debug!( error = ?error, session_id = state.owner.session_id(), @@ -389,7 +401,6 @@ pub(crate) fn materialize_navigation_load_result( Some(blocked) => blocked.to_string(), None => format!("{error:#}"), }; - let document_policy = FailedNavigationDocumentPolicy::for_navigation_error(&error_text); MaterializedNavigationLoadOutcome::Failed(materialize_failed_navigation_progress( conn, state, 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 bd7b17dae8..18d65e685a 100644 --- a/moli-protocol/src/domains/network/main_document_progress/tests.rs +++ b/moli-protocol/src/domains/network/main_document_progress/tests.rs @@ -20,21 +20,48 @@ use super::*; #[test] fn aborted_navigation_preserves_the_document_without_hiding_other_failures() { - for error_text in ["Aborted", "net::ERR_ABORTED"] { + use crate::devtools_runtime::DevToolsRequestFailure; + + for error in [ + anyhow::Error::new(moli_fetch::FetchCancelled), + anyhow::Error::new(DevToolsRequestFailure::Aborted), + anyhow::anyhow!("transport detail").context(moli_fetch::FetchCancelled), + anyhow::anyhow!("interception detail").context(DevToolsRequestFailure::Aborted), + ] { assert_eq!( - FailedNavigationDocumentPolicy::for_navigation_error(error_text), + FailedNavigationDocumentPolicy::for_navigation_error(&error), + FailedNavigationDocumentPolicy::PreserveCommittedDocument, + ); + let error = error + .context("failed to prepare page") + .context("failed to continue intercepted navigation"); + assert_eq!( + FailedNavigationDocumentPolicy::for_navigation_error(&error), FailedNavigationDocumentPolicy::PreserveCommittedDocument, ); } for error_text in [ + "Aborted", + "net::ERR_ABORTED", "Failed", "net::ERR_CONNECTION_RESET", "net::ERR_BLOCKED_BY_CLIENT", ] { - assert_eq!( - FailedNavigationDocumentPolicy::for_navigation_error(error_text), - FailedNavigationDocumentPolicy::InvalidateCommittedDocument, - ); + for error in [ + anyhow::anyhow!(error_text), + anyhow::Error::new(DevToolsRequestFailure::Failed(error_text.to_owned())), + ] { + assert_eq!( + FailedNavigationDocumentPolicy::for_navigation_error(&error), + FailedNavigationDocumentPolicy::InvalidateCommittedDocument, + ); + assert_eq!( + FailedNavigationDocumentPolicy::for_navigation_error( + &error.context("net::ERR_ABORTED") + ), + FailedNavigationDocumentPolicy::InvalidateCommittedDocument, + ); + } } } diff --git a/moli-protocol/src/domains/page/termination.rs b/moli-protocol/src/domains/page/termination.rs index 6fb233e3ac..3420cebe86 100644 --- a/moli-protocol/src/domains/page/termination.rs +++ b/moli-protocol/src/domains/page/termination.rs @@ -221,7 +221,8 @@ async fn fail_pending_fetch_state_for_owner_background_events_async( merge_renderer_output_predecessor(&mut renderer_output_predecessor, predecessor); } for pending in pending_response_navigations { - let (token, navigation, _) = pending.fail(navigation_error_text.to_owned()); + let (token, navigation, _) = + pending.fail(anyhow::Error::msg(navigation_error_text.to_owned())); let result = network::materialize_navigation_failure_preserving_committed_document( conn, &navigation, diff --git a/moli-renderer-v8/src/runtime/document_replacement.rs b/moli-renderer-v8/src/runtime/document_replacement.rs index 42ff84356d..b170406be4 100644 --- a/moli-renderer-v8/src/runtime/document_replacement.rs +++ b/moli-renderer-v8/src/runtime/document_replacement.rs @@ -28,7 +28,7 @@ impl RendererDocumentReplacement { pub(super) fn begin(self) -> anyhow::Result> { anyhow::ensure!( !self.cancellation.is_cancelled() && self.pause.begin_document_replacement(), - moli_fetch::NET_ERR_ABORTED_ERROR_TEXT, + moli_fetch::FetchCancelled, ); Ok(Arc::new(RendererDocumentReplacementScope { pause: self.pause, diff --git a/moli-renderer-v8/src/runtime/document_replacement/tests.rs b/moli-renderer-v8/src/runtime/document_replacement/tests.rs index 40a9306384..486039d4e1 100644 --- a/moli-renderer-v8/src/runtime/document_replacement/tests.rs +++ b/moli-renderer-v8/src/runtime/document_replacement/tests.rs @@ -234,9 +234,7 @@ async fn canceled_replacement_is_rejected_before_renderer_admission() { let input = replacement(&pause); input.cancellation.cancel(); let result = prepare(&runtime, reservation, Some(input)).await; - assert!( - matches!(result, Err(error) if error.to_string() == moli_fetch::NET_ERR_ABORTED_ERROR_TEXT) - ); + assert!(matches!(result, Err(error) if error.is::())); assert_debugging_enabled(&pause); assert!(matches!( output_rx.try_recv(),