rpc middleware: return Self::Response instead of Result<Self::Response, Self::Error>

[niklasad1]

I guess the point of Error = Infallible here is that any "error" should be returned in the form of a MethodResponse to a client, and so it enforces that all errors are converted into proper JSON-RPC errors or whatever before they are emitted. Am I understanding that right?

(A bit like how in HTTP servers like Axum, any errors need to be converted to HTTP responses)

I geuss the reason to keep the Error associated type at all is just for internal middleware that might want to signal something went wrong, and then some other middleware that wraps it can take that error and decide what sort of response to turn it into? I think this makes sense, but I wonder if we have any examples that show this is useful? (Or maybe I am missing something more obvious!)

Originally posted by @jsdw in #1521 (comment)

TLDR; The question whether it is most ergonomic to have:

pub trait RpcServiceT {
	/// The error type.
	type Error: std::fmt::Debug;

	/// The response type
	type Response: ToJson;

	/// Processes a single JSON-RPC call, which may be a subscription or regular call.
	fn call<'a>(&self, request: Request<'a>) -> impl Future<Output = Result<Self::Response, Self::Error>> + Send + 'a;
}

or

pub trait RpcServiceT {
	/// The response type
	type Response: ToJson;

	/// Processes a single JSON-RPC call, which may be a subscription or regular call.
	fn call<'a>(&self, request: Request<'a>) -> impl Future<Output = Self::Response> + Send + 'a;
}

[jsdw]

I don't know the answer to this (but am always allured by the idea of just one associated type because it feels more flexible).

We could also alter the ToJson trait to do whatever we want here, eg could call it ToMethodResponseJson and impl it for Result<T: ToMethodResponseJson,E> for instance (perhaps it would return Optiom<MethodResponseJson> or whatever. Depends what we actually need to support (and how we want to handle it in the error case).

[niklasad1]

Really that ToJson is not strictly needed but we need it for our RpcLoggerMiddleware, so we could do something else and remove it complete as trait bound here.

To re-phrase if we decide to split up the trait to different response types for batches, calls and notifications it will certainly be much more ergonomic to remove the error type.

Example design:

pub trait RpcServiceT {
	/// The response type
	type NotifResonse;
        type MethodResponse;
        type BatchResponse;

	fn call<'a>(&self, request: Request<'a>) -> impl Future<Output = Self::MethodResponse> + Send + 'a;
        fn batch<'a>(&self, batch: Batch<'a>) -> impl Future<Output = Self::BatchResponse> + Send + 'a;
        fn notification<'a>(&self,n: Notification<'a>) -> impl Future<Output = Self::NotifResponse> + Send + 'a;
}