Allow any type that implements FromServerFnError as a replacement of the ServerFnError in server_fn #3274
Conversation
This reverts commit fd68c76.
ryo33
changed the title
|
@nicolas-guichard Hi, this fix is similar to #3253, but takes a different approach, addressing #3153 and #3155. It would be great if you could take a look and review whether this approach works. |
| /// Converts the custom error type to a [`String`]. Defaults to serializing to JSON. | ||
| fn ser(&self) -> String { | ||
| serde_json::to_string(self).unwrap_or_else(|e| { | ||
| serde_json::to_string(&Self::from_server_fn_error( | ||
| ServerFnErrorErr::Serialization(e.to_string()), | ||
| )) | ||
| .expect( | ||
| "error serializing should success at least with the \ | ||
| Serialization error", | ||
| ) | ||
| }) | ||
| } | ||
|
|
||
| /// Deserializes the custom error type from a [`&str`]. Defaults to deserializing from JSON. | ||
| fn de(data: &str) -> Self { | ||
| serde_json::from_str(data).unwrap_or_else(|e| { | ||
| ServerFnErrorErr::Deserialization(e.to_string()).into_app_error() | ||
| }) | ||
| } |
There was a problem hiding this comment.
Now, custom error types have a default implementation for error serialization using serde_json. ServerFnError overrides it by using FromStr and Display for backward compatibility.
| #[derive(Debug, Clone, Display, Serialize, Deserialize)] | ||
| pub enum MyErrors { | ||
| InvalidArgument(InvalidArgument), | ||
| ServerFnError(ServerFnErrorErr), | ||
| Other(String), | ||
| } | ||
|
|
||
| impl From<InvalidArgument> for MyErrors { | ||
| fn from(value: InvalidArgument) -> Self { | ||
| MyErrors::InvalidArgument(value) | ||
| } | ||
| } | ||
|
|
||
| impl From<String> for MyErrors { | ||
| fn from(value: String) -> Self { | ||
| MyErrors::Other(value) | ||
| } | ||
| } | ||
|
|
||
| impl FromServerFnError for MyErrors { | ||
| fn from_server_fn_error(value: ServerFnErrorErr) -> Self { | ||
| MyErrors::ServerFnError(value) | ||
| } | ||
| } |
There was a problem hiding this comment.
This is the new way to define a custom error type for server fns. It can used directly in the signature of server fn like -> Result<(), MyErrors> instead of -> Result<(), ServerFnError<MyErrors>.
|
Thanks for this work! While I have not had time to manually test it, and have not reviewed every line, this looks like it is the right approach, and it certainly seems to make custom errors more usable, which I think is great. It comes at a slightly awkward time in our release cycle as we are planning a 0.7 release this weekend and this makes some breaking changes that won't have time to be tested in the beta/rc process before that release. So here's my suggestion:
|
|
Thank you for your feedback! That release schedule is reasonable to me. |
Closes #3270
Fixes #3153
Addresses #3155
FromServerFnErrorServerFnError::WrappedServerErrorvariant and manual error conversion helpersServerFnErrorErr::WrappedServerErrorandimpl From<ServerFnError> for ServerFnErrorErrserver_fn_macrosto use theFromServerFnErrortraitServerFnErrorErr::Middlewarefor a better context of the errorsexamples/server_fn_axumcargo +nightly make checkThe implementation is complete. The only breaking changes should be the third ones mentioned above, and other behaviors should remain retained as expected.