Skip to content

Conversation

@ealmloff
Copy link
Member

@ealmloff ealmloff commented Mar 11, 2025

This PR changes the router to throw a url parsing error into the context instead of panicing when it fails to parse the current url. On the server, it changes fullstack to return a 404 status if the route is not found or a 500 status if there is another error in the root context

Partially addresses #2739 with better default status codes

@ealmloff ealmloff added router Related to the router implementation fullstack related to the fullstack crate labels Mar 11, 2025
@ealmloff ealmloff marked this pull request as ready for review March 12, 2025 14:05
@ealmloff ealmloff requested a review from a team as a code owner March 12, 2025 14:05
Copy link
Member

@jkelleyrtp jkelleyrtp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!! I wonder if (for spas) we should render just a blank page for 404 routes or perhaps lint that the router doesn't have a 404 route enabled? But definitely the default behavior should be an error.

Just the clippy test (windows might just be failing now) and some docs and it's good to go!

Also - this doesn't completely address throwing a http::ErrorCode::MY_CODE does it?

@ealmloff
Copy link
Member Author

Also - this doesn't completely address throwing a http::ErrorCode::MY_CODE does it?

No, that will be in a follow up PR. It requires more integration with suspense to only send the first chunk down after the suspense boundary the router is in has been resolved

@ealmloff
Copy link
Member Author

Awesome!! I wonder if (for spas) we should render just a blank page for 404 routes or perhaps lint that the router doesn't have a 404 route enabled? But definitely the default behavior should be an error.

Maybe in the CLI? Figuring out if we are running in a spa might be difficult. You could nest dioxus-web inside of an axum router that handles the 404

@ealmloff ealmloff added the breaking This is a breaking change label Mar 13, 2025
@jkelleyrtp jkelleyrtp merged commit 09a4b5e into DioxusLabs:main Mar 14, 2025
16 of 17 checks passed
AnteDeliria pushed a commit to AnteDeliria/dioxus that referenced this pull request Jun 2, 2025
* return a 404 when paring the route fails

* add a playwright test for fullstack routing status codes

* Fix new playwright test

* document SSRError

* Make SSRError public and fix clippy

* Move the error display bound to the routable trait

* fix clippy

* return a more generic 404 message

* fix router tests
AnteDeliria pushed a commit to AnteDeliria/dioxus that referenced this pull request Jul 23, 2025
* return a 404 when paring the route fails

* add a playwright test for fullstack routing status codes

* Fix new playwright test

* document SSRError

* Make SSRError public and fix clippy

* Move the error display bound to the routable trait

* fix clippy

* return a more generic 404 message

* fix router tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking This is a breaking change fullstack related to the fullstack crate router Related to the router implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants