#avoid sending error message as response in `axum`

154 messages · Page 1 of 1 (latest)

gritty beacon
#

if i fail to layer an extension, i get this message in the response body with a 500 error

Missing request extension: Extension of type `sqlx_core::pool::Pool<sqlx_sqlite::database::Sqlite>` was not found. Perhaps you forgot to add it? See `axum::Extension`.

i instead want to just log it and send an empty response body.

async fn main() -> Result<(), anyhow::Error> {
    // ...
    let pool = SqlitePool::connect(&database_url).await?;

    let app = Router::new()
        .route("/", get(home))
        // some other routes ...
        .layer(Extension(pool)) // if i remove this line
        ;

    // ...
}
pseudo mantle
#

Use this

#

Or a middleware I guess

gritty beacon
# pseudo mantle https://docs.rs/axum-extra/latest/axum_extra/extract/struct.WithRejection.html

someone mentioned that it is possible to use State instead of extensions for things like database pools.
now i have another question.
i also have a middleware that assigns a unique request id to each request that the handlers can access as RequestId

pub struct RequestId(String);
impl RequestId {
    pub fn new() -> Self {
        Self(Uuid::new_v4().to_string())
    }
}


let app = Router::new()
        .route("/", get(home))
        // some other routes ...
        .layer(Extension(pool))
        .layer(axum::middleware::from_fn(
            |mut request: Request<Body>, next: Next| async {
                let request_id = RequestId::new();
                request.extensions_mut().insert(request_id);
                next.run(request).await
            },
         ));
        

pub async fn foo(
    Extension(request_id): Extension<RequestId>,
    Extension(pool): Extension<SqlitePool>,
) -> Result<StatusCode, HandlerError> { ... }

should that be state or extension?

pseudo mantle
gritty beacon
gritty beacon
pseudo mantle
#

I mean

#

state is generally for stuff that is constant across requests

#

it’s like, mildly tricky to get it to be variable

#

not worth it

pseudo mantle
#

it’s kind of cursed in comparison to WithRejection

#

but honestly axum error handling is cursed in general so

gritty beacon
# pseudo mantle it’s kind of cursed in comparison to `WithRejection`

so i have to do this?

struct RejectionHandler { ... }

pub async fn foo(
    WithRejection(request_id, _): WithRejection<RequestId, RejectionHandler>,
) -> Result<StatusCode, HandlerError> { ... }

instead of this?

pub async fn foo(
    Extension(request_id): Extension<RequestId>,
) -> Result<StatusCode, HandlerError> { ... }
pseudo mantle
#

or you could implement FromRequestParts for RequestId directly

#

to simplify things

#

then just make your own rejection type

gritty beacon
pseudo mantle
#

RequestId<RequestId> is not a type you’d want to write

# 
request_id: RequestId,
#

is the way to do it if RequestIs: FromRequestParts

gritty beacon
pseudo mantle
#

what type are you imagining in RequestId

# 
pub struct RequestId(pub /* what goes here…? */);
#

like that type I replaced with a comment is the type of request_id

gritty beacon
pseudo mantle
#

Okay so then why are you destructuring it in the function args

# 
request_id: RequestId,
#

if you just do this, you have request_id be of type RequestId

#

You could do RequestId(request_id) if you wanted request_id to be of type String

#

but why would you want that

gritty beacon
#

oh ok, the inner String will be encapsulated.

gritty beacon
#

and preferable

#

so is this a good idea?

so i have to write a middleware that create a RequestId and set it as a header value, say, X-REQUEST-ID, write a custom extractor to be used like this?

pseudo mantle
#

yeah sure

gritty beacon
#

and what happens if the request id is not set?

#

because this the thing i'm trying to avoid at the end

having internal details leaked as response body

Missing request extension: Extension of type `sqlx_core::pool::Pool<sqlx_sqlite::database::Sqlite>` was not found. Perhaps you forgot to add it? See `axum::Extension`.
pseudo mantle
#

well you’re writing the extractor

#

you can panic, return a bare 500

#

idk

#

what do you want to do

gritty beacon
#

oh right i can handle that situation

#[async_trait]
impl<S> FromRequestParts<S> for RequestId {
    type Rejection = HandlerError;
    async fn from_request_parts(parts: &mut Parts, _: &S) -> Result<Self, Self::Rejection> { ... }
}
gritty beacon
# pseudo mantle what do you want to do

this request_id will potentially be used in every handler function. and if there is any issue with getting the RequestId the whole app might start rejecting requests. is this a good idea?

or is it the case where the request_id is guaranteed to be always set?

pseudo mantle
#

like, if it’s with a middleware then it’ll be guaranteed to be set if the middleware is guaranteed to set it

gritty beacon
pseudo mantle
#

then yeah, as long as the middleware doesn’t set it only conditionally it’s fine

gritty beacon
# pseudo mantle then yeah, as long as the middleware doesn’t set it only conditionally it’s fine

the reason i'm doing this is because i want to return the request id in the json response when there is an error. so the users can raise a support ticket with that request id.

{
  "datetime": "2024-10-14T13:54:14.348240919Z",
  "error": "...",
  "request_id": "4af4d2bb-283e-4c83-85a3-c916621b3e03"
}

i also setup tracing span to show that request id in the logs

        .layer(
            TraceLayer::new_for_http().make_span_with(|request: &Request<_>| {
                let request_id = request
                    .extensions()
                    .get::<RequestId>()
                    .cloned()
                    .unwrap_or_else(|| {
                        tracing::warn!("unable to get RequestId extension when making span");
                        RequestId::unknown()
                    });

                tracing::info_span!(
                    "request",
                    request_id = %request_id,
                    method = %request.method(),
                    uri = %request.uri(),
                )
            }),
        )

2024-10-14T13:54:11.000543Z  INFO request{request_id=4395866c-ea1b-4e0c-ab5c-fbc99924db4e method=POST uri=/login}:login{username="zahash" remember=true}: server::login: user_id=1

2024-10-14T13:54:11.604185Z  INFO request{request_id=4395866c-ea1b-4e0c-ab5c-fbc99924db4e method=POST uri=/login}:login{username="zahash" remember=true}: server::login: session created session_id="***" created_at=2024-10-14 13:54:11.596599469 +00:00:00 expires_at=2024-11-13 13:54:11.596599469 +00:00:00 user_agent=Thunder Client (https://www.thunderclient.com)

2024-10-14T13:54:14.348198Z  INFO request{request_id=4af4d2bb-283e-4c83-85a3-c916621b3e03 method=POST uri=/login}: server::error: Auth(UserNotFound("asdf"))
gritty beacon
pseudo mantle
#

there’s no way to guarantee that all the errors from your service are of a certain form

#

you have to map the rejections individually

#

it sucks

#

i wanted to fix it but it’s also a massive breaking change Lol

gritty beacon
pseudo mantle
gritty beacon
pseudo mantle
#

wdym

gritty beacon
#

backend library / framework

pseudo mantle
#

i mean i like axum

gritty beacon
pseudo mantle
#

I haven’t tried many others

#

well I tried Warp way back but I don’t like that one

#

warp is just cursed

gritty beacon
#

wow i almost forgot it existed

gritty beacon
#

@pseudo mantle is there a way to compile time check this?

HeaderValue::from_str("foo").unwrap()
pseudo mantle
gritty beacon
pseudo mantle
#

because if it’s non-'static it’s only known at runtime

gritty beacon
#

ah right.

gritty beacon
# pseudo mantle that doesn’t even make sense to compile-time check it

i generally don't like unwraps

        .layer(axum::middleware::from_fn(
            |mut request: Request<Body>, next: Next| async {
                let RequestId(request_id) = RequestId::new();
                request.headers_mut().insert(
                    "X-REQUEST-ID",
                    HeaderValue::from_str(&request_id)
                        .inspect_err(|e| {
                            tracing::warn!("invalid X-REQUEST-ID HeaderValue :: {:?}", e)
                        })
                        .unwrap(), // eww get away from me panic demon!!
                );
                next.run(request).await
            },
        ))
pseudo mantle
#

skill issue

#

unwraps are fine

gritty beacon
# pseudo mantle unwraps are fine

is there a way to do some type gymnastics such that the unwrap is not required?

the method expects a str that contains Only visible ASCII characters (32-127)
is there a way to enforce that at the type level? create a new type that only contains those characters? that way, the unwrap is not required?

yes the uuid also only contains only those characters but it is not "enforced"

pseudo mantle
#

don’t worry about it

gritty beacon
pseudo mantle
#

i wouldn’t

#

I would just have a function that goes Uuid → HeaderValue directly

#

and unwrap in side that function

gritty beacon
pseudo mantle
gritty beacon
pseudo mantle
#

idk which part you’re finding hard

gritty beacon
gritty beacon
pseudo mantle
#

the middleware approach is to handle all 500 errors

#

there might be others possible

#

Anyway, you’re given a Response in the middleware

#

you just have to check its status code to see if it’s 500

#

then deal with it

gritty beacon
pseudo mantle
#

it should be clear where you get the response

gritty beacon
pseudo mantle
gritty beacon
# pseudo mantle Yeah

where should i put this? at the first or at the last?

        .route("/private", get(private))
        .with_state(AppState { pool })
        .layer(
            TraceLayer::new_for_http().make_span_with(|request: &Request<_>| {
                let request_id = request
                    .extensions()
                    .get::<RequestId>()
                    .cloned()
                    .unwrap_or_else(|| {
                        tracing::warn!("unable to get RequestId extension when making span");
                        RequestId::unknown()
                    });

                tracing::info_span!(
                    "request",
                    request_id = %request_id,
                    method = %request.method(),
                    uri = %request.uri(),
                )
            }),
        )
        .layer(axum::middleware::from_fn(
            |mut request: Request<Body>, next: Next| async {
                let request_id = RequestId::new();
                request.extensions_mut().insert(request_id);
                next.run(request).await
            },
        ))
pseudo mantle
gritty beacon
pseudo mantle
#

Oh

gritty beacon
# 
        .layer(axum::middleware::from_fn(
            |mut request: Request<Body>, next: Next| async {
                let response = next.run(request).await;
                // some checks
                response
            },
        ))
pseudo mantle
#

I mean you may as well build in that code into your existing middleware

#

instead of making new middleware

#

less middleware is better

#

since you already have a from_fn, put it there

gritty beacon
pseudo mantle
#

it’s just weird to have multiple consecutive calls to middleware::from_fn

gritty beacon
pseudo mantle
#

so its lots of boxing

#

🥊

gritty beacon
# pseudo mantle because every time you all that function every route gets boxed

how to log the contents of the body? i'm only seeing this

2024-10-14T16:20:48.175703Z ERROR server: Body(UnsyncBoxBody)

        .layer(axum::middleware::from_fn(
            |request: Request<Body>, next: Next| async {
                let response = next.run(request).await;

                if response.status().is_server_error() {
                    // if internal server error, only send status code as response
                    // to avoid leaking internal details in body
                    tracing::error!("{:?}", response.body());
                    return response.status().into_response();
                }

                response
            },
        ))
pseudo mantle
#

convert it to a string using something like String::from_utf8_lossy

gritty beacon
pseudo mantle
gritty beacon
pseudo mantle
#

This kind of thing can be discovered by reading the docs

gritty beacon
#

imma be lazy sometimes

#

@pseudo mantle the docs suggest keeping limit as usize::MAX if i don't care about it.
but is it safe to do so?
what are the situations where this could backfire?

pseudo mantle
#

this is a server-supplied body; you need not worry

gritty beacon
#

@pseudo mantle how do you know so much btw? do you write rust at your job?

pseudo mantle
#

I’ve just worked with axum before

gritty beacon
pseudo mantle
gritty beacon
pseudo mantle
#

I’m learning things

#

well in theory

gritty beacon
#

oh. what degree?

pseudo mantle
#

Maths

gritty beacon
pseudo mantle
#

Well I might do that in future

#

I program as a hobby basically, which I’ve done for years now

gritty beacon
#

nice. i used to work on some boring java backend at work.
but now i mostly do terraform + aws

#

@pseudo mantle can i add you as a friend? you seem to know a lot of rust

pseudo mantle
gritty beacon