From b0a2e2418cfdde439edb307f7780c93d5b106d5d Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Sat, 3 Aug 2024 04:01:55 +0200 Subject: [PATCH] Bubble errors from the server to the client on suspense boundaries (#2772) * Bubble errors from the server to the client on suspense boundaries * make resolving suspense boundaries O(n) --- Cargo.lock | 1 + packages/core/src/error_boundary.rs | 16 ++-- packages/core/src/lib.rs | 8 +- packages/core/src/scope_context.rs | 12 +-- .../fullstack/src/html_storage/serialize.rs | 12 ++- packages/fullstack/src/render.rs | 92 ++++++++++++++++--- packages/playwright-tests/fullstack.spec.js | 4 + .../playwright-tests/fullstack/Cargo.toml | 3 +- .../playwright-tests/fullstack/src/main.rs | 45 ++++++++- packages/web/src/hydration/deserialize.rs | 23 ++++- packages/web/src/hydration/hydrate.rs | 7 +- packages/web/src/lib.rs | 6 +- 12 files changed, 190 insertions(+), 39 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7d2380e0f..a2779e0a6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2903,6 +2903,7 @@ version = "0.1.0" dependencies = [ "dioxus", "serde", + "tokio", ] [[package]] diff --git a/packages/core/src/error_boundary.rs b/packages/core/src/error_boundary.rs index 8d86c03d7..7c8ea6057 100644 --- a/packages/core/src/error_boundary.rs +++ b/packages/core/src/error_boundary.rs @@ -40,13 +40,11 @@ impl Display for CapturedPanic { impl Error for CapturedPanic {} /// Provide an error boundary to catch errors from child components -pub fn use_error_boundary() -> ErrorContext { - use_hook(|| { - provide_context(ErrorContext::new( - Vec::new(), - current_scope_id().unwrap_or_else(|e| panic!("{}", e)), - )) - }) +pub fn provide_error_boundary() -> ErrorContext { + provide_context(ErrorContext::new( + Vec::new(), + current_scope_id().unwrap_or_else(|e| panic!("{}", e)), + )) } /// 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. @@ -750,7 +748,7 @@ impl< /// Error boundaries are quick to implement, but it can be useful to individually handle errors in your components to provide a better user experience when you know that an error is likely to occur. #[allow(non_upper_case_globals, non_snake_case)] pub fn ErrorBoundary(props: ErrorBoundaryProps) -> Element { - let error_boundary = use_error_boundary(); + let error_boundary = use_hook(provide_error_boundary); let errors = error_boundary.errors(); if errors.is_empty() { std::result::Result::Ok({ @@ -768,6 +766,8 @@ pub fn ErrorBoundary(props: ErrorBoundaryProps) -> Element { ) }) } else { + tracing::trace!("scope id: {:?}", current_scope_id()); + tracing::trace!("handling errors: {:?}", errors); (props.handle_error.0)(error_boundary.clone()) } } diff --git a/packages/core/src/lib.rs b/packages/core/src/lib.rs index 381c8b735..b58efa564 100644 --- a/packages/core/src/lib.rs +++ b/packages/core/src/lib.rs @@ -88,10 +88,10 @@ pub mod prelude { pub use crate::innerlude::{ consume_context, consume_context_from_scope, current_owner, current_scope_id, fc_to_builder, generation, has_context, needs_update, needs_update_any, parent_scope, - provide_context, provide_root_context, queue_effect, remove_future, schedule_update, - schedule_update_any, spawn, spawn_forever, spawn_isomorphic, suspend, throw_error, - try_consume_context, use_after_render, use_before_render, use_drop, use_error_boundary, - use_hook, use_hook_with_cleanup, with_owner, AnyValue, Attribute, Callback, Component, + provide_context, provide_error_boundary, provide_root_context, queue_effect, remove_future, + schedule_update, schedule_update_any, spawn, spawn_forever, spawn_isomorphic, suspend, + throw_error, try_consume_context, use_after_render, use_before_render, use_drop, use_hook, + use_hook_with_cleanup, with_owner, AnyValue, Attribute, Callback, Component, ComponentFunction, Context, Element, ErrorBoundary, ErrorContext, Event, EventHandler, Fragment, HasAttributes, IntoAttributeValue, IntoDynNode, OptionStringFromMarker, Properties, ReactiveContext, RenderError, RenderReturn, Runtime, RuntimeGuard, ScopeId, diff --git a/packages/core/src/scope_context.rs b/packages/core/src/scope_context.rs index 9c7abfb94..4fae8c1e4 100644 --- a/packages/core/src/scope_context.rs +++ b/packages/core/src/scope_context.rs @@ -190,10 +190,10 @@ impl Scope { /// Clones the state if it exists. pub fn consume_context(&self) -> Option { tracing::trace!( - "looking for context {} ({:?}) in {}", + "looking for context {} ({:?}) in {:?}", std::any::type_name::(), std::any::TypeId::of::(), - self.name + self.id ); if let Some(this_ctx) = self.has_context() { return Some(this_ctx); @@ -207,10 +207,10 @@ impl Scope { return None; }; tracing::trace!( - "looking for context {} ({:?}) in {}", + "looking for context {} ({:?}) in {:?}", std::any::type_name::(), std::any::TypeId::of::(), - parent.name + parent.id ); if let Some(shared) = parent.has_context() { return Some(shared); @@ -277,10 +277,10 @@ impl Scope { /// ``` pub fn provide_context(&self, value: T) -> T { tracing::trace!( - "providing context {} ({:?}) in {}", + "providing context {} ({:?}) in {:?}", std::any::type_name::(), std::any::TypeId::of::(), - self.name + self.id ); let mut contexts = self.shared_contexts.borrow_mut(); diff --git a/packages/fullstack/src/html_storage/serialize.rs b/packages/fullstack/src/html_storage/serialize.rs index c6de74d86..535c36828 100644 --- a/packages/fullstack/src/html_storage/serialize.rs +++ b/packages/fullstack/src/html_storage/serialize.rs @@ -1,7 +1,7 @@ use dioxus_lib::prelude::dioxus_core::DynamicNode; use dioxus_lib::prelude::{ - has_context, try_consume_context, ScopeId, SuspenseBoundaryProps, SuspenseContext, VNode, - VirtualDom, + has_context, try_consume_context, ErrorContext, ScopeId, SuspenseBoundaryProps, + SuspenseContext, VNode, VirtualDom, }; use serde::Serialize; @@ -26,6 +26,14 @@ impl super::HTMLData { /// We use depth first order instead of relying on the order the hooks are called in because during suspense on the server, the order that futures are run in may be non deterministic. pub(crate) fn extract_from_suspense_boundary(vdom: &VirtualDom, scope: ScopeId) -> Self { let mut data = Self::default(); + // If there is an error boundary on the suspense boundary, grab the error from the context API + // and throw it on the client so that it bubbles up to the nearest error boundary + let mut error = vdom.in_runtime(|| { + scope + .consume_context::() + .and_then(|error_context| error_context.errors().first().cloned()) + }); + data.push(&error); data.take_from_scope(vdom, scope); data } diff --git a/packages/fullstack/src/render.rs b/packages/fullstack/src/render.rs index 43dc5c489..ea06477ea 100644 --- a/packages/fullstack/src/render.rs +++ b/packages/fullstack/src/render.rs @@ -1,5 +1,5 @@ //! A shared pool of renderers for efficient server side rendering. -use crate::streaming::StreamingRenderer; +use crate::streaming::{Mount, StreamingRenderer}; use dioxus_interpreter_js::INITIALIZE_STREAMING_JS; use dioxus_ssr::{ incremental::{CachedRender, RenderFreshness}, @@ -15,6 +15,13 @@ use tokio::task::JoinHandle; use crate::prelude::*; use dioxus_lib::prelude::*; +/// A suspense boundary that is pending with a placeholder in the client +struct PendingSuspenseBoundary { + mount: Mount, + children: Vec, +} + +/// Spawn a task in the background. If wasm is enabled, this will use the single threaded tokio runtime fn spawn_platform(f: impl FnOnce() -> Fut + Send + 'static) -> JoinHandle where Fut: Future + 'static, @@ -56,6 +63,7 @@ impl SsrRendererPool { } } + /// Look for a cached route in the incremental cache and send it into the render channel if it exists fn check_cached_route( &self, route: &str, @@ -91,6 +99,8 @@ impl SsrRendererPool { None } + /// Render a virtual dom into a stream. This method will return immediately and continue streaming the result in the background + /// The streaming is canceled when the stream the function returns is dropped async fn render_to( self: Arc, cfg: &ServeConfig, @@ -181,6 +191,9 @@ impl SsrRendererPool { { let scope_to_mount_mapping = scope_to_mount_mapping.clone(); let stream = stream.clone(); + // We use a stack to keep track of what suspense boundaries we are nested in to add children to the correct boundary + // The stack starts with the root scope because the root is a suspense boundary + let pending_suspense_boundaries_stack = RwLock::new(vec![]); renderer.set_render_components(move |renderer, to, vdom, scope| { let is_suspense_boundary = SuspenseContext::downcast_suspense_boundary_from_scope( @@ -191,10 +204,47 @@ impl SsrRendererPool { .is_some(); if is_suspense_boundary { let mount = stream.render_placeholder( - |to| renderer.render_scope(to, vdom, scope), + |to| { + { + pending_suspense_boundaries_stack + .write() + .unwrap() + .push(scope); + } + let out = renderer.render_scope(to, vdom, scope); + { + pending_suspense_boundaries_stack.write().unwrap().pop(); + } + out + }, &mut *to, )?; - scope_to_mount_mapping.write().unwrap().insert(scope, mount); + // Add the suspense boundary to the list of pending suspense boundaries + // We will replace the mount with the resolved contents later once the suspense boundary is resolved + let mut scope_to_mount_mapping_write = + scope_to_mount_mapping.write().unwrap(); + scope_to_mount_mapping_write.insert( + scope, + PendingSuspenseBoundary { + mount, + children: vec![], + }, + ); + // Add the scope to the list of children of the parent suspense boundary + let pending_suspense_boundaries_stack = + pending_suspense_boundaries_stack.read().unwrap(); + // If there is a parent suspense boundary, add the scope to the list of children + // This suspense boundary will start capturing errors when the parent is resolved + if let Some(parent) = pending_suspense_boundaries_stack.last() { + let parent = scope_to_mount_mapping_write.get_mut(parent).unwrap(); + parent.children.push(scope); + } + // Otherwise this is a root suspense boundary, so we need to start capturing errors immediately + else { + vdom.in_runtime(|| { + start_capturing_errors(scope); + }); + } } else { renderer.render_scope(to, vdom, scope)? } @@ -233,12 +283,12 @@ impl SsrRendererPool { // Just rerender the resolved nodes for scope in resolved_suspense_nodes { - let mount = { + let pending_suspense_boundary = { let mut lock = scope_to_mount_mapping.write().unwrap(); lock.remove(&scope) }; // If the suspense boundary was immediately removed, it may not have a mount. We can just skip resolving it - if let Some(mount) = mount { + if let Some(pending_suspense_boundary) = pending_suspense_boundary { let mut resolved_chunk = String::new(); // After we replace the placeholder in the dom with javascript, we need to send down the resolved data so that the client can hydrate the node let render_suspense = |into: &mut String| { @@ -247,7 +297,7 @@ impl SsrRendererPool { }; let resolved_data = serialize_server_data(&virtual_dom, scope); if let Err(err) = stream.replace_placeholder( - mount, + pending_suspense_boundary.mount, render_suspense, resolved_data, &mut resolved_chunk, @@ -258,13 +308,22 @@ impl SsrRendererPool { } stream.render(resolved_chunk); - } - // Freeze the suspense boundary to prevent future reruns of any child nodes of the suspense boundary - if let Some(suspense) = SuspenseContext::downcast_suspense_boundary_from_scope( - &virtual_dom.runtime(), - scope, - ) { - suspense.freeze(); + // Freeze the suspense boundary to prevent future reruns of any child nodes of the suspense boundary + if let Some(suspense) = + SuspenseContext::downcast_suspense_boundary_from_scope( + &virtual_dom.runtime(), + scope, + ) + { + suspense.freeze(); + // Go to every child suspense boundary and add an error boundary. Since we cannot rerun any nodes above the child suspense boundary, + // we need to capture the errors and send them to the client as it resolves + virtual_dom.in_runtime(|| { + for &suspense_scope in pending_suspense_boundary.children.iter() { + start_capturing_errors(suspense_scope); + } + }); + } } } } @@ -305,6 +364,13 @@ impl SsrRendererPool { } } +/// 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); +} + fn serialize_server_data(virtual_dom: &VirtualDom, scope: ScopeId) -> String { // After we replace the placeholder in the dom with javascript, we need to send down the resolved data so that the client can hydrate the node // Extract any data we serialized for hydration (from server futures) diff --git a/packages/playwright-tests/fullstack.spec.js b/packages/playwright-tests/fullstack.spec.js index 748436044..9ce803ff0 100644 --- a/packages/playwright-tests/fullstack.spec.js +++ b/packages/playwright-tests/fullstack.spec.js @@ -28,4 +28,8 @@ test("hydration", async ({ page }) => { // Expect the page to contain the updated counter text. await expect(main).toContainText("Server said: Hello from the server!"); + + // Make sure the error that was thrown on the server is shown in the error boundary on the client + const errors = page.locator("#errors"); + await expect(errors).toContainText("Hmm, something went wrong."); }); diff --git a/packages/playwright-tests/fullstack/Cargo.toml b/packages/playwright-tests/fullstack/Cargo.toml index a18e74649..f92425fb9 100644 --- a/packages/playwright-tests/fullstack/Cargo.toml +++ b/packages/playwright-tests/fullstack/Cargo.toml @@ -9,8 +9,9 @@ publish = false [dependencies] dioxus = { workspace = true, features = ["fullstack"] } serde = "1.0.159" +tokio = { workspace = true, features = ["full"], optional = true } [features] default = [] -server = ["dioxus/axum"] +server = ["dioxus/axum", "dep:tokio"] web = ["dioxus/web"] diff --git a/packages/playwright-tests/fullstack/src/main.rs b/packages/playwright-tests/fullstack/src/main.rs index 95631b08e..59edd973e 100644 --- a/packages/playwright-tests/fullstack/src/main.rs +++ b/packages/playwright-tests/fullstack/src/main.rs @@ -5,7 +5,7 @@ // - Hydration #![allow(non_snake_case)] -use dioxus::prelude::*; +use dioxus::{prelude::*, CapturedError}; fn main() { LaunchBuilder::fullstack().launch(app); @@ -31,6 +31,10 @@ fn app() -> Element { "Run a server function!" } "Server said: {text}" + div { + id: "errors", + Errors {} + } } } @@ -45,3 +49,42 @@ async fn post_server_data(data: String) -> Result<(), ServerFnError> { async fn get_server_data() -> Result { Ok("Hello from the server!".to_string()) } + +#[server] +async fn server_error() -> Result { + 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! { + "Hmm, something went wrong." + }, + SuspenseBoundary { + fallback: |_: SuspenseContext| rsx! { + div { + "Loading..." + } + }, + ThrowsError {} + } + } + } +} + +#[component] +pub fn ThrowsError() -> Element { + use_server_future(server_error)? + .unwrap() + .map_err(|err| RenderError::Aborted(CapturedError::from_display(err)))?; + rsx! { + "success" + } +} diff --git a/packages/web/src/hydration/deserialize.rs b/packages/web/src/hydration/deserialize.rs index 86e9ed35d..f97677e51 100644 --- a/packages/web/src/hydration/deserialize.rs +++ b/packages/web/src/hydration/deserialize.rs @@ -1,6 +1,7 @@ use std::cell::{Cell, RefCell}; use std::io::Cursor; +use dioxus_core::CapturedError; use serde::de::DeserializeOwned; thread_local! { @@ -37,6 +38,7 @@ fn remove_server_data() { /// Data that is deserialized from the server during hydration pub(crate) struct HTMLDataCursor { + error: Option, data: Vec>>, index: Cell, } @@ -47,11 +49,28 @@ impl HTMLDataCursor { Self::new(deserialized) } + /// Get the error if there is one + pub(crate) fn error(&self) -> Option { + self.error.clone() + } + fn new(data: Vec>>) -> Self { - Self { + let mut myself = Self { + error: None, data, index: Cell::new(0), - } + }; + + // The first item is always an error if it exists + let error = myself + .take::>() + .ok() + .flatten() + .flatten(); + + myself.error = error; + + myself } pub fn take(&self) -> Result, TakeDataError> { diff --git a/packages/web/src/hydration/hydrate.rs b/packages/web/src/hydration/hydrate.rs index 0c54ccff1..d2765b8f6 100644 --- a/packages/web/src/hydration/hydrate.rs +++ b/packages/web/src/hydration/hydrate.rs @@ -138,7 +138,12 @@ impl WebsysDom { self.interpreter.base().push_root(node); } - with_server_data(HTMLDataCursor::from_serialized(&data), || { + let server_data = HTMLDataCursor::from_serialized(&data); + // If the server serialized an error into the suspense boundary, throw it on the client so that it bubbles up to the nearest error boundary + if let Some(error) = server_data.error() { + dom.in_runtime(|| id.throw_error(error)); + } + with_server_data(server_data, || { // rerun the scope with the new data SuspenseBoundaryProps::resolve_suspense( id, diff --git a/packages/web/src/lib.rs b/packages/web/src/lib.rs index 556d5bb84..1813de423 100644 --- a/packages/web/src/lib.rs +++ b/packages/web/src/lib.rs @@ -24,7 +24,7 @@ use std::{panic, rc::Rc}; pub use crate::cfg::Config; use crate::hydration::SuspenseMessage; -use dioxus_core::VirtualDom; +use dioxus_core::{ScopeId, VirtualDom}; use futures_util::{pin_mut, select, FutureExt, StreamExt}; mod cfg; @@ -98,6 +98,10 @@ pub async fn run(virtual_dom: VirtualDom, web_config: Config) -> ! { } let hydration_data = get_initial_hydration_data().to_vec(); let server_data = HTMLDataCursor::from_serialized(&hydration_data); + // If the server serialized an error into the root suspense boundary, throw it into the root scope + if let Some(error) = server_data.error() { + dom.in_runtime(|| ScopeId::APP.throw_error(error)); + } with_server_data(server_data, || { dom.rebuild(&mut websys_dom); });