From 72994c00e11c806212b8fb8e962e0f9e6f5594c9 Mon Sep 17 00:00:00 2001 From: ayushsingh82 Date: Sat, 8 Aug 2026 16:24:16 +0530 Subject: [PATCH] =?UTF-8?q?fix:=20implement=20application/graphql-response?= =?UTF-8?q?+json=20handling=20per=20spec=20=C2=A76.4.2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit process_graphql_response was a stub that just delegated to the legacy application/json handler instead of implementing the current GraphQL-over-HTTP media type's rules. Per spec §6.4.2, a response declaring application/graphql-response+json is guaranteed to carry a well-formed GraphQL response body regardless of HTTP status code, so clients should parse it unconditionally rather than gating on the status code the way the legacy path does for §6.4.1 compatibility. Extracts the shared body read/parse/dispatch logic into read_and_process_response_body, keeping the status-code guard only on the legacy path. --- Cargo.lock | 1 + thegraph-graphql-http/Cargo.toml | 1 + thegraph-graphql-http/src/http_client.rs | 128 ++++++++++++++++++++++- 3 files changed, 127 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 692afb22..c0d883f5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4951,6 +4951,7 @@ dependencies = [ "async-trait", "graphql-parser", "graphql_client", + "http 1.3.1", "indoc", "reqwest", "serde", diff --git a/thegraph-graphql-http/Cargo.toml b/thegraph-graphql-http/Cargo.toml index 41b8265b..3952ed4e 100644 --- a/thegraph-graphql-http/Cargo.toml +++ b/thegraph-graphql-http/Cargo.toml @@ -26,6 +26,7 @@ thiserror = "1.0" [dev-dependencies] assert_matches = "1.5.0" +http = "1" indoc = "2.0.5" tokio = { version = "1.37.0", features = ["rt", "macros", "time"] } diff --git a/thegraph-graphql-http/src/http_client.rs b/thegraph-graphql-http/src/http_client.rs index b5ca923a..ba51149e 100644 --- a/thegraph-graphql-http/src/http_client.rs +++ b/thegraph-graphql-http/src/http_client.rs @@ -195,9 +195,22 @@ mod reqwest_ext { where ResponseData: serde::de::DeserializeOwned, { - // TODO: Add support for the GraphQL-over-HTTP response media type (non-legacy) - // Fall back to legacy media type for now. - process_legacy_graphql_response(resp).await + let status = resp.status(); + + // [6.4.2 application/graphql-response+json](https://graphql.github.io/graphql-over-http/draft/#sec-application-graphql-response-json) + // + // > Clients should process a response using the `application/graphql-response+json` media + // > type as a well-formed GraphQL response independent of the HTTP status code. + // + // > In case of errors that completely prevent the generation of a well-formed GraphQL + // > response, the server SHOULD respond with the appropriate HTTP `4xx` or `5xx` status + // > code depending on the concrete error condition, and MUST NOT use the + // > `application/graphql-response+json` media type. + // + // Since this response already declared the `application/graphql-response+json` media type, + // its body is guaranteed by the spec to be a well-formed GraphQL response regardless of the + // HTTP status code, so unlike the legacy media type there is no need to gate on it here. + read_and_process_response_body(status, resp).await } /// Process the GraphQL-over-HTTP response when the legacy media type, `application/json`, is used. @@ -230,6 +243,17 @@ mod reqwest_ext { )); } + read_and_process_response_body(status, resp).await + } + + /// Read and deserialize the GraphQL-over-HTTP response body, then process it. + async fn read_and_process_response_body( + status: reqwest::StatusCode, + resp: reqwest::Response, + ) -> Result, RequestError> + where + ResponseData: serde::de::DeserializeOwned, + { // Receive the response body. let response = resp.bytes().await.map_err(|err| { RequestError::ResponseRecvError(status, format!("Error reading response body: {err}")) @@ -252,4 +276,102 @@ mod reqwest_ext { Ok(process_response_body(response)) } + + #[cfg(test)] + mod tests { + use assert_matches::assert_matches; + + use super::*; + use crate::http::response::{ + GRAPHQL_LEGACY_RESPONSE_MEDIA_TYPE, GRAPHQL_RESPONSE_MEDIA_TYPE, + }; + use crate::http_client::ResponseError; + + /// Build a `reqwest::Response` in-memory (no network I/O) with the given status, + /// `Content-Type` header and body. + fn build_response( + status: u16, + content_type: Option<&str>, + body: &str, + ) -> reqwest::Response { + let mut builder = http::Response::builder().status(status); + if let Some(content_type) = content_type { + builder = builder.header(CONTENT_TYPE, content_type); + } + let response = builder.body(body.to_string()).unwrap(); + reqwest::Response::from(response) + } + + #[derive(Debug, serde::Deserialize, PartialEq)] + struct Data { + value: String, + } + + #[test] + fn legacy_response_detected_from_content_type() { + let resp = build_response(200, Some(GRAPHQL_LEGACY_RESPONSE_MEDIA_TYPE), "{}"); + assert!(is_legacy_response(&resp)); + + let resp = build_response(200, Some(GRAPHQL_RESPONSE_MEDIA_TYPE), "{}"); + assert!(!is_legacy_response(&resp)); + + // No `Content-Type` header SHOULD be interpreted as the legacy media type. + let resp = build_response(200, None, "{}"); + assert!(is_legacy_response(&resp)); + } + + #[tokio::test] + async fn graphql_response_media_type_with_data_is_processed() { + let body = r#"{"data":{"value":"hello"}}"#; + let resp = build_response(200, Some(GRAPHQL_RESPONSE_MEDIA_TYPE), body); + + let result = process_graphql_response::(resp).await; + + assert_matches!(result, Ok(Ok(data)) => { + assert_eq!(data, Data { value: "hello".to_string() }); + }); + } + + /// Per spec §6.4.2, a response using the `application/graphql-response+json` media type is + /// guaranteed to carry a well-formed GraphQL response body regardless of the HTTP status + /// code, so the body must still be parsed even on a `4xx`/`5xx` status. + #[tokio::test] + async fn graphql_response_media_type_ignores_status_code() { + let body = r#"{"errors":[{"message":"not found"}]}"#; + let resp = build_response(404, Some(GRAPHQL_RESPONSE_MEDIA_TYPE), body); + + let result = process_graphql_response::(resp).await; + + assert_matches!(result, Ok(Err(ResponseError::Failure { errors })) => { + assert_eq!(errors.len(), 1); + assert_eq!(errors[0].message, "not found"); + }); + } + + /// Unlike the graphql-response+json path, the legacy media type bails out on statuses that + /// are neither success, client error nor server error (e.g. a redirect) without attempting + /// to parse the body, per the compatibility rationale in spec §6.4.1. + #[tokio::test] + async fn legacy_response_media_type_rejects_unexpected_status_class() { + let body = r#"{"data":{"value":"hello"}}"#; + let resp = build_response(304, Some(GRAPHQL_LEGACY_RESPONSE_MEDIA_TYPE), body); + + let result = process_legacy_graphql_response::(resp).await; + + assert_matches!(result, Err(RequestError::ResponseRecvError(status, _)) => { + assert_eq!(status.as_u16(), 304); + }); + } + + #[tokio::test] + async fn graphql_response_media_type_empty_body_errors() { + let resp = build_response(200, Some(GRAPHQL_RESPONSE_MEDIA_TYPE), ""); + + let result = process_graphql_response::(resp).await; + + assert_matches!(result, Err(RequestError::ResponseRecvError(status, _)) => { + assert_eq!(status.as_u16(), 200); + }); + } + } }