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 server function client side error handling #1597

Merged
merged 1 commit into from
Aug 30, 2023

Conversation

drdo
Copy link
Contributor

@drdo drdo commented Aug 26, 2023

Handle all error codes 401-499 in addition to the 400 and 500-599 that were already handled.

In addition, handle them all in the same way and improve the error message.

The idea came about when trying to handle authentication in an axum layer
by setting the 401 status code.
In this situation the client side still tried to parse the body as if this was a
success case and failed with a deserialization error message.

Handle all error codes 401-499 in addition to the
400 and 500-599 that were already handled.

In addition, handle them all in the same way
and improve the error message.
@gbj
Copy link
Collaborator

gbj commented Aug 27, 2023

Hm okay so the only complication I can see, and which would be a breaking change relative to the status quo, is this:

// semi-pseudo-code because I'm being lazy
#[server] 
pub async fn am_teapot() -> Result<String, ServerFnError> {
  let resp = expect_context::<ResponseOptions>();
  resp.set_status(418);
  Ok("I'm a little teapot".to_string())
}

The server function is returning Ok("I'm a little teapot"), but the client deserializes it as Err(ServerFnError::ServerError("418 IM A TEAPOT: I'm a little teapot.") or something.

i.e., it's possible for a user to manually set a 4xx response and also return a valid Ok condition from a server function. I'm open to opinions on how to handle this.

@drdo
Copy link
Contributor Author

drdo commented Aug 27, 2023

In my opinion I wouldn't even try to parse JSON at all on the client if the HTTP response is 4xx or 5xx.
These are error codes and I don't see the upside to using them for successful requests.
I simply left that in to keep the same behaviour that was already happening in the 5xx case.

I like the idea of the server being able to provide a more descriptive error string though.

@gbj
Copy link
Collaborator

gbj commented Aug 30, 2023

Make sense. This looks good to me.

@gbj gbj merged commit 4d7e1f4 into leptos-rs:main Aug 30, 2023
@drdo drdo deleted the improve-4xx-handling-server-fn branch August 30, 2023 08:26
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.

2 participants