mirror of
https://github.com/lexmount/moli.git
synced 2026-09-30 08:01:36 +00:00
fix(navigation): preserve typed cancellation causes
This commit is contained in:
+60
-2
@@ -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::<FetchCancelled>()
|
||||
|| error.chain().any(|cause| {
|
||||
cause.is::<FetchCancelled>()
|
||||
|| cause
|
||||
.downcast_ref::<curl::Error>()
|
||||
.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))
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -864,7 +864,11 @@ impl RuntimeOwner {
|
||||
mut job: RuntimeJob,
|
||||
) -> std::result::Result<JobOutcome, (RuntimeResponseTx, anyhow::Error)> {
|
||||
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;
|
||||
}
|
||||
|
||||
@@ -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()),
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -1214,7 +1214,7 @@ impl PausedDocumentTransfer {
|
||||
|
||||
pub(crate) fn fail(
|
||||
self,
|
||||
error_text: String,
|
||||
error: anyhow::Error,
|
||||
) -> (
|
||||
Option<DocumentNavigationToken>,
|
||||
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<DocumentNavigationToken>,
|
||||
NavigationDispatchState,
|
||||
anyhow::Result<NavigationLoadOutcome>,
|
||||
) {
|
||||
(
|
||||
self.document_navigation_token,
|
||||
self.navigation,
|
||||
Err(anyhow::Error::msg(error_text)),
|
||||
)
|
||||
(self.document_navigation_token, self.navigation, Err(error))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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::<moli_fetch::FetchCancelled>());
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn streaming_body_failure_preserves_source_for_renderer_and_navigation() {
|
||||
let (chunks_tx, chunks_rx) = tokio::sync::mpsc::unbounded_channel();
|
||||
|
||||
@@ -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::<moli_fetch::NetworkFetchFailureContext>()
|
||||
.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 {
|
||||
|
||||
@@ -1362,11 +1362,29 @@ pub struct DevToolsContinueWithAuthCommand {
|
||||
pub password: Option<String>,
|
||||
}
|
||||
|
||||
/// 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)]
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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>) -> 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<ErrorReason>) -> 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<crate::conn::PendingFetchNavigation>,
|
||||
error_text: String,
|
||||
failure: DevToolsRequestFailure,
|
||||
},
|
||||
SubresourceFetch {
|
||||
pending: Box<crate::conn::PendingSubresourceFetchRequest>,
|
||||
@@ -470,7 +473,7 @@ pub(super) enum PendingFailRequestState {
|
||||
},
|
||||
ResponseTransfer {
|
||||
transfer: Box<crate::conn::PausedDocumentTransfer>,
|
||||
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::<DevToolsRequestFailure>(),
|
||||
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]
|
||||
|
||||
@@ -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<String>,
|
||||
pub(super) error_reason: Option<ErrorReason>,
|
||||
}
|
||||
|
||||
#[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::<FailRequestParams>(serde_json::json!({
|
||||
"requestId": "request", "errorReason": error_text
|
||||
}))
|
||||
.is_err()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn websocket_message_opcode_parses_supported_cdp_tokens() {
|
||||
|
||||
@@ -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::<DevToolsRequestFailure>(),
|
||||
Some(DevToolsRequestFailure::Aborted)
|
||||
)
|
||||
|| error.chain().any(|cause| {
|
||||
matches!(
|
||||
cause.downcast_ref::<DevToolsRequestFailure>(),
|
||||
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,
|
||||
|
||||
@@ -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,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -28,7 +28,7 @@ impl RendererDocumentReplacement {
|
||||
pub(super) fn begin(self) -> anyhow::Result<Arc<RendererDocumentReplacementScope>> {
|
||||
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,
|
||||
|
||||
@@ -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::<moli_fetch::FetchCancelled>()));
|
||||
assert_debugging_enabled(&pause);
|
||||
assert!(matches!(
|
||||
output_rx.try_recv(),
|
||||
|
||||
Reference in New Issue
Block a user