From d65f9a53c4ef92f5d99d7942f688d843ab1607bc Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Fri, 3 Oct 2025 00:21:41 -0500 Subject: [PATCH] Fix hydration error suspense (#4640) * Fix hydration of error boundaries * add a regression test * add more comments * fix typos * more typos * fix clippy --- Cargo.lock | 10 +++ Cargo.toml | 1 + packages/core/src/error_boundary.rs | 81 +++++++++++++++---- packages/core/src/lib.rs | 25 +++--- packages/fullstack-hooks/Cargo.toml | 1 + packages/fullstack-hooks/src/errors.rs | 29 +++++++ packages/fullstack-hooks/src/lib.rs | 1 + packages/fullstack-protocol/src/lib.rs | 2 +- .../playwright-tests/fullstack-errors.spec.js | 13 +++ .../fullstack-errors/.gitignore | 3 + .../fullstack-errors/Cargo.toml | 17 ++++ .../fullstack-errors/src/main.rs | 67 +++++++++++++++ .../playwright-tests/playwright.config.js | 9 +++ packages/server/src/render.rs | 18 +++-- packages/web/src/lib.rs | 8 +- 15 files changed, 249 insertions(+), 36 deletions(-) create mode 100644 packages/fullstack-hooks/src/errors.rs create mode 100644 packages/playwright-tests/fullstack-errors.spec.js create mode 100644 packages/playwright-tests/fullstack-errors/.gitignore create mode 100644 packages/playwright-tests/fullstack-errors/Cargo.toml create mode 100644 packages/playwright-tests/fullstack-errors/src/main.rs diff --git a/Cargo.lock b/Cargo.lock index 87e6c8680..48c596cda 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5546,6 +5546,7 @@ dependencies = [ "dioxus-hooks", "dioxus-signals", "futures-channel", + "futures-util", "serde", ] @@ -5748,6 +5749,15 @@ dependencies = [ "dioxus", ] +[[package]] +name = "dioxus-playwright-fullstack-errors-test" +version = "0.1.0" +dependencies = [ + "dioxus", + "serde", + "tokio", +] + [[package]] name = "dioxus-playwright-fullstack-mounted-test" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index fd6ff6b94..3a4f0c894 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -125,6 +125,7 @@ members = [ "packages/playwright-tests/web-hash-routing", "packages/playwright-tests/barebones-template", "packages/playwright-tests/fullstack", + "packages/playwright-tests/fullstack-errors", "packages/playwright-tests/fullstack-mounted", "packages/playwright-tests/fullstack-routing", "packages/playwright-tests/suspense-carousel", diff --git a/packages/core/src/error_boundary.rs b/packages/core/src/error_boundary.rs index 67651da6f..698e4b64d 100644 --- a/packages/core/src/error_boundary.rs +++ b/packages/core/src/error_boundary.rs @@ -1,6 +1,7 @@ use crate::{ - global_context::current_scope_id, innerlude::provide_context, use_hook, Element, IntoDynNode, - Properties, ScopeId, Template, TemplateAttribute, TemplateNode, VNode, + global_context::current_scope_id, innerlude::provide_context, try_consume_context, use_hook, + Element, IntoDynNode, Properties, ReactiveContext, ScopeId, Subscribers, Template, + TemplateAttribute, TemplateNode, VNode, }; use std::{ any::Any, @@ -39,12 +40,35 @@ impl Display for CapturedPanic { impl Error for CapturedPanic {} -/// Provide an error boundary to catch errors from child components -pub fn provide_error_boundary() -> ErrorContext { - provide_context(ErrorContext::new( - Vec::new(), - current_scope_id().unwrap_or_else(|e| panic!("{}", e)), - )) +/// A context supplied by fullstack to create hydration compatible error boundaries. Generally, this +/// is not present and the default in memory error boundary is used. If fullstack is enabled, it will +/// provide its own factory that handles syncing errors to the hydration context +#[derive(Clone, Copy)] +struct CreateErrorBoundary(fn() -> ErrorContext); + +impl Default for CreateErrorBoundary { + fn default() -> Self { + Self(|| ErrorContext::new(Vec::new())) + } +} + +/// Provides a method that is used to create error boundaries in `use_error_boundary_provider`. +/// This is only called from fullstack to create a hydration compatible error boundary +#[doc(hidden)] +pub fn provide_create_error_boundary(create_error_boundary: fn() -> ErrorContext) { + provide_context(CreateErrorBoundary(create_error_boundary)); +} + +/// Create an error boundary with the current error boundary factory (either hydration compatible or default) +fn create_error_boundary() -> ErrorContext { + let create_error_boundary: CreateErrorBoundary = try_consume_context().unwrap_or_default(); + (create_error_boundary.0)() +} + +/// Provide an error boundary to catch errors from child components. This needs to called in a hydration comptable +/// order if fullstack is enabled +pub fn use_error_boundary_provider() -> ErrorContext { + use_hook(|| provide_context(create_error_boundary())) } /// A trait for any type that can be downcast to a concrete type and implements Debug. This is automatically implemented for all types that implement Any + Debug. @@ -228,10 +252,18 @@ impl AnyError for T { } /// A context with information about suspended components -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct ErrorContext { errors: Rc>>, - id: ScopeId, + subscribers: Subscribers, +} + +impl Debug for ErrorContext { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ErrorContext") + .field("errors", &self.errors) + .finish() + } } impl PartialEq for ErrorContext { @@ -241,16 +273,22 @@ impl PartialEq for ErrorContext { } impl ErrorContext { - /// Create a new suspense boundary in a specific scope - pub(crate) fn new(errors: Vec, id: ScopeId) -> Self { + /// Create a new in memory error context. This will not be serialized to the hydration context if fullstack + /// is enabled + pub fn new(errors: Vec) -> Self { Self { errors: Rc::new(RefCell::new(errors)), - id, + subscribers: Subscribers::default(), } } /// Get all errors thrown from child components pub fn errors(&self) -> Ref<'_, [CapturedError]> { + // Subscribe to the current reactive context if one exists. This is usually + // the error boundary component that is rendering the errors + if let Some(rc) = ReactiveContext::current() { + self.subscribers.add(rc); + } Ref::map(self.errors.borrow(), |errors| errors.as_slice()) } @@ -262,13 +300,24 @@ impl ErrorContext { /// Push an error into this Error Boundary pub fn insert_error(&self, error: CapturedError) { self.errors.borrow_mut().push(error); - self.id.needs_update(); + self.mark_dirty() } /// Clear all errors from this Error Boundary pub fn clear_errors(&self) { self.errors.borrow_mut().clear(); - self.id.needs_update(); + self.mark_dirty(); + } + + /// Mark the error context as dirty and notify all subscribers + fn mark_dirty(&self) { + let mut this_subscribers_vec = Vec::new(); + self.subscribers + .visit(|subscriber| this_subscribers_vec.push(*subscriber)); + for subscriber in this_subscribers_vec { + self.subscribers.remove(&subscriber); + subscriber.mark_dirty(); + } } } @@ -800,7 +849,7 @@ impl< /// ``` #[allow(non_upper_case_globals, non_snake_case)] pub fn ErrorBoundary(props: ErrorBoundaryProps) -> Element { - let error_boundary = use_hook(provide_error_boundary); + let error_boundary = use_error_boundary_provider(); let errors = error_boundary.errors(); let has_errors = !errors.is_empty(); // Drop errors before running user code that might borrow the error lock diff --git a/packages/core/src/lib.rs b/packages/core/src/lib.rs index c093003e0..cb5ec4892 100644 --- a/packages/core/src/lib.rs +++ b/packages/core/src/lib.rs @@ -79,18 +79,19 @@ pub(crate) mod innerlude { pub use crate::innerlude::{ consume_context, consume_context_from_scope, current_owner, current_scope_id, fc_to_builder, force_all_dirty, generation, has_context, needs_update, needs_update_any, parent_scope, - provide_context, provide_error_boundary, provide_root_context, queue_effect, remove_future, - schedule_update, schedule_update_any, spawn, spawn_forever, spawn_isomorphic, suspend, - suspense_context, throw_error, try_consume_context, use_after_render, use_before_render, - use_drop, use_hook, use_hook_with_cleanup, vdom_is_rendering, with_owner, AnyValue, Attribute, - AttributeValue, Callback, CapturedError, Component, ComponentFunction, Context, DynamicNode, - Element, ElementId, ErrorBoundary, ErrorContext, Event, EventHandler, Fragment, HasAttributes, - IntoAttributeValue, IntoDynNode, LaunchConfig, ListenerCallback, MarkerWrapper, Mutation, - Mutations, NoOpMutations, Ok, OptionStringFromMarker, Properties, ReactiveContext, RenderError, - Result, Runtime, RuntimeGuard, ScopeId, ScopeState, SpawnIfAsync, SubscriberList, Subscribers, - SuperFrom, SuperInto, SuspendedFuture, SuspenseBoundary, SuspenseBoundaryProps, - SuspenseContext, SuspenseExtension, Task, Template, TemplateAttribute, TemplateNode, - VComponent, VNode, VNodeInner, VPlaceholder, VText, VirtualDom, WriteMutations, + provide_context, provide_create_error_boundary, provide_root_context, queue_effect, + remove_future, schedule_update, schedule_update_any, spawn, spawn_forever, spawn_isomorphic, + suspend, suspense_context, throw_error, try_consume_context, use_after_render, + use_before_render, use_drop, use_error_boundary_provider, use_hook, use_hook_with_cleanup, + vdom_is_rendering, with_owner, AnyValue, Attribute, AttributeValue, Callback, CapturedError, + Component, ComponentFunction, Context, DynamicNode, Element, ElementId, ErrorBoundary, + ErrorContext, Event, EventHandler, Fragment, HasAttributes, IntoAttributeValue, IntoDynNode, + LaunchConfig, ListenerCallback, MarkerWrapper, Mutation, Mutations, NoOpMutations, Ok, + OptionStringFromMarker, Properties, ReactiveContext, RenderError, Result, Runtime, + RuntimeGuard, ScopeId, ScopeState, SpawnIfAsync, SubscriberList, Subscribers, SuperFrom, + SuperInto, SuspendedFuture, SuspenseBoundary, SuspenseBoundaryProps, SuspenseContext, + SuspenseExtension, Task, Template, TemplateAttribute, TemplateNode, VComponent, VNode, + VNodeInner, VPlaceholder, VText, VirtualDom, WriteMutations, }; pub use const_format; diff --git a/packages/fullstack-hooks/Cargo.toml b/packages/fullstack-hooks/Cargo.toml index 6e659767d..483f58b48 100644 --- a/packages/fullstack-hooks/Cargo.toml +++ b/packages/fullstack-hooks/Cargo.toml @@ -19,6 +19,7 @@ futures-channel = { workspace = true } serde = { workspace = true } dioxus-history.workspace = true dioxus-document.workspace = true +futures-util.workspace = true [dev-dependencies] dioxus-fullstack = { workspace = true } diff --git a/packages/fullstack-hooks/src/errors.rs b/packages/fullstack-hooks/src/errors.rs new file mode 100644 index 000000000..033b066b0 --- /dev/null +++ b/packages/fullstack-hooks/src/errors.rs @@ -0,0 +1,29 @@ +//! Contains a hydration compatible error boundary context. + +use dioxus_core::{spawn_isomorphic, CapturedError, ErrorContext, ReactiveContext}; +use dioxus_fullstack_protocol::serialize_context; +use futures_util::StreamExt; + +/// Initializes an error boundary context that is compatible with hydration. +pub fn init_error_boundary() -> ErrorContext { + let initial_errors = serialize_context().create_entry::>(); + let (rx_context, mut rx) = ReactiveContext::new(); + let errors = ErrorContext::new(Vec::new()); + for error in initial_errors.get().iter().flatten() { + errors.insert_error(error.clone()); + } + rx_context.run_in(|| { + errors.errors(); + }); + spawn_isomorphic({ + let errors = errors.clone(); + async move { + if rx.next().await.is_some() { + rx_context.run_in(|| { + initial_errors.insert(&errors.errors().to_vec(), std::panic::Location::caller()) + }); + } + } + }); + errors +} diff --git a/packages/fullstack-hooks/src/lib.rs b/packages/fullstack-hooks/src/lib.rs index f5da3d6b5..8eb696498 100644 --- a/packages/fullstack-hooks/src/lib.rs +++ b/packages/fullstack-hooks/src/lib.rs @@ -2,6 +2,7 @@ #![doc = include_str!("../README.md")] pub mod document; +pub mod errors; pub mod history; mod hooks; mod streaming; diff --git a/packages/fullstack-protocol/src/lib.rs b/packages/fullstack-protocol/src/lib.rs index 5f81d1203..5c9d95229 100644 --- a/packages/fullstack-protocol/src/lib.rs +++ b/packages/fullstack-protocol/src/lib.rs @@ -128,7 +128,7 @@ impl Clone for SerializeContextEntry { impl SerializeContextEntry { /// Insert data into an entry that was created with [`HydrationContext::create_entry`] - pub fn insert(self, value: &T, location: &'static std::panic::Location<'static>) + pub fn insert(&self, value: &T, location: &'static std::panic::Location<'static>) where T: Serialize, { diff --git a/packages/playwright-tests/fullstack-errors.spec.js b/packages/playwright-tests/fullstack-errors.spec.js new file mode 100644 index 000000000..432f61295 --- /dev/null +++ b/packages/playwright-tests/fullstack-errors.spec.js @@ -0,0 +1,13 @@ +// @ts-check +const { test, expect } = require("@playwright/test"); + +test("errors", async ({ page }) => { + await page.goto("http://localhost:3232"); + + // Make sure the error that was thrown on the server is shown in the error boundary on the client + const errors = page.locator("#error-fallback-button"); + await expect(errors).toContainText("Error fallback button clicked 0 times"); + // Make sure the fallback is interactive + await errors.click(); + await expect(errors).toContainText("Error fallback button clicked 1 times"); +}); diff --git a/packages/playwright-tests/fullstack-errors/.gitignore b/packages/playwright-tests/fullstack-errors/.gitignore new file mode 100644 index 000000000..4a7bcb809 --- /dev/null +++ b/packages/playwright-tests/fullstack-errors/.gitignore @@ -0,0 +1,3 @@ +.dioxus +dist +target \ No newline at end of file diff --git a/packages/playwright-tests/fullstack-errors/Cargo.toml b/packages/playwright-tests/fullstack-errors/Cargo.toml new file mode 100644 index 000000000..3a667b42c --- /dev/null +++ b/packages/playwright-tests/fullstack-errors/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "dioxus-playwright-fullstack-errors-test" +version = "0.1.0" +edition = "2021" +publish = false + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +dioxus = { workspace = true, features = ["fullstack"] } +serde = "1.0.219" +tokio = { workspace = true, features = ["full"], optional = true } + +[features] +default = [] +server = ["dioxus/server", "dep:tokio"] +web = ["dioxus/web"] diff --git a/packages/playwright-tests/fullstack-errors/src/main.rs b/packages/playwright-tests/fullstack-errors/src/main.rs new file mode 100644 index 000000000..2c2f7c70a --- /dev/null +++ b/packages/playwright-tests/fullstack-errors/src/main.rs @@ -0,0 +1,67 @@ +// This test is used by playwright configured in the root of the repo +// Tests: +// - Errors that originate in the initial render + +#![allow(non_snake_case)] +use dioxus::prelude::*; + +fn main() { + launch(app); +} + +fn app() -> Element { + rsx! { + Errors {} + } +} + +#[server] +async fn server_error() -> ServerFnResult { + tokio::time::sleep(tokio::time::Duration::from_millis(1000)).await; + Err(ServerFnError::new("the server threw an error!")) +} + +#[component] +fn Errors() -> Element { + rsx! { + // This is a tricky case for suspense https://github.com/DioxusLabs/dioxus/issues/2570 + // Root suspense boundary is already resolved when the inner suspense boundary throws an error. + // We need to throw the error from the inner suspense boundary on the server to the hydrated + // suspense boundary on the client + ErrorBoundary { + handle_error: |_| rsx! { + ErrorFallbackButton {} + }, + SuspenseBoundary { + fallback: |_: SuspenseContext| rsx! { + div { + "Loading..." + } + }, + ThrowsError {} + } + } + } +} + +#[component] +pub fn ErrorFallbackButton() -> Element { + let mut count = use_signal(|| 0); + + rsx! { + // Make sure the error fallback is interactive after hydration + button { + id: "error-fallback-button", + onclick: move |_| count += 1, + "Error fallback button clicked {count} times" + } + } +} + +#[component] +pub fn ThrowsError() -> Element { + use_server_future(server_error)?.unwrap()?; + rsx! { + "success" + } +} diff --git a/packages/playwright-tests/playwright.config.js b/packages/playwright-tests/playwright.config.js index 0078fdca4..c3151e01d 100644 --- a/packages/playwright-tests/playwright.config.js +++ b/packages/playwright-tests/playwright.config.js @@ -120,6 +120,15 @@ module.exports = defineConfig({ reuseExistingServer: !process.env.CI, stdout: "pipe", }, + { + cwd: path.join(process.cwd(), "fullstack-errors"), + command: + 'cargo run --package dioxus-cli --release -- run --verbose --force-sequential --web --addr "127.0.0.1" --port 3232', + port: 3232, + timeout: 50 * 60 * 1000, + reuseExistingServer: !process.env.CI, + stdout: "pipe", + }, { cwd: path.join(process.cwd(), "fullstack-mounted"), command: diff --git a/packages/server/src/render.rs b/packages/server/src/render.rs index 4ee870726..c99d1c111 100644 --- a/packages/server/src/render.rs +++ b/packages/server/src/render.rs @@ -6,8 +6,7 @@ use crate::{ }; use dioxus_cli_config::base_path; use dioxus_core::{ - has_context, provide_error_boundary, DynamicNode, ErrorContext, ScopeId, SuspenseContext, - VNode, VirtualDom, + has_context, DynamicNode, ErrorContext, ScopeId, SuspenseContext, VNode, VirtualDom, }; use dioxus_fullstack_hooks::history::provide_fullstack_history_context; use dioxus_fullstack_hooks::{StreamingContext, StreamingStatus}; @@ -207,8 +206,14 @@ impl SsrRendererPool { virtual_dom.provide_root_context(document.clone() as Rc); virtual_dom.provide_root_context(streaming_context); - // Wrap the memory history in a fullstack history provider to provide the initial route for hydration - in_root_scope(&virtual_dom, || provide_fullstack_history_context(history)); + in_root_scope(&virtual_dom, || { + // Wrap the memory history in a fullstack history provider to provide the initial route for hydration + provide_fullstack_history_context(history); + // Provide a hydration compatible error boundary that serializes errors for the client + dioxus_core::provide_create_error_boundary( + dioxus_fullstack_hooks::errors::init_error_boundary, + ); + }); // rebuild the virtual dom virtual_dom.rebuild_in_place(); @@ -482,8 +487,9 @@ fn streaming_render_component_callback( /// Start capturing errors at a suspense boundary. If the parent suspense boundary is frozen, we need to capture the errors in the suspense boundary /// and send them to the client to continue bubbling up fn start_capturing_errors(suspense_scope: ScopeId) { - // Add an error boundary to the scope - suspense_scope.in_runtime(provide_error_boundary); + // Add an error boundary to the scope. We serialize the suspense error boundary separately so we can use + // the normal in memory ErrorContext here + suspense_scope.in_runtime(|| dioxus_core::provide_context(ErrorContext::new(Vec::new()))); } fn serialize_server_data(virtual_dom: &VirtualDom, scope: ScopeId) -> SerializedHydrationData { diff --git a/packages/web/src/lib.rs b/packages/web/src/lib.rs index 5afca0dab..d16d4be00 100644 --- a/packages/web/src/lib.rs +++ b/packages/web/src/lib.rs @@ -135,8 +135,14 @@ pub async fn run(mut virtual_dom: VirtualDom, web_config: Config) -> ! { virtual_dom.in_runtime(|| dioxus_core::ScopeId::APP.throw_error(error)); } server_data.in_context(|| { - #[cfg(feature = "document")] virtual_dom.in_runtime(|| { + // Provide a hydration compatible create error boundary method + dioxus_core::ScopeId::ROOT.in_runtime(|| { + dioxus_core::provide_create_error_boundary( + dioxus_fullstack_hooks::errors::init_error_boundary, + ); + }); + #[cfg(feature = "document")] document::init_fullstack_document(); }); virtual_dom.rebuild(&mut websys_dom);