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

Result handling #73

Open
lucasavila00 opened this issue Aug 4, 2024 · 2 comments
Open

Result handling #73

lucasavila00 opened this issue Aug 4, 2024 · 2 comments

Comments

@lucasavila00
Copy link
Contributor

lucasavila00 commented Aug 4, 2024

I would like if qubit handled Results for me.

I'm using a database for the queries and every operation returns a Result.

I would like if qubit:

  • Did not send the Err contents to the client-side
  • Had a compile time setting that forced every handler to return a QubitResult
  • Had an option on the client to throw on Errs, generates types for the successful Ok only

For now I'm using a custom result type:

use std::error::Error;
use std::sync::Arc;

use serde::Serialize;
use ts_rs::TS;
#[derive(Debug, Clone)]
pub enum RpcErrorMessage {
    Anyhow(Arc<anyhow::Error>),
    Public(String),
}

impl Serialize for RpcErrorMessage {
    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
    where
        S: serde::Serializer,
    {
        let msg = match self {
            RpcErrorMessage::Anyhow(e) => {
                dbg!(e);
                "Internal Server Error"
            }
            RpcErrorMessage::Public(s) => s,
        };
        serializer.serialize_str(msg)
    }
}

#[derive(Debug, Clone, Serialize, TS)]
pub struct RpcError {
    #[ts(as = "String")]
    message: RpcErrorMessage,
}

fn app_anyhow<T: Error + Send + Sync + 'static>(it: T) -> RpcError {
    RpcError {
        message: RpcErrorMessage::Anyhow(Arc::new(anyhow::Error::new(it))),
    }
}

impl<T: Error + Send + Sync + 'static> From<T> for RpcError {
    fn from(value: T) -> Self {
        app_anyhow(value)
    }
}
pub fn public_err<T>(msg: &str) -> Result<T, RpcError> {
    Err(RpcError {
        message: RpcErrorMessage::Public(msg.to_string()),
    })
}

pub type RpcResult<T> = Result<T, RpcError>;

Usage:

#[handler(query)]
async fn read_posts(ctx: AuthCtx) -> RpcResult<Vec<Post>> {
    use crate::schema::posts::dsl::*;
    let results = posts
        .limit(5)
        .select(Post::as_select())
        .load(&mut ctx.pg_pool.get()?)?;

    Ok(results)
}

And an unwrap function in TS

type AnyResult = { Ok: any } | { Err: { message: string } };

type ExtractOk<T extends AnyResult> = T extends { Ok: infer U } ? U : never;

export const Unwrap = <T extends AnyResult>(x: T): ExtractOk<T> => {
    if ("Ok" in x) {
        return x.Ok;
    }
    if ("Err" in x) {
        if ("message" in x.Err) {
            throw new Error(x.Err.message);
        }
    }
    console.log(x);
    throw new Error("Unwrap failed: " + String(x));
};

Thanks!

@andogq
Copy link
Owner

andogq commented Aug 12, 2024

Thanks for the idea! I've felt that there could be an improvement to the error handling model in Qubit but I haven't quite been able to work out what feels good.

Up until this point, I've been taking inspiration from both Axum and jsonrpsee's handlers. In both of these libraries, they don't really do anything super special with errors outside of what their underlying protocols (HTTP/JSONRPC) supply. For example, Axum doesn't really care what you do as long as you return a status code/headers/response/etc. jsonrpsee will allow a Result to be returned, however the error component must be their RpcError.

I do like the idea of upgrading the handlers to return like so:

  • Result<T, QubitError>: Special case, will generate return type for T and throw on the client if QubitError is returned
  • Result<T, E>: Could be returned like normal? Or could cause client to throw - Would need to decide
  • T: Any other type, serialised and returned as normal

There are also some other potential ways of achieving this without introducing special behaviour for results into Qubit:

Create an application error using anyhow/thiserror/enums, which should contain all appropriate user-facing error messages (friendly, no sensitive information), and handle the transformation from internal error types (eg DB errors). Have your handlers return Result<T, ApplicationError>, which should enable you to use ? syntax as you like.

Using client plugins (currently quite new and undocumented, but they provide a way to add additional commands to your chain), add an unwrap method which could look like this:

api.read_posts.unwrapQuery()

This unwrap method can query the API as normal, but perform your unwrapping logic. Given that the plugin definition is correct, the type of the Ok value will be passed through, just like a regular unwrap in Rust.


I hope these options make sense, I'm interested in your opinion on the two, or if you've got any other ideas on how to implement this.

@lucasavila00
Copy link
Contributor Author

I do like the idea of upgrading the handlers to return like so:

    Result<T, QubitError>: Special case, will generate return type for T and throw on the client if QubitError is returned
    Result<T, E>: Could be returned like normal? Or could cause client to throw - Would need to decide
    T: Any other type, serialised and returned as normal

Perfect.

For me QubitError should implement the ? support, where it defaults to trace::warning the content, and returning "Internal Error" to the client.

There should also be a QubitError::Public("...") where public-facing non-structured errors can be shown.

And on the client there would be no changes, since the QubitError would become a 500-status HTTP response.

I have worked with other frameworks (eg: https://www.meteor.com/) where this is standard behavior and I found it very intuitive, and I replicate this with my RPC framework things too.

Structured errors or more complex handling would be out-of-scope.

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

No branches or pull requests

2 participants