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

Improve top-level error handling #526

Open
1 task done
kflansburg opened this issue Apr 2, 2024 · 3 comments
Open
1 task done

Improve top-level error handling #526

kflansburg opened this issue Apr 2, 2024 · 3 comments

Comments

@kflansburg
Copy link
Member

Is there an existing issue for this?

  • I have searched the existing issues

Description

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

@DylanRJohnston
Copy link
Contributor

DylanRJohnston commented Apr 3, 2024

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.

@Jasper-Bekkers
Copy link
Contributor

Jasper-Bekkers commented Apr 15, 2024

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.

@kflansburg
Copy link
Member Author

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

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