Skip to content

SubscriptionError: make more strict by enforcing T: Serialize #1555

@niklasad1

Description

@niklasad1

#1545 (comment)

Every subscription callback that returns an error we utilize this ToString impl to make it easy to use with
error types such as:

	module
		.register_subscription(
			SUB_METHOD_NAME,
			SUB_METHOD_NAME,
			UNSUB_METHOD_NAME,
			|_params, pending, _ctx, _| async move {
				let sink = pending.accept().await?;
				let json = serde_json::value::to_raw_value(&"Hello").unwrap();
				
                                // This won't work without the error implements T: ToString which we convert 
                                // to error subscription notif i.e StringError/SubscriptionError
                                sink.send(json).await?;

				Ok(())
			},
		)
		.unwrap();

Ideally we want this error type to be enforced to be serialized to JSON but makes ergonomics worse. Such as this:

--- a/core/src/error.rs
+++ b/core/src/error.rs
@@ -27,46 +27,14 @@
 use serde::Serialize;
 use serde_json::value::RawValue;
 
-#[derive(Debug)]
-pub(crate) enum InnerSubscriptionErr {
-       String(String),
-       Json(Box<RawValue>),
-}
-
 /// Error returned when a subscription fails.
-///
-/// It's recommended to use `SubscriptionErr::from_json` to create a new instance of this error
-/// because using the `String` representation may not very ergonomic for clients to parse.
 #[derive(Debug)]
-pub struct SubscriptionError(pub(crate) InnerSubscriptionErr);
-
-impl Serialize for SubscriptionError {
-       fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-       where
-               S: serde::Serializer,
-       {
-               match &self.0 {
-                       InnerSubscriptionErr::String(s) => serializer.serialize_str(s),
-                       InnerSubscriptionErr::Json(json) => json.serialize(serializer),
-               }
-       }
-}
+pub struct SubscriptionError(pub(crate) Box<RawValue>);
 
-impl<T: ToString> From<T> for SubscriptionError {
+impl<T: Serialize> From<T> for SubscriptionError {
        fn from(val: T) -> Self {
-               Self(InnerSubscriptionErr::String(val.to_string()))
-       }
-}
-
-impl SubscriptionError {
-       /// Create a new `SubscriptionErr` from a JSON value.
-       pub fn from_json(json: Box<RawValue>) -> Self {
-               Self(InnerSubscriptionErr::Json(json))
-       }
-
-       /// Create a new `SubscriptionErr` from a String.
-       pub fn from_string(s: String) -> Self {
-               Self::from(s)
+               let json = serde_json::value::to_raw_value(&val);
+               Self(json.unwrap_or_default())
        }
 }

It would be a PITA because it requires every error type returned from subscription callback to satisfy T: Serialize but would be more explicit to use.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions