Return response with details from the server on error

[aklajnert]

Resolves #616

This PR introduces a new enum variant for FetchError that contains the response status together with the JSON response returned from the server.

Initially, I wanted to change the check_status() method to try to return the new variant first, and in case of JSON parse error just return the regular StatusError, but it would require changing the method to async which would be a breaking change. Therefore I decided to add another method.

Here's how it looks when you log!() such error:

DetailedStatusError(
    Status {
        code: 500,
        text: "Internal Server Error",
        category: ServerError,
    },
    Object({
        "details": String("cannot connect to the database",),
    }),
)

[glennsl]

This seems a bit bolted-on TBH. Adding another case toFetchError means you'll also have to check for DetailedStatusError wherever you check for StatusError, which is both boilerplate-y and rather error-prone. I would also consider this a breaking change, since consumers who take advantage of exhaustiveness checking of enums will have to change their code to include this new case.

What about having check_detailed_status return a tuple instead of adding a new case?

Also, why limit this to only work with JSON responses? What about XML or whatever newfangled format that will replace JSON? Could it not return the response object instead, and have the consumer decide how to decode the body? Or, alternatively, pass a decoder in as an argument?

[aklajnert]

Thanks @glennsl, these were all fair points that I've missed. I didn't really consider that adding an enum variant is a breaking change, but you're right. Also, I was a bit selfish and didn't consider other response formats, as I work mostly with JSON.
I've changed my code to fix that.

[flosse]

BTW: This is my current solution that works for me:

#[derive(Debug)]
pub enum Error<E> {
    Fetch(FetchError),
    Api(boundary::Error<E>),
}

impl<E> From<FetchError> for Error<E> {
    fn from(e: FetchError) -> Self {
        Self::Fetch(e)
    }
}

pub type Result<T, E> = result::Result<T, Error<E>>;

async fn to_result<T, E>(res: Response) -> Result<T, E>
where
    T: for<'de> Deserialize<'de> + 'static,
    E: for<'de> Deserialize<'de> + 'static,
{
    if res.status().is_ok() {
        let data = res.json().await?;
        Ok(data)
    } else {
        let error = res.json().await?;
        Err(Error::Api(error))
    }
}

And this is how I use it:

pub async fn get_json<T, E>(url: &str) -> Result<T, E>
where
    T: for<'de> Deserialize<'de> + 'static,
    E: for<'de> Deserialize<'de> + 'static,
{
    let res = fetch(url).await?;
    to_result(res).await
}

pub async fn post_json<R, T, E>(url: &str, req: &R) -> Result<T, E>
where
    R: Serialize,
    T: for<'de> Deserialize<'de> + 'static,
    E: for<'de> Deserialize<'de> + 'static,
{
    let req = Request::new(url).method(Method::Post).json(req)?;
    let res = fetch(req).await?;
    to_result(res).await
}

[MartinKavik]

Thanks for PR!

Also thanks @flosse for comments and @glennsl for excellent code review / suggestions.

Let's wait a bit until https://github.com/seed-rs/seed/pull/618 is merged, then we can rebase (to also resolve clippy/compiler errors (?)) and merge it.

[MartinKavik]

https://github.com/seed-rs/seed/pull/618 has been merged. Could you please rebase, update CHANGELOG and force-push?

[aklajnert]

@MartinKavik - done