github / twirp-rs

Twirp RPC for Rust
MIT License
20 stars 2 forks source link

Middleware errors are hard to use #16

Open ctian1 opened 4 months ago

ctian1 commented 4 months ago

Right now middleware handle function is expected to return twirp::ClientError which makes it hard to return any special middleware errors. Maybe there can be another option in the enum such as ClientError::Middleware(String)?

tclem commented 4 months ago

That's true. Do you have a motivating example or use case?

MatteoJoliveau commented 5 days ago

@tclem I have a use case: we're writing a CLI tool that sends Twirp requests to our backend. This CLI authenticates the user against our company SSO and stores the access token in the system keyring.

We wrote a small middleware to load the access token from the keyring and add it to Twirp requests, and we need to return a custom error to the user if reading the token fails or it's not found (as this requires the user to re-start the login flow).

Example code:

  #[async_trait]
    impl<T: TokenStore + Send + Sync + 'static> Middleware for SetToken<T> {
        async fn handle(&self, mut req: Request, next: Next<'_>) -> client::Result<Response> {
            // I would have preferred to map and return the error, but Twirp doesn't support a generic user error variant
            if let Some(token) = self
                .token_store
                .access_token()
                .expect("failed to load access token from store")
            {
                req.headers_mut().insert(header::AUTHORIZATION, token.try_into().unwrap());
            }

            next.run(req).await
        }
    }

We have resorted to panicking since we currently cannot return a custom ClientError, but the UX is less then optimal :c

tclem commented 2 days ago

Here's a very rough proposal: https://github.com/github/twirp-rs/pull/67!