workers-rs icon indicating copy to clipboard operation
workers-rs copied to clipboard

Improve top-level error handling

Open kflansburg opened this issue 10 months ago • 3 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Description

Should we just accept anyhow::Error or something else generic?

kflansburg avatar Apr 02 '24 11:04 kflansburg

I'd just like to throw in my 5 cents while you're considering error handling. With the new axum support, the lack of implementation of IntoResponse for worker::Error presents a bit of an awkward usability problem when working with axum.

I currently have this newtype wrapper to work around and preserve the ergonomics of the ? operator.

pub struct ErrWrapper(worker::Error);

impl From<worker::Error> for ErrWrapper {
    fn from(value: worker::Error) -> Self {
        ErrWrapper(value)
    }
}

impl IntoResponse for ErrWrapper {
    fn into_response(self) -> axum::response::Response {
        (StatusCode::INTERNAL_SERVER_ERROR, self.0.to_string()).into_response()
    }
}

pub type Result<T> = StdResult<T, ErrWrapper>;

#[axum::debug_handler]
#[worker::send]
async fn create_game(
    State(env): State<Env>,
    req: Request<axum::body::Body>,
) -> Result<worker::HttpResponse> {
    let game_code = generate_game_code();

    Ok(env
        .durable_object("GAME")?
        .id_from_name(game_code.deref())?
        .get_stub()?
        .fetch_with_request(req.try_into()?)
        .await?
        .try_into()?)
}

The examples in the repo just unwrap the error which panics, however if I understand correctly wasm_bindgen will turn any Err in a Result into an exception anyway, so if you just keep propagating the Err to the caller there isn't any difference.

However when trying to use APIs like the Durable Object API inside an axum handler, if you'd like middleware or other tower service's to work with the Result you definitely don't want it to panic right away.

Although perhaps there isn't a canonical implementation of IntoResponse for worker::Err as it may not cleanly map onto http status codes. I'm currently using the brute force approach of always turning it into a 500 Internal Server Error.

DylanRJohnston avatar Apr 03 '24 09:04 DylanRJohnston

Should we just accept anyhow::Error or something else generic?

Can't it just take a Result<T, Box<dyn std::error::Error>> in that case users can just use anyhow or thiserror or their own error types.

Jasper-Bekkers avatar Apr 15 '24 19:04 Jasper-Bekkers

Yes, I believe the current consensus is that the return type is something like that where T: Into<web_sys::Response>.

kflansburg avatar Apr 15 '24 19:04 kflansburg