Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ErrorBoundary to use miette::Diagnostic instead of Error, and various other tweaks #401

Merged
merged 10 commits into from
Feb 1, 2023

Conversation

benwis
Copy link
Contributor

@benwis benwis commented Jan 28, 2023

So my goal here has always been to have one standard, pretty page for errors, and a general way to catch errors both at the routing level and inside the app.

This PR should allow one to use one template for both kinds of Errors, and provide additional metadata. It also creates a neat(IMO) way to map between unique Rust Errors and HTTP statuscodes.

You can see it working in the todo_app_sqlite_axum example.

What's still left would be making error_template actually return the error code from the Rust error, but I wanted to get input on this app structure.

It also adds miette as a dependency, which should have a small impact on WASM size, but we should test it.

@benwis
Copy link
Contributor Author

benwis commented Jan 29, 2023

So this should be ready to merge, assuming the test passes. The todo_sqlite_axum example has been updated to show a more complete error handling scheme, and there are still some convenience changes to unify Error handling and clear up some API quirks

@gbj
Copy link
Collaborator

gbj commented Jan 30, 2023

Yeah looks good. But I think you can remove the miette dependency from leptos_dom, right?

I'm also wondering if we should abstract away from making the HashMap that's internal to Errors public: i.e., give ways to insert and remove and get an iterator, but without promising that it's backed by a HashMap? (For example given the likely small n here it's almost always more efficient to store them in a Vec<_> and do a linear search.)

This PR is already a breaking change from 0.1.3 (because the callback is RwSignal<Errors> instead of Option<RwSignal<Errors>> so if there are any other API changes we should make them now.

@benwis
Copy link
Contributor Author

benwis commented Jan 30, 2023

Forgot to save that file.

I mean we certainly could limit visibility to the HashMap, I'm just not sure I want to reimplement all or some of hashmap's methods.

I thought you wanted HydrationKey so you can do cleanup?

@gbj gbj merged commit 160f336 into leptos-rs:main Feb 1, 2023
gbj pushed a commit that referenced this pull request Mar 21, 2023
…various other tweaks (#401)

* Switch RwLock to parking_lot so they are no longer async
* cleanup todo_app_sqlite_axum
* add errors_axum example

---------

Co-authored-by: Indrazar <110272232+Indrazar@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants