obsidian-rs / obsidian

Ergonomic async http framework for reliable and efficient web
MIT License
26 stars 4 forks source link

Proposal for better error handling #61

Open jk-gan opened 4 years ago

jk-gan commented 4 years ago

1. Early return Error

Question: How do we know what response content-type to be returned by the Handler if user using ? to early return Error?

Currently, the Error will always return String which might not be what users expecting if they are developing json API.

Note: for Obsidian 0.2.x, the user only can handle this by manually write Error response instead of early return using ?.

From

pub async fn create_user(mut ctx: Context) -> ContextResult {
    #[derive(Serialize, Deserialize)]
    struct User {
        name: String,
        age: i8,
    }

    let user: User = match ctx.json().await {
        Ok(body) => body,
        Err(error) => {
            return ctx
                .build_json(error)
                .with_status(StatusCode::INTERNAL_SERVER_ERROR)
                .ok()
        }
    };

    // create user logic here

    ctx.build_json(user)
        .with_status(StatusCode::CREATED)
        .ok()
}

To

pub async fn create_user(mut ctx: Context) -> ContextResult {
    #[derive(Serialize, Deserialize)]
    struct User {
        name: String,
        age: i8,
    }

    let user: User = ctx.json().await?; // this will return a response with json content-type if Error

    // create user logic here

    ctx.build_json(user)
        .with_status(StatusCode::CREATED)
        .ok()
}

Expected response

// Http Status Code: 400
{"error":"'name' is required"}

The suggestion section is a work-in-progress

Suggestion

1. Trait

To allow the user to control error handling globally, we can provide an ErrorResponse trait.

pub trait ErrorResponse {
    fn respond_to(&self) -> Response;
}

impl ErrorResponse for SomeError {
    fn respond_to(&self) -> Response {
        // generate Response here
    }
}

2. Middleware

We can have a EarlyReturnErrorMiddleware to handle it:

#[async_trait]
impl Middleware for EarlyReturnErrorMiddleware {
    async fn handle<'a>(
        &'a self,
        context: Context,
        ep_executor: EndpointExecutor<'a>,
    ) -> ContextResult {
        if matches!(ep_executor.next(context).await, Err(error)) {
            // handle error   
        }
    }
}

3. Annotation

#[response(type="json")]
async fn get_user(ctx: Context) -> ContextResult {}
jk-gan commented 4 years ago

2. Third Pary Error Type

Question: How do we support different Error type returned by different crate and return in the format users are expection?

Currently, we're expecting ObsidianError to be returned by the handler if an error occurs. However, we can't always know the Error type to be returned by different crates. It will be compilation error. This is annoying.

Technically we can allow any Error which impl std::error::Error trait to be returned in Handler, but we need an ergonomic way to allow users to modify the error response pattern as well.

Suggestion

1. Trait

To allow the user to control error handling globally, we can provide an ErrorResponse trait. All the Error return in Handler must impl this trait.

pub trait ErrorResponse {
    fn respond_to(&self) -> Response;
}

impl ErrorResponse for SomeError {
    fn respond_to(&self) -> Response {
        // generate Response here
    }
}

2. Using From trait

I would suggest we convert ObsidianError to become a struct to hold more data, or let Obsidian::ThirdPartyError to be an enum struct:

impl From<SomeError> for ObsidianError {
    fn from(error: SomeError) -> Self {
        ObsidianError::ThirdPartyError { crate: "some-crate", error }
    }
}
jk-gan commented 4 years ago

3. Error status code

Question: How do we support different http status code if the handler return Err(error)?

Currently, the endpoint_resolver will return StatusCode::INTERNAL_SERVER_ERROR(500) if the handler return Err(error). However, our framework should allow user to set proper status code based on the error: 4xx for client error and 5xx for server error.

jk-gan commented 4 years ago

4. Can't get Context data when the Handler return Err(error)

Question: How do we get Context data in the middleware after the Handler return Err(error)?

Currently, the context is moved into handler so there is no way to get the context if handler return Err(error). The return type for handler is pub type ContextResult<T = ObsidianError> = Result<Context, T>;.

jk-gan commented 4 years ago

Just found this crate (anyhow), let me try to use it for Error handling.

jk-gan commented 4 years ago

this is good reference too

jk-gan commented 4 years ago

99436742_3232543743423768_749704812307677184_o

Error will have to impl IntoErrorResponse trait in order to be handled by the framework. This is my initial thought:

pub trait IntoErrorResponse: fmt::Debug + fmt::Display {
    /// convert Error into error response
    fn into_error_response(&self) -> Result<Response<Body>, http::Error> {
        let body = Body::from(self.to_string());
        Response::builder().status(self.status_code()).body(body)
    }

    /// status code for this error response
    /// this will return Internal Server Error (500) by default
    fn status_code(&self) -> StatusCode {
        StatusCode::INTERNAL_SERVER_ERROR
    }
}

#[derive(Debug)]
pub struct ObsidianError {
    inner: Box<dyn IntoErrorResponse>,
}

impl ObsidianError {
    pub fn into_response(&self) -> Result<Response<Body>, http::Error> {
        self.into_error_response()
    }
}

impl<T: IntoErrorResponse + 'static> From<T> for ObsidianError {
    fn from(err: T) -> Self {
        ObsidianError {
            inner: Box::new(err),
        }
    }
}