feat: include HTTP response body in error messages (#3347)

Co-authored-by: Tushar Mathur <tusharmath@gmail.com>

Author: amitksingh1490

src/cli/runtime/http.rs

@@ -176,12 +176,10 @@ impl HttpIO for NativeHttp {
             tracing::Span::current().set_attribute(status_code.key, status_code.value);
         }
 
-        Ok(Response::from_reqwest(
-            response?
-                .error_for_status()
-                .map_err(|err| err.without_url())?,
-        )
-        .await?)
+        // Get the response
+        let response = response?;
+
+        Response::from_reqwest_with_error_handling(response).await
     }
 }
 
@@ -282,4 +280,70 @@ mod tests {
         let resp = make_request(&url1, &native_http).await;
         assert_eq!(resp.headers.get("x-cache-lookup").unwrap(), "MISS");
     }
+    #[tokio::test]
+    async fn test_native_http_error_with_body() {
+        let server = start_mock_server();
+
+        // Mock a 404 response with an error message body
+        server.mock(|when, then| {
+            when.method(httpmock::Method::GET).path("/error-with-body");
+            then.status(404).body("{\"error\":\"Resource not found\"}");
+        });
+
+        let native_http = NativeHttp::init(&Default::default(), &Default::default());
+        let port = server.port();
+        let request_url = format!("http://localhost:{}/error-with-body", port);
+
+        // Create a request that will result in a 404 error
+        let request = reqwest::Request::new(Method::GET, request_url.parse().unwrap());
+        let result = native_http.execute(request).await;
+
+        // Assert that we get an error
+        assert!(result.is_err());
+
+        // Convert the error to a string to check its content
+        let error = result.unwrap_err();
+        let error_string = format!("{:?}", error);
+        // Check that the error contains both the status code and the error message
+        assert!(
+            error_string.contains("404"),
+            "Error should contain status code"
+        );
+        assert!(
+            error_string.contains("Resource not found"),
+            "Error should contain the error message body"
+        );
+    }
+
+    #[tokio::test]
+    async fn test_native_http_error_without_body() {
+        let server = start_mock_server();
+
+        // Mock a 500 response with an empty body
+        server.mock(|when, then| {
+            when.method(httpmock::Method::GET)
+                .path("/error-without-body");
+            then.status(500).body("");
+        });
+
+        let native_http = NativeHttp::init(&Default::default(), &Default::default());
+        let port = server.port();
+        let request_url = format!("http://localhost:{}/error-without-body", port);
+
+        // Create a request that will result in a 500 error
+        let request = reqwest::Request::new(Method::GET, request_url.parse().unwrap());
+        let result = native_http.execute(request).await;
+
+        // Assert that we get an error
+        assert!(result.is_err());
+
+        // Convert the error to a string to check its content
+        let error = result.unwrap_err();
+        let error_string = format!("{:?}", error);
+        // Check that the error contains the status code but the body is empty
+        assert!(
+            error_string.contains("500"),
+            "Error should contain status code"
+        );
+    }
 }

src/core/http/response.rs

@@ -49,6 +49,27 @@ impl FromValue for ConstValue {
 }
 
 impl Response<Bytes> {
+    /// Handle error responses with body extraction for better error messages
+    /// This is a common pattern used across different HTTP clients
+    pub async fn from_reqwest_with_error_handling(
+        response: reqwest::Response,
+    ) -> anyhow::Result<Self> {
+        // Check if it's an error status
+        if let Err(err) = response.error_for_status_ref() {
+            // Get the body content first (this is the key step)
+            let body_text = response.text().await?;
+            // Create an error with the status code and add body content as context
+            let err = Error::HTTP {
+                message: err.without_url().to_string(),
+                body: body_text.clone(),
+            };
+            return Err(anyhow::Error::new(err).context(body_text));
+        }
+
+        // If not an error status, proceed normally
+        Self::from_reqwest(response).await
+    }
+
     pub async fn from_reqwest(resp: reqwest::Response) -> Result<Self> {
         let status = resp.status();
         let headers = resp.headers().to_owned();

src/core/ir/error.rs

@@ -11,7 +11,10 @@ use crate::core::{auth, cache, worker, Errata};
 #[derive(From, Debug, Error, Clone)]
 pub enum Error {
     IO(String),
-
+    HTTP {
+        message: String,
+        body: String,
+    },
     GRPC {
         grpc_code: i32,
         grpc_description: String,
@@ -47,6 +50,8 @@ impl From<Error> for Errata {
     fn from(value: Error) -> Self {
         match value {
             Error::IO(message) => Errata::new("IOException").description(message),
+            Error::HTTP{ message, body:_ } => Errata::new("HTTP Error")
+                .description(message),
             Error::GRPC {
                 grpc_code,
                 grpc_description,
@@ -87,6 +92,14 @@ impl ErrorExtensions for Error {
                 e.set("grpcStatusMessage", grpc_status_message);
                 e.set("grpcStatusDetails", grpc_status_details.clone());
             }
+
+            if let Error::HTTP { message: _, body } = self {
+                if let Ok(ConstValue::Object(map)) = serde_json::from_str::<ConstValue>(body) {
+                    e.extend(map);
+                } else {
+                    e.set("cause", body);
+                }
+            }
         })
     }
 }

src/core/jit/graphql_error.rs

@@ -138,6 +138,19 @@ impl ErrorExtensionValues {
         self.0.insert(name.as_ref().to_string(), value.into());
     }
 
+    /// Extend the extension values with key-value pairs
+    pub fn extend<K, V, I>(&mut self, iter: I)
+    where
+        K: AsRef<str>,
+        V: Into<async_graphql::Value>,
+        I: IntoIterator<Item = (K, V)>,
+    {
+        self.0.extend(
+            iter.into_iter()
+                .map(|(k, v)| (k.as_ref().to_string(), v.into())),
+        );
+    }
+
     /// Unset an extension value.
     pub fn unset(&mut self, name: impl AsRef<str>) {
         self.0.remove(name.as_ref());

src/core/runtime.rs

@@ -118,13 +118,8 @@ pub mod test {
     #[async_trait::async_trait]
     impl HttpIO for TestHttp {
         async fn execute(&self, request: reqwest::Request) -> Result<Response<Bytes>> {
-            let response = self.client.execute(request).await;
-            Response::from_reqwest(
-                response?
-                    .error_for_status()
-                    .map_err(|err| err.without_url())?,
-            )
-            .await
+            let response = self.client.execute(request).await?;
+            Response::from_reqwest_with_error_handling(response).await
         }
     }
 

tailcall-aws-lambda/src/http.rs

@@ -27,15 +27,11 @@ impl LambdaHttp {
 #[async_trait::async_trait]
 impl HttpIO for LambdaHttp {
     async fn execute(&self, request: reqwest::Request) -> Result<Response<Bytes>> {
-        let req_str = format!("{} {}", request.method(), request.url());
-        let response = self
-            .client
-            .execute(request)
-            .await?
-            .error_for_status()
-            .map_err(|err| err.without_url())?;
-        let res = Response::from_reqwest(response).await?;
-        tracing::info!("{} {}", req_str, res.status.as_u16());
+        let request_str = format!("{} {}", request.method(), request.url());
+        let response = self.client.execute(request).await?;
+
+        let res = Response::from_reqwest_with_error_handling(response).await?;
+        tracing::info!("{} {}", request_str, res.status.as_u16());
         Ok(res)
     }
 }

tailcall-cloudflare/src/http.rs

@@ -37,12 +37,8 @@ impl HttpIO for CloudflareHttp {
         let url = request.url().clone();
         // TODO: remove spawn local
         let res = spawn_local(async move {
-            let response = client
-                .execute(request)
-                .await?
-                .error_for_status()
-                .map_err(|err| err.without_url())?;
-            Response::from_reqwest(response).await
+            let response = client.execute(request).await?;
+            Response::from_reqwest_with_error_handling(response).await
         })
         .await?;
         tracing::info!("{} {} {}", method, url, res.status.as_u16());

tailcall-wasm/src/http.rs

@@ -33,12 +33,8 @@ impl HttpIO for WasmHttp {
         let url = request.url().clone();
         // TODO: remove spawn local
         let res = spawn_local(async move {
-            let response = client
-                .execute(request)
-                .await?
-                .error_for_status()
-                .map_err(|err| err.without_url())?;
-            Response::from_reqwest(response).await
+            let response = client.execute(request).await?;
+            Response::from_reqwest_with_error_handling(response).await
         })
         .await?;
         tracing::info!("{} {} {}", method, url, res.status.as_u16());

tests/core/http.rs

@@ -9,6 +9,7 @@ use anyhow::anyhow;
 use http::header::{HeaderName, HeaderValue};
 use hyper::body::Bytes;
 use tailcall::core::http::Response;
+use tailcall::core::ir::Error;
 use tailcall::core::HttpIO;
 
 use super::runtime::{ExecutionMock, ExecutionSpec};
@@ -115,7 +116,20 @@ impl HttpIO for Http {
         let status_code = reqwest::StatusCode::from_u16(mock_response.0.status)?;
 
         if status_code.is_client_error() || status_code.is_server_error() {
-            return Err(anyhow::format_err!("Status code error"));
+            // Include the actual error body from the mock in the error
+            let error_body = mock_response
+                .0
+                .body
+                .map(|body| String::from_utf8_lossy(&body.to_bytes()).to_string())
+                .unwrap_or_default();
+
+            // Return the JSON error body directly as the error so it can be processed in
+            // the error module
+            let error = Error::HTTP {
+                message: format!("{}: {}", status_code, error_body),
+                body: error_body,
+            };
+            return Err(error.into());
         }
 
         let mut response = Response { status: status_code, ..Default::default() };

tests/core/snapshots/upstream-fail-request-with-body.md_0.snap

@@ -0,0 +1,29 @@
+---
+source: tests/core/spec.rs
+expression: response
+---
+{
+  "status": 200,
+  "headers": {
+    "content-type": "application/json"
+  },
+  "body": {
+    "data": null,
+    "errors": [
+      {
+        "message": "HTTP Error: 429 Too Many Requests: {\"code\":\"UM0018\",\"message\":\"change limit exceeded\",\"cause\":\"exceeded the maximum allowed number of name changes\"}",
+        "locations": [
+          {
+            "line": 1,
+            "column": 9
+          }
+        ],
+        "extensions": {
+          "cause": "exceeded the maximum allowed number of name changes",
+          "code": "UM0018",
+          "message": "change limit exceeded"
+        }
+      }
+    ]
+  }
+}

tests/core/snapshots/upstream-fail-request-with-body.md_client.snap

@@ -0,0 +1,16 @@
+---
+source: tests/core/spec.rs
+expression: formatted
+---
+type Query {
+  user: User
+}
+
+type User {
+  id: Int
+  name: String
+}
+
+schema {
+  query: Query
+}

tests/core/snapshots/upstream-fail-request-with-body.md_merged.snap

@@ -0,0 +1,16 @@
+---
+source: tests/core/spec.rs
+expression: formatter
+---
+schema @server @upstream @link(src: "schema_0.graphql", type: Config) {
+  query: Query
+}
+
+type Query {
+  user: User @http(url: "http://jsonplaceholder.typicode.com/users/1")
+}
+
+type User {
+  id: Int
+  name: String
+}

tests/core/snapshots/upstream-fail-request.md_0.snap

@@ -1,7 +1,6 @@
 ---
 source: tests/core/spec.rs
 expression: response
-snapshot_kind: text
 ---
 {
   "status": 200,
@@ -12,7 +11,7 @@ snapshot_kind: text
     "data": null,
     "errors": [
       {
-        "message": "IOException: Status code error",
+        "message": "HTTP Error: 503 Service Unavailable: {}",
         "locations": [
           {
             "line": 1,

tests/execution/upstream-fail-request-with-body.md

@@ -0,0 +1,33 @@
+# Simple GraphQL Request
+
+```graphql @schema
+schema {
+  query: Query
+}
+
+type User {
+  id: Int
+  name: String
+}
+
+type Query {
+  user: User @http(url: "http://jsonplaceholder.typicode.com/users/1")
+}
+```
+
+```yml @mock
+- request:
+    method: GET
+    url: http://jsonplaceholder.typicode.com/users/1
+  response:
+    status: 429
+    body:
+      {code: "UM0018", message: "change limit exceeded", cause: "exceeded the maximum allowed number of name changes"}
+```
+
+```yml @test
+- method: POST
+  url: http://localhost:8080/graphql
+  body:
+    query: query { user { name } }
+```