fix: Handle HTTP errors in FromResponseParts (#5243)

* fix: Handle HTTP errors in FromResponseParts

Check for non-success status codes before attempting to decode response
parts. For error responses, parse the JSON error payload and convert it
to a ServerFnError::ServerError.

Also make ErrorPayload fields pub(crate) to allow internal access.

* fixed test name

Author: nerjs

packages/fullstack/src/magic.rs

@@ -87,12 +87,12 @@ pub enum RestEndpointPayload<T, E> {
 /// The error payload structure for REST API errors.
 #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
 pub struct ErrorPayload<E> {
-    message: String,
+    pub(crate) message: String,
 
-    code: u16,
+    pub(crate) code: u16,
 
     #[serde(skip_serializing_if = "Option::is_none")]
-    data: Option<E>,
+    pub(crate) data: Option<E>,
 }
 
 /// Convert a `RequestError` into a `ServerFnError`.

packages/fullstack/src/request.rs

@@ -4,7 +4,7 @@ use headers::Header;
 use http::response::Parts;
 use std::{future::Future, pin::Pin};
 
-use crate::{ClientRequest, ClientResponse};
+use crate::{ClientRequest, ClientResponse, ErrorPayload};
 
 /// The `IntoRequest` trait allows types to be used as the body of a request to a HTTP endpoint or server function.
 ///
@@ -54,9 +54,24 @@ where
 {
     fn from_response(res: ClientResponse) -> impl Future<Output = Result<Self, ServerFnError>> {
         async move {
-            let (parts, _body) = res.into_parts();
-            let mut parts = parts;
-            A::from_response_parts(&mut parts)
+            let status = res.status();
+
+            if status.is_success() {
+                let (parts, _body) = res.into_parts();
+                let mut parts = parts;
+                A::from_response_parts(&mut parts)
+            } else {
+                let ErrorPayload::<serde_json::Value> {
+                    message,
+                    code,
+                    data,
+                } = res.json().await?;
+                Err(ServerFnError::ServerError {
+                    message,
+                    code,
+                    details: data,
+                })
+            }
         }
     }
 }
@@ -202,3 +217,80 @@ pub fn assert_can_encode(_t: impl AssertCanEncode) {}
 
 #[doc(hidden)]
 pub fn assert_can_decode(_t: impl AssertCanDecode) {}
+
+#[cfg(test)]
+mod test {
+    use http::Extensions;
+
+    use super::*;
+
+    #[derive(Debug)]
+    struct TestFromResponse;
+
+    impl FromResponseParts for TestFromResponse {
+        fn from_response_parts(_parts: &mut Parts) -> Result<Self, ServerFnError> {
+            Ok(Self)
+        }
+    }
+
+    fn build_response(status: u16, body: String) -> ClientResponse {
+        let http_response = http::Response::builder()
+            .status(status)
+            .body(body.into_bytes())
+            .unwrap();
+        let reqwest_response = reqwest::Response::from(http_response);
+        ClientResponse {
+            response: Box::new(reqwest_response),
+            extensions: Extensions::new(),
+        }
+    }
+
+    #[test]
+    fn fromresponseparts_path_decodes_ok_on_2xx() {
+        futures::executor::block_on(async {
+            let response = build_response(200, "".to_string());
+            let result = TestFromResponse::from_response(response).await;
+            assert!(
+                result.is_ok(),
+                "expected Ok(..) for HTTP 200 success case, got: {:?}",
+                result
+            );
+        });
+    }
+
+    #[test]
+    fn fromresponseparts_falls_back_to_request_error_on_unparsable_error_body() {
+        futures::executor::block_on(async {
+            let response = build_response(400, "".to_string());
+            let result = TestFromResponse::from_response(response).await;
+            assert!(result.is_err(), "expected Err(..) for HTTP 400 failed case");
+            let error = result.unwrap_err();
+            assert!(matches!(
+                error,
+                ServerFnError::Request(RequestError::Decode(_))
+            ));
+        });
+    }
+
+    #[test]
+    fn fromresponseparts_parses_error_payload_on_http_error() {
+        futures::executor::block_on(async {
+            let body = r#"{
+                "message": "qwerty",
+                "code": 400
+            }"#;
+            let response = build_response(400, body.to_string());
+            let result = TestFromResponse::from_response(response).await;
+            assert!(result.is_err(), "expected Err(..) for HTTP 400 failed case");
+            let error = result.unwrap_err();
+            assert_eq!(
+                error,
+                ServerFnError::ServerError {
+                    message: "qwerty".to_string(),
+                    code: 400,
+                    details: None
+                }
+            );
+        });
+    }
+}