diff --git a/Cargo.lock b/Cargo.lock index 3b4b68b5db..1e96dbcad8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3921,6 +3921,7 @@ dependencies = [ "dioxus-isrg", "dioxus-lib", "dioxus-mobile", + "dioxus-router", "dioxus-ssr", "dioxus-web", "dioxus_server_macro", @@ -4123,6 +4124,15 @@ dependencies = [ "tokio", ] +[[package]] +name = "dioxus-playwright-fullstack-routing-test" +version = "0.1.0" +dependencies = [ + "dioxus", + "serde", + "tokio", +] + [[package]] name = "dioxus-playwright-fullstack-test" version = "0.1.0" @@ -15400,9 +15410,9 @@ checksum = "1e9df38ee2d2c3c5948ea468a8406ff0db0b29ae1ffde1bcf20ef305bcc95c51" [[package]] name = "wry" -version = "0.50.3" +version = "0.50.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d2ec139df5102db821f92a42033c3fa0467c5ab434511e79c65881d6bdf2b369" +checksum = "804a7d1613bd699beccaa60f3b3c679acee21cebba1945a693f5eab95c08d1fa" dependencies = [ "base64 0.22.1", "block2 0.6.0", diff --git a/Cargo.toml b/Cargo.toml index ed48fc020d..e6dd71b6d6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -98,6 +98,7 @@ members = [ "packages/playwright-tests/web", "packages/playwright-tests/fullstack", "packages/playwright-tests/fullstack-mounted", + "packages/playwright-tests/fullstack-routing", "packages/playwright-tests/suspense-carousel", "packages/playwright-tests/nested-suspense", "packages/playwright-tests/cli-optimization", diff --git a/packages/core/src/error_boundary.rs b/packages/core/src/error_boundary.rs index 8d39b6fde7..2311cfd62b 100644 --- a/packages/core/src/error_boundary.rs +++ b/packages/core/src/error_boundary.rs @@ -3,7 +3,7 @@ use crate::{ Properties, ScopeId, Template, TemplateAttribute, TemplateNode, VNode, }; use std::{ - any::{Any, TypeId}, + any::Any, backtrace::Backtrace, cell::{Ref, RefCell}, error::Error, @@ -493,11 +493,7 @@ impl Display for CapturedError { impl CapturedError { /// Downcast the error type into a concrete error type pub fn downcast(&self) -> Option<&T> { - if TypeId::of::() == (*self.error).type_id() { - self.error.as_any().downcast_ref::() - } else { - None - } + self.error.as_any().downcast_ref::() } } diff --git a/packages/fullstack/Cargo.toml b/packages/fullstack/Cargo.toml index 6b8c04874e..803757c684 100644 --- a/packages/fullstack/Cargo.toml +++ b/packages/fullstack/Cargo.toml @@ -25,6 +25,7 @@ generational-box = { workspace = true } # Dioxus + SSR dioxus-ssr = { workspace = true, optional = true } dioxus-isrg = { workspace = true, optional = true } +dioxus-router = { workspace = true, optional = true } hyper = { workspace = true, optional = true } http = { workspace = true, optional = true } @@ -96,6 +97,7 @@ server = [ "dep:tokio-stream", "dep:dioxus-ssr", "dep:dioxus-isrg", + "dep:dioxus-router", "dep:tower", "dep:hyper", "dep:http", diff --git a/packages/fullstack/src/render.rs b/packages/fullstack/src/render.rs index 5f683b256a..9586868d06 100644 --- a/packages/fullstack/src/render.rs +++ b/packages/fullstack/src/render.rs @@ -6,6 +6,7 @@ use dioxus_cli_config::base_path; use dioxus_interpreter_js::INITIALIZE_STREAMING_JS; use dioxus_isrg::{CachedRender, IncrementalRendererError, RenderFreshness}; use dioxus_lib::document::Document; +use dioxus_router::prelude::ParseRouteError; use dioxus_ssr::Renderer; use futures_channel::mpsc::Sender; use futures_util::{Stream, StreamExt}; @@ -50,6 +51,14 @@ where } } +/// Errors that can occur during server side rendering before the initial chunk is sent down +pub enum SSRError { + /// An error from the incremental renderer. This should result in a 500 code + Incremental(IncrementalRendererError), + /// An error from the dioxus router. This should result in a 404 code + Routing(ParseRouteError), +} + struct SsrRendererPool { renderers: RwLock>, incremental_cache: Option>, @@ -112,7 +121,7 @@ impl SsrRendererPool { RenderFreshness, impl Stream>, ), - dioxus_isrg::IncrementalRendererError, + SSRError, > { struct ReceiverWithDrop { receiver: futures_channel::mpsc::Receiver< @@ -145,6 +154,8 @@ impl SsrRendererPool { Result, >(1000); + let (initial_result_tx, initial_result_rx) = futures_channel::oneshot::channel(); + // before we even spawn anything, we can check synchronously if we have the route cached if let Some(freshness) = self.check_cached_route(&route, &mut into) { return Ok(( @@ -188,7 +199,7 @@ impl SsrRendererPool { virtual_dom.provide_root_context(Rc::new(history) as Rc); virtual_dom.provide_root_context(document.clone() as std::rc::Rc); - // poll the future, which may call server_context() + // rebuild the virtual dom, which may call server_context() with_server_context(server_context.clone(), || virtual_dom.rebuild_in_place()); // If streaming is disabled, wait for the virtual dom to finish all suspense work @@ -197,6 +208,41 @@ impl SsrRendererPool { ProvideServerContext::new(virtual_dom.wait_for_suspense(), server_context.clone()) .await } + // check if there are any errors + let errors = with_server_context(server_context.clone(), || { + virtual_dom.in_runtime(|| { + let error_context: ErrorContext = ScopeId::APP + .consume_context() + .expect("The root should be under an error boundary"); + let errors = error_context.errors(); + errors.to_vec() + }) + }); + if errors.is_empty() { + // If routing was successful, we can return a 200 status and render into the stream + _ = initial_result_tx.send(Ok(())); + } else { + // If there was an error while routing, return the error with a 400 status + // Return a routing error if any of the errors were a routing error + let routing_error = errors.iter().find_map(|err| err.downcast().cloned()); + if let Some(routing_error) = routing_error { + _ = initial_result_tx.send(Err(SSRError::Routing(routing_error))); + return; + } + #[derive(thiserror::Error, Debug)] + #[error("{0}")] + pub struct ErrorWhileRendering(String); + let mut all_errors = String::new(); + for error in errors { + all_errors += &error.to_string(); + all_errors += "\n" + } + let error = ErrorWhileRendering(all_errors); + _ = initial_result_tx.send(Err(SSRError::Incremental( + IncrementalRendererError::Other(Box::new(error)), + ))); + return; + } let mut pre_body = String::new(); @@ -325,6 +371,11 @@ impl SsrRendererPool { myself.renderers.write().unwrap().push(renderer); }); + // Wait for the initial result which determines the status code + initial_result_rx.await.map_err(|err| { + SSRError::Incremental(IncrementalRendererError::Other(Box::new(err))) + })??; + Ok(( RenderFreshness::now(None), ReceiverWithDrop { @@ -447,7 +498,7 @@ impl SSRState { RenderFreshness, impl Stream>, ), - dioxus_isrg::IncrementalRendererError, + SSRError, > { self.renderers .clone() diff --git a/packages/fullstack/src/server/mod.rs b/packages/fullstack/src/server/mod.rs index f600adeee1..fe725d45be 100644 --- a/packages/fullstack/src/server/mod.rs +++ b/packages/fullstack/src/server/mod.rs @@ -69,6 +69,7 @@ use http::header::*; use std::sync::Arc; +use crate::render::SSRError; use crate::{prelude::*, ContextProviders}; /// A extension trait with utilities for integrating Dioxus with your Axum router. @@ -413,10 +414,17 @@ pub async fn render_handler( apply_request_parts_to_response(headers, &mut response); Result::, StatusCode>::Ok(response) } - Err(e) => { + Err(SSRError::Incremental(e)) => { tracing::error!("Failed to render page: {}", e); Ok(report_err(e).into_response()) } + Err(SSRError::Routing(e)) => { + tracing::trace!("Page not found: {}", e); + Ok(Response::builder() + .status(StatusCode::NOT_FOUND) + .body(Body::from("Page not found")) + .unwrap()) + } } } diff --git a/packages/isrg/src/lib.rs b/packages/isrg/src/lib.rs index 6f50fe1454..b4f76509c8 100644 --- a/packages/isrg/src/lib.rs +++ b/packages/isrg/src/lib.rs @@ -156,7 +156,7 @@ pub enum IncrementalRendererError { /// An IO error occurred while rendering a route. #[error("IoError: {0}")] IoError(#[from] std::io::Error), - /// An IO error occurred while rendering a route. + /// An error occurred while rendering a route. #[error("Other: {0}")] Other(#[from] Box), } diff --git a/packages/playwright-tests/fullstack-routing.spec.js b/packages/playwright-tests/fullstack-routing.spec.js new file mode 100644 index 0000000000..3085bd557b --- /dev/null +++ b/packages/playwright-tests/fullstack-routing.spec.js @@ -0,0 +1,53 @@ +// @ts-check +const { test, expect } = require("@playwright/test"); + +// Wait for the build to finish +async function waitForBuild(request) { + for (let i = 0; i < 10; i++) { + const build = await request.get("http://localhost:8888"); + let text = await build.text(); + if (!text.includes("Backend connection failed")) { + return; + } + await new Promise((r) => setTimeout(r, 1000)); + } +} + +// The home and id routes should return 200 +test("home route", async ({ request }) => { + await waitForBuild(request); + const response = await request.get("http://localhost:8888"); + + expect(response.status()).toBe(200); + + const text = await response.text(); + expect(text).toContain("Home"); +}); + +test("blog route", async ({ request }) => { + await waitForBuild(request); + const response = await request.get("http://localhost:8888/blog/123"); + + expect(response.status()).toBe(200); + + const text = await response.text(); + expect(text).toContain("id: 123"); +}); + +// The error route should return 500 +test("error route", async ({ request }) => { + await waitForBuild(request); + const response = await request.get("http://localhost:8888/error"); + + expect(response.status()).toBe(500); +}); + +// An unknown route should return 404 +test("unknown route", async ({ request }) => { + await waitForBuild(request); + const response = await request.get( + "http://localhost:8888/this-route-does-not-exist" + ); + + expect(response.status()).toBe(404); +}); diff --git a/packages/playwright-tests/fullstack-routing/.gitignore b/packages/playwright-tests/fullstack-routing/.gitignore new file mode 100644 index 0000000000..4a7bcb8092 --- /dev/null +++ b/packages/playwright-tests/fullstack-routing/.gitignore @@ -0,0 +1,3 @@ +.dioxus +dist +target \ No newline at end of file diff --git a/packages/playwright-tests/fullstack-routing/Cargo.toml b/packages/playwright-tests/fullstack-routing/Cargo.toml new file mode 100644 index 0000000000..8238f57a5a --- /dev/null +++ b/packages/playwright-tests/fullstack-routing/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "dioxus-playwright-fullstack-routing-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", "router"] } +serde = "1.0.159" +tokio = { workspace = true, features = ["full"], optional = true } + +[features] +default = [] +server = ["dioxus/server", "dep:tokio"] +web = ["dioxus/web"] diff --git a/packages/playwright-tests/fullstack-routing/src/main.rs b/packages/playwright-tests/fullstack-routing/src/main.rs new file mode 100644 index 0000000000..b7f56f11ff --- /dev/null +++ b/packages/playwright-tests/fullstack-routing/src/main.rs @@ -0,0 +1,54 @@ +// This test is used by playwright configured in the root of the repo +// Tests: +// - 200 Routes +// - 404 Routes +// - 500 Routes + +#![allow(non_snake_case)] +use dioxus::{prelude::*, CapturedError}; + +fn main() { + dioxus::LaunchBuilder::new() + .with_cfg(server_only! { + dioxus::fullstack::ServeConfig::builder().enable_out_of_order_streaming() + }) + .launch(app); +} + +fn app() -> Element { + rsx! { Router:: {} } +} + +#[derive(Clone, Routable, Debug, PartialEq, serde::Serialize, serde::Deserialize)] +enum Route { + #[route("/")] + Home, + + #[route("/blog/:id/")] + Blog { id: i32 }, + + #[route("/error")] + ThrowsError, +} + +#[component] +fn Blog(id: i32) -> Element { + rsx! { + Link { to: Route::Home {}, "Go home" } + "id: {id}" + } +} + +#[component] +fn ThrowsError() -> Element { + return Err(RenderError::Aborted(CapturedError::from_display( + "This route tests uncaught errors in the server", + ))); +} + +#[component] +fn Home() -> Element { + rsx! { + "Home" + } +} diff --git a/packages/playwright-tests/playwright.config.js b/packages/playwright-tests/playwright.config.js index 0899cd40c6..44f84ecc63 100644 --- a/packages/playwright-tests/playwright.config.js +++ b/packages/playwright-tests/playwright.config.js @@ -111,6 +111,15 @@ module.exports = defineConfig({ reuseExistingServer: !process.env.CI, stdout: "pipe", }, + { + cwd: path.join(process.cwd(), "fullstack-routing"), + command: + 'cargo run --package dioxus-cli --release -- serve --force-sequential --platform web --addr "127.0.0.1" --port 8888', + port: 8888, + timeout: 50 * 60 * 1000, + reuseExistingServer: !process.env.CI, + stdout: "pipe", + }, { cwd: path.join(process.cwd(), "suspense-carousel"), command: diff --git a/packages/router/src/components/router.rs b/packages/router/src/components/router.rs index 84dda98def..0d5d668164 100644 --- a/packages/router/src/components/router.rs +++ b/packages/router/src/components/router.rs @@ -1,7 +1,5 @@ use dioxus_lib::prelude::*; -use std::str::FromStr; - use crate::{ prelude::{provide_router_context, Outlet}, routable::Routable, @@ -39,10 +37,7 @@ impl PartialEq for RouterProps { } /// A component that renders the current route. -pub fn Router(props: RouterProps) -> Element -where - ::Err: std::fmt::Display, -{ +pub fn Router(props: RouterProps) -> Element { use crate::prelude::{outlet::OutletContext, RouterContext}; use_hook(|| { diff --git a/packages/router/src/contexts/router.rs b/packages/router/src/contexts/router.rs index 65b9fd43f3..2abccfdc92 100644 --- a/packages/router/src/contexts/router.rs +++ b/packages/router/src/contexts/router.rs @@ -1,5 +1,7 @@ use std::{ collections::HashSet, + error::Error, + fmt::Display, sync::{Arc, Mutex}, }; @@ -11,6 +13,19 @@ use crate::{ prelude::SiteMapSegment, routable::Routable, router_cfg::RouterConfig, }; +/// An error that is thrown when the router fails to parse a route +#[derive(Debug, Clone)] +pub struct ParseRouteError { + message: String, +} + +impl Error for ParseRouteError {} +impl Display for ParseRouteError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.message.fmt(f) + } +} + /// This context is set in the root of the virtual dom if there is a router present. #[derive(Clone, Copy)] struct RootRouterContext(Signal>); @@ -94,10 +109,7 @@ pub struct RouterContext { } impl RouterContext { - pub(crate) fn new(cfg: RouterConfig) -> Self - where - ::Err: std::fmt::Display, - { + pub(crate) fn new(cfg: RouterConfig) -> Self { let subscribers = Arc::new(Mutex::new(HashSet::new())); let mapping = consume_child_route_mapping(); @@ -233,15 +245,21 @@ impl RouterContext { let absolute_route = self.full_route_string(); // If this is a child route, map the absolute route to the child route before parsing let mapping = consume_child_route_mapping::(); - match mapping.as_ref() { + let route = match mapping.as_ref() { Some(mapping) => mapping .parse_route_from_root_route(&absolute_route) - .unwrap_or_else(|| { - panic!("route's display implementation must be parsable by FromStr") - }), - None => R::from_str(&absolute_route).unwrap_or_else(|_| { - panic!("route's display implementation must be parsable by FromStr") - }), + .ok_or_else(|| "Failed to parse route".to_string()), + None => { + R::from_str(&absolute_route).map_err(|err| format!("Failed to parse route {err}")) + } + }; + + match route { + Ok(route) => route, + Err(err) => { + throw_error(ParseRouteError { message: err }); + "/".parse().unwrap_or_else(|err| panic!("{err}")) + } } } diff --git a/packages/router/src/lib.rs b/packages/router/src/lib.rs index f7d529909a..eda92422ef 100644 --- a/packages/router/src/lib.rs +++ b/packages/router/src/lib.rs @@ -39,7 +39,7 @@ mod contexts { pub(crate) mod router; pub use navigator::*; pub(crate) use router::*; - pub use router::{root_router, GenericRouterContext, RouterContext}; + pub use router::{root_router, GenericRouterContext, ParseRouteError, RouterContext}; } mod router_cfg; diff --git a/packages/router/src/routable.rs b/packages/router/src/routable.rs index 23cfd9c46f..34017cd8ef 100644 --- a/packages/router/src/routable.rs +++ b/packages/router/src/routable.rs @@ -592,7 +592,7 @@ type SiteMapFlattened<'a> = FlatMap< note = "Routable should generally be derived using the `#[derive(Routable)]` macro." ) )] -pub trait Routable: FromStr + Display + Clone + 'static { +pub trait Routable: FromStr + Display + Clone + 'static { /// The error that can occur when parsing a route. const SITE_MAP: &'static [SiteMapSegment]; diff --git a/packages/router/src/router_cfg.rs b/packages/router/src/router_cfg.rs index 675956f3a1..78c5a38250 100644 --- a/packages/router/src/router_cfg.rs +++ b/packages/router/src/router_cfg.rs @@ -43,7 +43,6 @@ impl Default for RouterConfig { impl RouterConfig where R: Routable, - ::Err: std::fmt::Display, { /// A function to be called whenever the routing is updated. /// diff --git a/packages/router/tests/via_ssr/link.rs b/packages/router/tests/via_ssr/link.rs index 2075d632ac..58c64e8d5c 100644 --- a/packages/router/tests/via_ssr/link.rs +++ b/packages/router/tests/via_ssr/link.rs @@ -1,19 +1,13 @@ use dioxus::prelude::*; use dioxus_history::{History, MemoryHistory}; use dioxus_router::components::HistoryProvider; -use std::{rc::Rc, str::FromStr}; +use std::rc::Rc; -fn prepare() -> String -where - ::Err: std::fmt::Display, -{ +fn prepare() -> String { prepare_at::("/") } -fn prepare_at(at: impl ToString) -> String -where - ::Err: std::fmt::Display, -{ +fn prepare_at(at: impl ToString) -> String { let mut vdom = VirtualDom::new_with_props( App, AppProps:: { @@ -46,10 +40,7 @@ where } #[allow(non_snake_case)] - fn App(props: AppProps) -> Element - where - ::Err: std::fmt::Display, - { + fn App(props: AppProps) -> Element { rsx! { h1 { "App" } HistoryProvider {