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

Integrating with actix-web #72

Open
xrl opened this issue Mar 15, 2022 · 3 comments
Open

Integrating with actix-web #72

xrl opened this issue Mar 15, 2022 · 3 comments

Comments

@xrl
Copy link

xrl commented Mar 15, 2022

Hi, great library, I'm enjoying it. I would like to somehow support the ? operator in my actix-web app. The actix-web HTTP handler's support a Result<_,_> return value so that means ? will work, but the Err variant needs to implement their ResponseError trait: https://github.com/actix/actix-web/blob/master/actix-web/src/error/response_error.rs .

I can't implement ResponseError for eyre::Result because of the orphan rule. Any tips on how I could use eyre::Result in this scenario? And food for thought, here is how I hacked some existing code to use the stdlib Result in actix-web:

use actix_web::ResponseError;
use std::fmt::{Display, Formatter};

#[derive(Debug)]
pub struct WebError {}

impl Display for WebError {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
        write!(f, "WebError{{}}")
    }
}

impl ResponseError for WebError {}

#[get("/exports/{id}/run")]
pub async fn exports_run(id: Identity, req: HttpRequest) -> Result<HttpResponse, WebError> {
    let identity = id.identity().unwrap();

    let pg_conn = pg_conn();

    let id = req.match_info().get("id").unwrap();
    let id: i32 = id.parse().map_err(|_| WebError {})?;

    let job: ExportJob = export_jobs::table
        .filter(export_jobs_dsl::identity.eq(&identity))
        .filter(export_jobs_dsl::id.eq(id))
        .first::<ExportJob>(&pg_conn)
        .map_err(|_| WebError {})?;

    let athena_query = sparkle_athena::StreamableAthenaResult {
        athena_database: job.athena_database,
        athena_view: job.athena_view,
    };

    let csv = execute_query(athena_query).await.map_err(|_| WebError {})?;

    let mut oauth2_token = oauth_tokens_dsl::oauth2_tokens
        .filter(oauth_tokens_dsl::identity.eq(&identity))
        .first::<OAuth2Token>(&pg_conn)
        .map_err(|_| WebError {})?;
    oauth2_token
        .refresh(&pg_conn)
        .await
        .map_err(|_| WebError {})?;


    Ok(HttpResponse::Ok().body("not implemented yet"))
}
@yaahc
Copy link
Collaborator

yaahc commented Mar 15, 2022

Hi, great library, I'm enjoying it.

Thank you and I'm glad. Hearing that puts a smile on my face ^_^

Any tips on how I could use eyre::Result in this scenario? And food for thought, here is how I hacked some existing code to use the stdlib Result in actix-web:

The standard approach is to use a feature flag to implement the given trait in one crate or the other. It looks like someone already requested1 support for anyhow in actix-web and got denied, so having the trait implementation live in actix-web is likely out of the question, tho I suppose it would be worth double checking if the maintainer involved in that decision is still the lead maintainer or not. Edit: it seems that they are not infact involved anymore

Looking at ResponseError it looks like it's meant to solve the same problems I'm trying to solve with generic member access, though it does so statically rather than dynamically which has a different set of tradeoffs. Either way though it wouldn't work with eyre because of limitations in rust's trait system that currently prevent eyre from implementing Error, even if we could convince actix-web to switch back to the standard library Error trait rather than requiring their own.

In the end it looks like this is a rather large mess. I can understand why actix decided to define their own Error trait but doing so makes them fundamentally incompatible with the rest of the ecosystem's error handling, so I can't immediately think of any good solutions for using eyre in that environment. It might be that we can convince them to switch back to the Error trait when generic member access is stabilized, and I would certainly love to get their feedback on that feature to know whether or not it would be an acceptable solution for their usecases, but that's only one of the problems to solve here. I'm sorry I don't have better news >_<.

Footnotes

  1. https://github.com/actix/actix-web/pull/1241

@yaahc
Copy link
Collaborator

yaahc commented Mar 15, 2022

It actually looks like the maintainer who closed that PR isn't the one maintaining actix-web anymore, so I think it's almost certainly worth pinging @robjtede here to get their opinion and to see if we could start working on migrating actix-web back to the standard library error trait.

@mickdekkers
Copy link

For those looking for a solution to use in the meantime, this pattern combining eyre with thiserror works quite well:

#[derive(thiserror::Error, Debug)]
pub enum RouteNameHereError {
    #[error("{0}")]
    ValidationError(String),
    #[error(transparent)]
    UnexpectedError(#[from] eyre::Error),
}

impl ResponseError for RouteNameHereError {
    fn status_code(&self) -> StatusCode {
        match self {
            RouteNameHereError::ValidationError(_) => StatusCode::BAD_REQUEST,
            RouteNameHereError::UnexpectedError(_) => StatusCode::INTERNAL_SERVER_ERROR,
        }
    }
}

#[get("/route-name-here")]
async fn route_name_here() -> Result<(), RouteNameHereError> {
    Err(eyre!("Something went wrong!"))
}

Source: https://www.lpalmieri.com/posts/error-handling-rust/#using-anyhow-as-opaque-error-type

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

3 participants