fix(fetch): share timeout across replacement navigation

This commit is contained in:
ldm0
2026-08-17 01:23:24 +08:00
committed by Donough Liu
parent be85a67d54
commit d7df97dc4f
6 changed files with 84 additions and 172 deletions
+8 -105
View File
@@ -1,24 +1,16 @@
//! Core fetch bridge for synchronous renderer lifecycle-target decisions.
//!
//! The renderer owns the exact DCL/load boundary. This module owns the async
//! host-side budget transition: the original fetch deadline remains active
//! until the synchronous decider chooses `FollowNextDocument`, at which point a
//! separate successor-navigation budget replaces it.
//! The renderer owns the exact DCL/load boundary and any successor-navigation
//! grace period. The host keeps one deadline around the complete fetch, so a
//! lifecycle decision cannot reset or extend the caller's timeout budget.
use super::{
Browser, FetchedDocument, PageVmInitStage, RenderedDomWaitUntil, RendererLifecycleDecider,
Browser, FetchedDocument, RenderedDomWaitUntil, RendererLifecycleDecider,
RendererLifecycleDecision, RendererLifecycleSnapshot, RendererReplyBoundary,
};
use anyhow::{Context, Result, anyhow};
use moli_fetch::Request;
use std::{future::Future, time::Duration};
use tokio::sync::oneshot;
use tracing::warn;
pub(super) struct FollowTimeout {
follow_rx: oneshot::Receiver<()>,
successor_timeout: Duration,
}
use std::time::Duration;
impl Browser {
/// Fetches an executable document with a synchronous one-shot policy at
@@ -26,14 +18,13 @@ impl Browser {
///
/// The decision runs in the renderer owner turn that observes DCL/load;
/// it does not expose an intermediate Page or require a second owner
/// command. `successor_timeout` reserves the independent budget used
/// after a decision chooses to follow a successor Document.
/// command. The original `timeout` covers the request, the first lifecycle
/// target, any successor-navigation grace period, and the successor target.
pub async fn fetch_document_with_lifecycle_decider<F>(
&self,
request: Request,
wait_until: RenderedDomWaitUntil,
timeout: Duration,
successor_timeout: Duration,
decider: F,
) -> Result<FetchedDocument>
where
@@ -48,27 +39,13 @@ impl Browser {
),
"a lifecycle decider requires DCL, load, or done"
);
let (follow_tx, follow_rx) = oneshot::channel();
let decider = RendererLifecycleDecider::new(move |target| {
let decision = decider(target)?;
if matches!(
decision,
RendererLifecycleDecision::FollowNextDocument { .. }
) {
let _ = follow_tx.send(());
}
Ok(decision)
});
let decider = RendererLifecycleDecider::new(decider);
self.fetch_document_with_wait(
request,
wait_until,
timeout,
RendererReplyBoundary::Stage,
Some(decider),
Some(FollowTimeout {
follow_rx,
successor_timeout,
}),
)
.await
.with_context(|| {
@@ -77,78 +54,4 @@ impl Browser {
)
})
}
pub(super) async fn materialize_with_follow_timeout<T, F>(
&self,
raw_url: &str,
wait_until: RenderedDomWaitUntil,
timeout: Duration,
stage: PageVmInitStage,
initial_timeout: Duration,
extension: Option<FollowTimeout>,
future: F,
) -> Result<T>
where
F: Future<Output = Result<T>>,
{
let Some(mut extension) = extension else {
return self
.fetch_document_wait_timeout(
raw_url,
wait_until,
timeout,
stage,
initial_timeout,
future,
)
.await;
};
let initial_deadline = tokio::time::Instant::now()
.checked_add(initial_timeout)
.unwrap_or_else(tokio::time::Instant::now);
let initial_deadline_sleep = tokio::time::sleep_until(initial_deadline);
tokio::pin!(initial_deadline_sleep);
tokio::pin!(future);
let follow = tokio::select! {
biased;
result = &mut future => return result,
selected = &mut extension.follow_rx => selected,
_ = &mut initial_deadline_sleep => {
return Err(self.fetch_document_wait_timeout_error(
raw_url, wait_until, timeout, stage,
));
}
};
if follow.is_err() {
// Finish drops the extension sender in the same callback turn.
// Keep the original deadline while the Page creation reply is
// finalized; only Follow is allowed to replace this budget.
return match tokio::time::timeout_at(initial_deadline, &mut future).await {
Ok(result) => result,
Err(_) => {
Err(self.fetch_document_wait_timeout_error(raw_url, wait_until, timeout, stage))
}
};
}
match tokio::time::timeout(extension.successor_timeout, &mut future).await {
Ok(result) => result,
Err(_) => {
warn!(
url = %raw_url,
wait_until = ?wait_until,
timeout_ms = extension.successor_timeout.as_millis(),
stage = ?stage,
"successor navigation lifecycle target timed out"
);
Err(anyhow!(
"successor navigation to {stage:?} timed out after {} ms for `{raw_url}`",
extension.successor_timeout.as_millis()
))
}
}
}
}
+18 -26
View File
@@ -50,8 +50,6 @@ pub use navigation_engine::{
static NEXT_SESSION_ID: AtomicU64 = AtomicU64::new(1);
use self::lifecycle_fetch::FollowTimeout;
fn wait_until_outer_timeout(wait_until: RenderedDomWaitUntil, timeout: Duration) -> Duration {
match wait_until {
RenderedDomWaitUntil::NetworkIdle | RenderedDomWaitUntil::DomStable => {
@@ -418,7 +416,6 @@ impl Browser {
timeout,
RendererReplyBoundary::Stage,
None,
None,
)
.await
}
@@ -430,7 +427,6 @@ impl Browser {
timeout: Duration,
reply_boundary: RendererReplyBoundary,
lifecycle_decider: Option<RendererLifecycleDecider>,
follow_timeout: Option<FollowTimeout>,
) -> Result<FetchedDocument> {
let stage = match wait_until {
RenderedDomWaitUntil::DomContentLoaded => PageVmInitStage::DomContentLoaded,
@@ -449,17 +445,23 @@ impl Browser {
"starting fetch_document_allow_http_error_with_wait_until deadline"
);
let outer_timeout = wait_until_outer_timeout(wait_until, timeout);
let timeout_started = Instant::now();
let deadline = tokio::time::Instant::now()
.checked_add(outer_timeout)
.with_context(|| {
anyhow!(
"fetch wait_until {wait_until:?} timeout of {} ms exceeds the supported range",
timeout.as_millis()
)
})?;
let requested_url = request.url.clone();
if is_about_blank_url(&requested_url) {
return self
.materialize_with_follow_timeout(
.fetch_document_wait_timeout(
&raw_url,
wait_until,
timeout,
stage,
outer_timeout,
follow_timeout,
deadline,
self.materialize_static_html_page(
&raw_url,
requested_url,
@@ -484,7 +486,7 @@ impl Browser {
wait_until,
timeout,
stage,
outer_timeout,
deadline,
self.fetch_service_worker_main_resource_for_navigation(
&request,
&navigation_loader,
@@ -506,17 +508,13 @@ impl Browser {
raw_response,
))));
}
let remaining_timeout = outer_timeout
.checked_sub(timeout_started.elapsed())
.unwrap_or_default();
return self
.materialize_with_follow_timeout(
.fetch_document_wait_timeout(
&raw_url,
wait_until,
timeout,
stage,
remaining_timeout,
follow_timeout,
deadline,
self.materialize_streaming_raw_response_page(
&raw_url,
requested_url,
@@ -538,9 +536,7 @@ impl Browser {
wait_until,
timeout,
stage,
outer_timeout
.checked_sub(timeout_started.elapsed())
.unwrap_or_default(),
deadline,
navigation_loader.fetch_raw_stream(request),
)
.await?;
@@ -555,17 +551,13 @@ impl Browser {
.map(|raw| FetchedDocument::Raw(Box::new(raw)));
}
let remaining_timeout = outer_timeout
.checked_sub(timeout_started.elapsed())
.unwrap_or_default();
let document_fetch_context_seed = navigation_loader.commit(response.final_url.clone())?;
self.materialize_with_follow_timeout(
self.fetch_document_wait_timeout(
&raw_url,
wait_until,
timeout,
stage,
remaining_timeout,
follow_timeout,
deadline,
self.materialize_streaming_raw_response_page(
&raw_url,
requested_url,
@@ -939,13 +931,13 @@ impl Browser {
wait_until: RenderedDomWaitUntil,
timeout: Duration,
stage: PageVmInitStage,
outer_timeout: Duration,
deadline: tokio::time::Instant,
future: F,
) -> Result<T>
where
F: std::future::Future<Output = Result<T>>,
{
match tokio::time::timeout(outer_timeout, future).await {
match tokio::time::timeout_at(deadline, future).await {
Ok(result) => result,
Err(_) => {
Err(self.fetch_document_wait_timeout_error(raw_url, wait_until, timeout, stage))
+40 -17
View File
@@ -34,15 +34,14 @@ async fn follow_http_error_navigation(
url: &str,
wait_until: RenderedDomWaitUntil,
navigation_grace_ms: u64,
successor_timeout: Duration,
timeout: Duration,
) -> Result<Page> {
executable_page(
browser
.fetch_document_with_lifecycle_decider(
Request::get(url)?,
wait_until,
Duration::from_secs(5),
successor_timeout,
timeout,
move |target| {
ensure!(
(400..=599).contains(&target.status),
@@ -72,7 +71,6 @@ async fn lifecycle_decider_finishes_without_extra_owner_command() -> Result<()>
Request::get(&url)?,
RenderedDomWaitUntil::DomContentLoaded,
Duration::from_secs(5),
Duration::ZERO,
move |target| {
observed_targets_for_decider.lock().push(target);
Ok(RendererLifecycleDecision::Finish)
@@ -119,7 +117,6 @@ async fn lifecycle_decider_supports_static_about_blank() -> Result<()> {
Request::get("about:blank")?,
RenderedDomWaitUntil::Done,
Duration::from_secs(1),
Duration::ZERO,
move |target| {
*observed_target_for_decider.lock() = Some(target);
Ok(RendererLifecycleDecision::Finish)
@@ -139,7 +136,7 @@ async fn lifecycle_decider_supports_static_about_blank() -> Result<()> {
}
#[tokio::test(flavor = "multi_thread")]
async fn follow_budget_does_not_extend_initial_stage_timeout() -> Result<()> {
async fn lifecycle_decider_does_not_extend_initial_stage_timeout() -> Result<()> {
let server = FixtureServer::spawn().await?;
let browser = Browser::new(AppConfig::default())?;
let decider_was_called = Arc::new(Mutex::new(false));
@@ -150,14 +147,13 @@ async fn follow_budget_does_not_extend_initial_stage_timeout() -> Result<()> {
Request::get(&server.url("/wait-until-domcontentloaded-runtime-script-very-slow"))?,
RenderedDomWaitUntil::Load,
Duration::from_millis(100),
Duration::from_secs(5),
move |_| {
*decider_was_called_in_hook.lock() = true;
Ok(RendererLifecycleDecision::Finish)
},
)
.await
.expect_err("a follow budget must not relax the initial Load deadline");
.expect_err("a lifecycle decider must not relax the initial Load deadline");
assert!(
format!("{error:#}").contains("timed out after 100 ms"),
@@ -168,6 +164,42 @@ async fn follow_budget_does_not_extend_initial_stage_timeout() -> Result<()> {
Ok(())
}
#[tokio::test(flavor = "multi_thread")]
async fn follow_navigation_grace_cannot_extend_fetch_timeout() -> Result<()> {
let browser = Browser::new(AppConfig::default())?;
let result = tokio::time::timeout(
Duration::from_secs(1),
browser.fetch_document_with_lifecycle_decider(
Request::get("about:blank")?,
RenderedDomWaitUntil::Done,
Duration::from_millis(100),
|_| {
Ok(RendererLifecycleDecision::FollowNextDocument {
navigation_grace_ms: 10_000,
})
},
),
)
.await
.context("successor grace escaped the fetch deadline")?;
let error = result.expect_err("the original fetch timeout must interrupt successor grace");
assert!(
format!("{error:#}").contains("timed out after 100 ms"),
"error={error:#}"
);
let page = tokio::time::timeout(
Duration::from_secs(1),
browser.fetch_request_document_allow_http_error(Request::get("about:blank")?),
)
.await
.context("timed-out lifecycle follow left the renderer owner blocked")??;
assert_eq!(executable_page(page)?.status(), 200);
Ok(())
}
#[tokio::test(flavor = "multi_thread")]
async fn lifecycle_decider_error_and_panic_retire_only_pending_page() -> Result<()> {
let browser = Browser::new(AppConfig::default())?;
@@ -177,7 +209,6 @@ async fn lifecycle_decider_error_and_panic_retire_only_pending_page() -> Result<
Request::get("about:blank")?,
RenderedDomWaitUntil::Done,
Duration::from_secs(1),
Duration::ZERO,
|_| Err(anyhow!("policy rejected target")),
)
.await
@@ -192,7 +223,6 @@ async fn lifecycle_decider_error_and_panic_retire_only_pending_page() -> Result<
Request::get("about:blank")?,
RenderedDomWaitUntil::Done,
Duration::from_secs(1),
Duration::ZERO,
|_| -> Result<RendererLifecycleDecision> { panic!("policy panic sentinel") },
)
.await
@@ -227,7 +257,6 @@ async fn http_error_navigation_wait_follows_same_url_reload_to_domcontentloaded(
Request::get(&url)?,
RenderedDomWaitUntil::DomContentLoaded,
Duration::from_secs(5),
Duration::from_secs(6),
move |target| {
*observed_target_for_decider.lock() = Some(target);
Ok(RendererLifecycleDecision::FollowNextDocument {
@@ -356,7 +385,6 @@ async fn http_error_navigation_wait_follows_same_url_reload_to_load() -> Result<
Request::get(&url)?,
RenderedDomWaitUntil::Load,
Duration::from_secs(5),
Duration::from_secs(6),
move |target| {
*observed_target_for_decider.lock() = Some(target);
Ok(RendererLifecycleDecision::FollowNextDocument {
@@ -403,7 +431,6 @@ async fn http_error_navigation_wait_reports_no_navigation_without_refetching() -
Request::get(&url)?,
RenderedDomWaitUntil::Load,
Duration::from_secs(5),
Duration::from_millis(1_100),
|target| {
assert_eq!(target.status, 404);
Ok(RendererLifecycleDecision::FollowNextDocument {
@@ -432,7 +459,6 @@ async fn same_document_navigation_does_not_satisfy_http_error_replacement_wait()
Request::get(&url)?,
RenderedDomWaitUntil::Load,
Duration::from_secs(5),
Duration::from_secs(1),
|target| {
ensure!(target.status == 403, "expected 403, got {}", target.status);
Ok(RendererLifecycleDecision::FollowNextDocument {
@@ -460,7 +486,6 @@ async fn dropping_http_error_navigation_wait_keeps_renderer_owner_usable() -> Re
let mut waiting_fetch = Box::pin(browser.fetch_document_with_lifecycle_decider(
Request::get(&url)?,
RenderedDomWaitUntil::Load,
Duration::from_secs(5),
Duration::from_secs(11),
move |target| {
ensure!(target.status == 404, "expected 404, got {}", target.status);
@@ -503,7 +528,6 @@ async fn parked_http_error_navigation_wait_does_not_block_another_page() -> Resu
let mut waiting_fetch = Box::pin(browser.fetch_document_with_lifecycle_decider(
Request::get(&url)?,
RenderedDomWaitUntil::Load,
Duration::from_secs(5),
Duration::from_secs(11),
move |target| {
ensure!(target.status == 404, "expected 404, got {}", target.status);
@@ -546,7 +570,6 @@ async fn http_error_replacement_wait_keeps_chained_navigation_limit() -> Result<
.fetch_document_with_lifecycle_decider(
Request::get(&url)?,
RenderedDomWaitUntil::Load,
Duration::from_secs(5),
Duration::from_secs(20),
|target| {
ensure!(target.status == 403, "expected 403, got {}", target.status);
+13 -18
View File
@@ -18,29 +18,24 @@ pub(super) async fn fetch_with_http_error_navigation(
browser: &Browser,
request: Request,
wait_until: RenderedDomWaitUntil,
stage_timeout: Duration,
timeout: Duration,
navigation_grace: Duration,
) -> Result<FetchedDocument> {
let navigation_grace_ms = navigation_grace.as_millis().min(u128::from(u64::MAX)) as u64;
// The first DCL/load is delivered to this synchronous decision normally.
// A 4xx/5xx gets one grace window to start a replacement navigation; the
// successor then has its own complete budget to reach the same stage.
// A 4xx/5xx may spend up to the configured grace waiting for a replacement
// navigation. That wait and the successor's matching lifecycle stage both
// consume the original timeout budget; neither starts a fresh budget.
browser
.fetch_document_with_lifecycle_decider(
request,
wait_until,
stage_timeout,
navigation_grace.saturating_add(stage_timeout),
move |target| {
Ok(if is_http_error_status(target.status) {
RendererLifecycleDecision::FollowNextDocument {
navigation_grace_ms,
}
} else {
RendererLifecycleDecision::Finish
})
},
)
.fetch_document_with_lifecycle_decider(request, wait_until, timeout, move |target| {
Ok(if is_http_error_status(target.status) {
RendererLifecycleDecision::FollowNextDocument {
navigation_grace_ms,
}
} else {
RendererLifecycleDecision::Finish
})
})
.await
}
+3 -2
View File
@@ -135,8 +135,9 @@ pub struct FetchArgs {
#[arg(long, value_parser = parse_response_json_path_arg)]
pub wait_response_json: Option<ResponseJsonPathArg>,
/// Maximum wait time in milliseconds. Network-idle and DOM-stable fetches
/// return the current page with a warning when this deadline expires.
/// Maximum wait time in milliseconds. Initial and HTTP-error replacement
/// navigations share one lifecycle deadline. Network-idle and DOM-stable
/// fetches return the current page with a warning when it expires.
#[arg(long, alias = "wait-ms", default_value_t = 25_000)]
pub timeout: u64,
+2 -4
View File
@@ -2126,7 +2126,7 @@ fn cli_load_follows_five_same_url_replacements_after_403() -> Result<()> {
}
#[test]
fn cli_load_keeps_the_successor_stage_timeout_after_403_navigation() -> Result<()> {
fn cli_load_uses_one_timeout_across_403_replacement_navigation() -> Result<()> {
let runtime = tokio::runtime::Runtime::new()?;
let server = runtime.block_on(FixtureServer::spawn())?;
let url = server.url("/wait-until-http-error-navigation");
@@ -2138,9 +2138,7 @@ fn cli_load_keeps_the_successor_stage_timeout_after_403_navigation() -> Result<(
let stderr = clean_output(&output.stderr);
assert!(stdout.is_empty(), "stdout={stdout}");
assert!(
stderr.contains("successor navigation")
&& stderr.contains("Load")
&& stderr.contains("timed out"),
stderr.contains("allow-http-error wait_until Load timed out after 250 ms"),
"stderr={stderr}"
);
Ok(())