Skip to content

Commit a1e9789

Browse files
amitksingh1490tusharmath
amitksingh1490
and
tusharmath
authored
feat: include HTTP response body in error messages (#3347)
Co-authored-by: Tushar Mathur <[email protected]>
1 parent 7c5f07d commit a1e9789

14 files changed

+239
-38
lines changed

‎src/cli/runtime/http.rs

+70-6
Original file line numberDiff line numberDiff line change
@@ -176,12 +176,10 @@ impl HttpIO for NativeHttp {
176176
tracing::Span::current().set_attribute(status_code.key, status_code.value);
177177
}
178178

179-
Ok(Response::from_reqwest(
180-
response?
181-
.error_for_status()
182-
.map_err(|err| err.without_url())?,
183-
)
184-
.await?)
179+
// Get the response
180+
let response = response?;
181+
182+
Response::from_reqwest_with_error_handling(response).await
185183
}
186184
}
187185

@@ -282,4 +280,70 @@ mod tests {
282280
let resp = make_request(&url1, &native_http).await;
283281
assert_eq!(resp.headers.get("x-cache-lookup").unwrap(), "MISS");
284282
}
283+
#[tokio::test]
284+
async fn test_native_http_error_with_body() {
285+
let server = start_mock_server();
286+
287+
// Mock a 404 response with an error message body
288+
server.mock(|when, then| {
289+
when.method(httpmock::Method::GET).path("/error-with-body");
290+
then.status(404).body("{\"error\":\"Resource not found\"}");
291+
});
292+
293+
let native_http = NativeHttp::init(&Default::default(), &Default::default());
294+
let port = server.port();
295+
let request_url = format!("http://localhost:{}/error-with-body", port);
296+
297+
// Create a request that will result in a 404 error
298+
let request = reqwest::Request::new(Method::GET, request_url.parse().unwrap());
299+
let result = native_http.execute(request).await;
300+
301+
// Assert that we get an error
302+
assert!(result.is_err());
303+
304+
// Convert the error to a string to check its content
305+
let error = result.unwrap_err();
306+
let error_string = format!("{:?}", error);
307+
// Check that the error contains both the status code and the error message
308+
assert!(
309+
error_string.contains("404"),
310+
"Error should contain status code"
311+
);
312+
assert!(
313+
error_string.contains("Resource not found"),
314+
"Error should contain the error message body"
315+
);
316+
}
317+
318+
#[tokio::test]
319+
async fn test_native_http_error_without_body() {
320+
let server = start_mock_server();
321+
322+
// Mock a 500 response with an empty body
323+
server.mock(|when, then| {
324+
when.method(httpmock::Method::GET)
325+
.path("/error-without-body");
326+
then.status(500).body("");
327+
});
328+
329+
let native_http = NativeHttp::init(&Default::default(), &Default::default());
330+
let port = server.port();
331+
let request_url = format!("http://localhost:{}/error-without-body", port);
332+
333+
// Create a request that will result in a 500 error
334+
let request = reqwest::Request::new(Method::GET, request_url.parse().unwrap());
335+
let result = native_http.execute(request).await;
336+
337+
// Assert that we get an error
338+
assert!(result.is_err());
339+
340+
// Convert the error to a string to check its content
341+
let error = result.unwrap_err();
342+
let error_string = format!("{:?}", error);
343+
// Check that the error contains the status code but the body is empty
344+
assert!(
345+
error_string.contains("500"),
346+
"Error should contain status code"
347+
);
348+
}
285349
}

‎src/core/http/response.rs

+21
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,27 @@ impl FromValue for ConstValue {
4949
}
5050

5151
impl Response<Bytes> {
52+
/// Handle error responses with body extraction for better error messages
53+
/// This is a common pattern used across different HTTP clients
54+
pub async fn from_reqwest_with_error_handling(
55+
response: reqwest::Response,
56+
) -> anyhow::Result<Self> {
57+
// Check if it's an error status
58+
if let Err(err) = response.error_for_status_ref() {
59+
// Get the body content first (this is the key step)
60+
let body_text = response.text().await?;
61+
// Create an error with the status code and add body content as context
62+
let err = Error::HTTP {
63+
message: err.without_url().to_string(),
64+
body: body_text.clone(),
65+
};
66+
return Err(anyhow::Error::new(err).context(body_text));
67+
}
68+
69+
// If not an error status, proceed normally
70+
Self::from_reqwest(response).await
71+
}
72+
5273
pub async fn from_reqwest(resp: reqwest::Response) -> Result<Self> {
5374
let status = resp.status();
5475
let headers = resp.headers().to_owned();

‎src/core/ir/error.rs

+14-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ use crate::core::{auth, cache, worker, Errata};
1111
#[derive(From, Debug, Error, Clone)]
1212
pub enum Error {
1313
IO(String),
14-
14+
HTTP {
15+
message: String,
16+
body: String,
17+
},
1518
GRPC {
1619
grpc_code: i32,
1720
grpc_description: String,
@@ -47,6 +50,8 @@ impl From<Error> for Errata {
4750
fn from(value: Error) -> Self {
4851
match value {
4952
Error::IO(message) => Errata::new("IOException").description(message),
53+
Error::HTTP{ message, body:_ } => Errata::new("HTTP Error")
54+
.description(message),
5055
Error::GRPC {
5156
grpc_code,
5257
grpc_description,
@@ -87,6 +92,14 @@ impl ErrorExtensions for Error {
8792
e.set("grpcStatusMessage", grpc_status_message);
8893
e.set("grpcStatusDetails", grpc_status_details.clone());
8994
}
95+
96+
if let Error::HTTP { message: _, body } = self {
97+
if let Ok(ConstValue::Object(map)) = serde_json::from_str::<ConstValue>(body) {
98+
e.extend(map);
99+
} else {
100+
e.set("cause", body);
101+
}
102+
}
90103
})
91104
}
92105
}

‎src/core/jit/graphql_error.rs

+13
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,19 @@ impl ErrorExtensionValues {
138138
self.0.insert(name.as_ref().to_string(), value.into());
139139
}
140140

141+
/// Extend the extension values with key-value pairs
142+
pub fn extend<K, V, I>(&mut self, iter: I)
143+
where
144+
K: AsRef<str>,
145+
V: Into<async_graphql::Value>,
146+
I: IntoIterator<Item = (K, V)>,
147+
{
148+
self.0.extend(
149+
iter.into_iter()
150+
.map(|(k, v)| (k.as_ref().to_string(), v.into())),
151+
);
152+
}
153+
141154
/// Unset an extension value.
142155
pub fn unset(&mut self, name: impl AsRef<str>) {
143156
self.0.remove(name.as_ref());

‎src/core/runtime.rs

+2-7
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,8 @@ pub mod test {
118118
#[async_trait::async_trait]
119119
impl HttpIO for TestHttp {
120120
async fn execute(&self, request: reqwest::Request) -> Result<Response<Bytes>> {
121-
let response = self.client.execute(request).await;
122-
Response::from_reqwest(
123-
response?
124-
.error_for_status()
125-
.map_err(|err| err.without_url())?,
126-
)
127-
.await
121+
let response = self.client.execute(request).await?;
122+
Response::from_reqwest_with_error_handling(response).await
128123
}
129124
}
130125

‎tailcall-aws-lambda/src/http.rs

+5-9
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,11 @@ impl LambdaHttp {
2727
#[async_trait::async_trait]
2828
impl HttpIO for LambdaHttp {
2929
async fn execute(&self, request: reqwest::Request) -> Result<Response<Bytes>> {
30-
let req_str = format!("{} {}", request.method(), request.url());
31-
let response = self
32-
.client
33-
.execute(request)
34-
.await?
35-
.error_for_status()
36-
.map_err(|err| err.without_url())?;
37-
let res = Response::from_reqwest(response).await?;
38-
tracing::info!("{} {}", req_str, res.status.as_u16());
30+
let request_str = format!("{} {}", request.method(), request.url());
31+
let response = self.client.execute(request).await?;
32+
33+
let res = Response::from_reqwest_with_error_handling(response).await?;
34+
tracing::info!("{} {}", request_str, res.status.as_u16());
3935
Ok(res)
4036
}
4137
}

‎tailcall-cloudflare/src/http.rs

+2-6
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,8 @@ impl HttpIO for CloudflareHttp {
3737
let url = request.url().clone();
3838
// TODO: remove spawn local
3939
let res = spawn_local(async move {
40-
let response = client
41-
.execute(request)
42-
.await?
43-
.error_for_status()
44-
.map_err(|err| err.without_url())?;
45-
Response::from_reqwest(response).await
40+
let response = client.execute(request).await?;
41+
Response::from_reqwest_with_error_handling(response).await
4642
})
4743
.await?;
4844
tracing::info!("{} {} {}", method, url, res.status.as_u16());

‎tailcall-wasm/src/http.rs

+2-6
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,8 @@ impl HttpIO for WasmHttp {
3333
let url = request.url().clone();
3434
// TODO: remove spawn local
3535
let res = spawn_local(async move {
36-
let response = client
37-
.execute(request)
38-
.await?
39-
.error_for_status()
40-
.map_err(|err| err.without_url())?;
41-
Response::from_reqwest(response).await
36+
let response = client.execute(request).await?;
37+
Response::from_reqwest_with_error_handling(response).await
4238
})
4339
.await?;
4440
tracing::info!("{} {} {}", method, url, res.status.as_u16());

‎tests/core/http.rs

+15-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use anyhow::anyhow;
99
use http::header::{HeaderName, HeaderValue};
1010
use hyper::body::Bytes;
1111
use tailcall::core::http::Response;
12+
use tailcall::core::ir::Error;
1213
use tailcall::core::HttpIO;
1314

1415
use super::runtime::{ExecutionMock, ExecutionSpec};
@@ -115,7 +116,20 @@ impl HttpIO for Http {
115116
let status_code = reqwest::StatusCode::from_u16(mock_response.0.status)?;
116117

117118
if status_code.is_client_error() || status_code.is_server_error() {
118-
return Err(anyhow::format_err!("Status code error"));
119+
// Include the actual error body from the mock in the error
120+
let error_body = mock_response
121+
.0
122+
.body
123+
.map(|body| String::from_utf8_lossy(&body.to_bytes()).to_string())
124+
.unwrap_or_default();
125+
126+
// Return the JSON error body directly as the error so it can be processed in
127+
// the error module
128+
let error = Error::HTTP {
129+
message: format!("{}: {}", status_code, error_body),
130+
body: error_body,
131+
};
132+
return Err(error.into());
119133
}
120134

121135
let mut response = Response { status: status_code, ..Default::default() };

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

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
source: tests/core/spec.rs
3+
expression: response
4+
---
5+
{
6+
"status": 200,
7+
"headers": {
8+
"content-type": "application/json"
9+
},
10+
"body": {
11+
"data": null,
12+
"errors": [
13+
{
14+
"message": "HTTP Error: 429 Too Many Requests: {\"code\":\"UM0018\",\"message\":\"change limit exceeded\",\"cause\":\"exceeded the maximum allowed number of name changes\"}",
15+
"locations": [
16+
{
17+
"line": 1,
18+
"column": 9
19+
}
20+
],
21+
"extensions": {
22+
"cause": "exceeded the maximum allowed number of name changes",
23+
"code": "UM0018",
24+
"message": "change limit exceeded"
25+
}
26+
}
27+
]
28+
}
29+
}

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

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
source: tests/core/spec.rs
3+
expression: formatted
4+
---
5+
type Query {
6+
user: User
7+
}
8+
9+
type User {
10+
id: Int
11+
name: String
12+
}
13+
14+
schema {
15+
query: Query
16+
}

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

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
source: tests/core/spec.rs
3+
expression: formatter
4+
---
5+
schema @server @upstream @link(src: "schema_0.graphql", type: Config) {
6+
query: Query
7+
}
8+
9+
type Query {
10+
user: User @http(url: "http://jsonplaceholder.typicode.com/users/1")
11+
}
12+
13+
type User {
14+
id: Int
15+
name: String
16+
}

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

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
---
22
source: tests/core/spec.rs
33
expression: response
4-
snapshot_kind: text
54
---
65
{
76
"status": 200,
@@ -12,7 +11,7 @@ snapshot_kind: text
1211
"data": null,
1312
"errors": [
1413
{
15-
"message": "IOException: Status code error",
14+
"message": "HTTP Error: 503 Service Unavailable: {}",
1615
"locations": [
1716
{
1817
"line": 1,

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

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# Simple GraphQL Request
2+
3+
```graphql @schema
4+
schema {
5+
query: Query
6+
}
7+
8+
type User {
9+
id: Int
10+
name: String
11+
}
12+
13+
type Query {
14+
user: User @http(url: "http://jsonplaceholder.typicode.com/users/1")
15+
}
16+
```
17+
18+
```yml @mock
19+
- request:
20+
method: GET
21+
url: http://jsonplaceholder.typicode.com/users/1
22+
response:
23+
status: 429
24+
body:
25+
{code: "UM0018", message: "change limit exceeded", cause: "exceeded the maximum allowed number of name changes"}
26+
```
27+
28+
```yml @test
29+
- method: POST
30+
url: http://localhost:8080/graphql
31+
body:
32+
query: query { user { name } }
33+
```

0 commit comments

Comments
 (0)