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

A re-work of the error handling system #389

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kbillings
Copy link

This is my idea of error system that should work with the ideas mentioned by @yoshuawuyts
in #371 (comment) .

This system should both handle errors that have a custom IntoResponse implementation
as well as more generic errors using methods on ResultExt to create an Error type.

Some changes:

  • Rework methods on ResultExt
  • Add From<E> for Error where E: Display
  • Add back impl<T, E> IntoResponse for Result<T, E>
  • Changed Result alias from Result<T> to Result<T, E = Error>

How it should look in endpoints:

struct CustomError;
impl IntoResponse for CustomError {
    fn into_response(self) -> Response { Response::new(418) }
}

async fn endpoint_a(_req: tide::Request<()>) -> Result<String, CustomError> {
    Err(CustomError)
}

async fn endpoint_b(_req: tide::Request<()>) -> tide::Result<String> {
    use tide::ResultExt;

    example_fn().await?;                         // returns 500 with body of "error"
    example_fn().await.with_empty(400)?;         // returns 400 with empty body
    example_fn().await.with_status(400)?;        // returns 400 with body of "error"
    example_fn().await.with_err_res(|_| "hello") // returns 200 with body of "hello"

    Ok(String::from("ok"))
}

// do stuff that can return errors
async fn example_fn() -> Result<(), String> {
    Err(String::from("error"))
}

@yoshuawuyts yoshuawuyts mentioned this pull request Jan 20, 2020
4 tasks
@petejodo
Copy link
Collaborator

petejodo commented Jan 20, 2020

I hope this is a useful question/thought but what could be a way a user determines the environment their app is running and if it's development, return useful debugging information?

Would it require the user having to do something explicit for each endpoint function that can potentially return an error, for example referring to app state every time i.e. CustomError::from_state(reg.state()).

To me the ideal situation would be handling that on a per-error basis

Edit: I realized this is an MR so maybe that's a better question to ask in the issue, sorry!

@isgj
Copy link
Contributor

isgj commented Feb 14, 2020

It will be handy to have a method to map also a serializable error, not only the ones that implement the Display trait.

new_data.validate().map_to_json(400)?;
// the next line doesn't work either
// .map_err(|e| Response::new(400).body_json(&e))?;

where validate() return Result<NewData, E:Serialize>

@yoshuawuyts
Copy link
Member

@kbillings thanks so much for this PR! Wrote about this more last week: https://blog.yoshuawuyts.com/async-http/#error-handling. We now have a full error implementation as as part of http-types, which we'll likely merge in the near future.

Some of the ideas in this patch have been integrated into http-types as well, especially the status method that allows setting a status as a shorthand.

This is one issue of about 8? that touch on error handling in Tide, and wanted to share an update on progress that has been made since.

@goto-bus-stop goto-bus-stop changed the base branch from master to main August 9, 2020 12:56
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

Successfully merging this pull request may close these issues.

4 participants