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

Use associated type instead of eyre for error message #69

Merged
merged 1 commit into from
May 25, 2023
Merged

Use associated type instead of eyre for error message #69

merged 1 commit into from
May 25, 2023

Conversation

iamsauravsharma
Copy link
Contributor

@iamsauravsharma iamsauravsharma commented May 25, 2023

Use thiserror instead of eyre so error can be handled by library user. https://docs.rs/eyre/latest/eyre/#comparison-to-thiserror for more information Use trait type instead of using eyre for error message

Copy link
Contributor

@epilys epilys left a comment

Choose a reason for hiding this comment

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

This is useful because I also hit this problem when I was writing some stuff with axum-login

axum-login/src/error.rs Outdated Show resolved Hide resolved
@maxcountryman
Copy link
Owner

What about allowing the user to supply their own error type via the trait directly? Something like this:

diff --git a/axum-login/src/user_store.rs b/axum-login/src/user_store.rs
index 4ea9f1d..4bf6e6c 100644
--- a/axum-login/src/user_store.rs
+++ b/axum-login/src/user_store.rs
@@ -1,6 +1,6 @@
 use async_trait::async_trait;

-use crate::{AuthUser, Result};
+use crate::AuthUser;

 /// A trait which defines a method that allows retrieval of users from an
 /// arbitrary backend.
@@ -12,6 +12,9 @@ where
     /// An associated user type which will be loaded from the store.
     type User: AuthUser<UserId, Role>;

+    /// An associated error type.
+    type Error: std::error::Error;
+
     /// Load and return a user.
     ///
     /// This provides a generic interface for loading a user from some store.
@@ -19,5 +22,8 @@ where
     /// unique, stable identifier of the user is available. See [`AuthUser`]
     /// for expected minimal interface of the user type itself.
     #[must_use]
-    async fn load_user(&self, user_id: &UserId) -> Result<Option<Self::User>>;
+    async fn load_user(
+        &self,
+        user_id: &UserId,
+    ) -> std::result::Result<Option<Self::User>, Self::Error>;
 }

@iamsauravsharma iamsauravsharma changed the title Use thiserror instead of eyre for error message Use associated type instead of eyre for error message May 25, 2023
@iamsauravsharma
Copy link
Contributor Author

I have added support for associated type Error without using both thiserror and eyre

@maxcountryman
Copy link
Owner

Thanks. Do you mind also updating the change log?

Signed-off-by: Saurav Sharma <appdroiddeveloper@gmail.com>
@iamsauravsharma
Copy link
Contributor Author

I have added changelog under unreleased header for PR

@maxcountryman maxcountryman merged commit acca6d8 into maxcountryman:main May 25, 2023
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.

3 participants