Merge pull request #8 from GraphQLSwift/fix/graphql-json-response-status

Fixes `application/graphql-response+json` status codes

Author: NeedleInAJayStack

Sources/GraphQLHummingbird/Coding/GraphQLJSONEncoder+encode.swift

@@ -7,11 +7,24 @@ extension GraphQLJSONEncoder: @retroactive ResponseEncoder {
     ///   - value: Value to encode
     ///   - request: Request used to generate response
     ///   - context: Request context
-    public func encode(_ value: some Encodable, from _: Request, context _: some RequestContext) throws -> Response {
+    public func encode(_ value: some Encodable, from request: Request, context: some RequestContext) throws -> Response {
+        try encode(value, status: .ok, from: request, context: context)
+    }
+}
+
+extension GraphQLJSONEncoder {
+    /// Extend GraphQLJSONEncoder to support generating a ``HummingbirdCore/Response``. Sets body and header values. Similar to the
+    /// `ResponseEncoder`-required version, except it allows setting the reponse status as well.
+    /// - Parameters:
+    ///   - value: Value to encode
+    ///   - status: The status of the response
+    ///   - request: Request used to generate response
+    ///   - context: Request context
+    func encode(_ value: some Encodable, status: HTTPResponse.Status, from _: Request, context _: some RequestContext) throws -> Response {
         let data = try encode(value)
         let buffer = ByteBuffer(bytes: data)
         return Response(
-            status: .ok,
+            status: status,
             headers: [
                 .contentType: "\(MediaType.applicationJsonGraphQL); charset=utf-8",
                 .contentLength: data.count.description,
@@ -20,3 +33,20 @@ extension GraphQLJSONEncoder: @retroactive ResponseEncoder {
         )
     }
 }
+
+extension GraphQLJSONEncoder {
+    /// Overload for GraphQLJSONEncoder to support generating a ``HummingbirdCore/Response`` from a GraphQLResult. Sets body, headers, and status, according to [GraphQL-over-HTTP spec](https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#applicationgraphql-responsejson).
+    /// - Parameters:
+    ///   - value: GraphQLResult to encode
+    ///   - request: Request used to generate response
+    ///   - context: Request context
+    func encode(_ value: GraphQLResult, from request: Request, context: some RequestContext) throws -> Response {
+        var status = HTTPResponse.Status.ok
+        // We must return `bad request` with the content if there were failures preventing a partial result
+        // https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#applicationgraphql-responsejson
+        if value.data == nil {
+            status = .badRequest
+        }
+        return try encode(value, status: status, from: request, context: context)
+    }
+}

Sources/GraphQLHummingbird/HTTP/GraphQLHandler+HTTP.swift

@@ -25,7 +25,7 @@ extension GraphQLHandler {
             throw HTTPError(.methodNotAllowed, message: "Mutations using GET are disallowed")
         }
         let graphqlContext = try await computeContext(request, context)
-        let result = try await execute(
+        let result = await execute(
             graphQLRequest: graphQLRequest,
             context: graphqlContext,
             additionalValidationRules: config.additionalValidationRules
@@ -43,15 +43,23 @@ extension GraphQLHandler {
         let graphQLRequest: GraphQLRequest
         switch mediaType {
         case .applicationJson, .applicationJsonGraphQL:
-            graphQLRequest = try await JSONDecoder().decode(GraphQLRequest.self, from: request, context: context)
+            do {
+                graphQLRequest = try await JSONDecoder().decode(GraphQLRequest.self, from: request, context: context)
+            } catch {
+                throw HTTPError(.badRequest, message: error.localizedDescription)
+            }
         case .applicationUrlEncoded:
-            graphQLRequest = try await URLEncodedFormDecoder().decode(GraphQLRequest.self, from: request, context: context)
+            do {
+                graphQLRequest = try await URLEncodedFormDecoder().decode(GraphQLRequest.self, from: request, context: context)
+            } catch {
+                throw HTTPError(.badRequest, message: error.localizedDescription)
+            }
         default:
             throw HTTPError(.unsupportedMediaType)
         }
 
         let graphqlContext = try await computeContext(request, context)
-        let result = try await execute(
+        let result = await execute(
             graphQLRequest: graphQLRequest,
             context: graphqlContext,
             additionalValidationRules: config.additionalValidationRules
@@ -63,7 +71,7 @@ extension GraphQLHandler {
         graphQLRequest: GraphQLRequest,
         context: GraphQLContext,
         additionalValidationRules: [@Sendable (ValidationContext) -> Visitor]
-    ) async throws -> GraphQLResult {
+    ) async -> GraphQLResult {
         // https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#validation
         let validationRules = GraphQL.specifiedRules + additionalValidationRules
 
@@ -79,9 +87,11 @@ extension GraphQLHandler {
                 operationName: graphQLRequest.operationName,
                 validationRules: validationRules
             )
-        } catch {
+        } catch let error as GraphQLError {
             // This indicates a request parsing error
-            throw HTTPError(.badRequest, message: error.localizedDescription)
+            return GraphQLResult(data: nil, errors: [error])
+        } catch {
+            return GraphQLResult(data: nil, errors: [GraphQLError(message: error.localizedDescription)])
         }
         return result
     }

Tests/GraphQLHummingbirdTests/HTTPStatusCodeGraphQLJSONTests.swift

@@ -0,0 +1,195 @@
+import Foundation
+import GraphQL
+@testable import GraphQLHummingbird
+import GraphQLTransportWS
+import GraphQLWS
+import Hummingbird
+import HummingbirdTesting
+import NIOFoundationCompat
+import Testing
+
+/// Validates status code behavior for the `application/graphql-response+json` media type.
+///
+/// https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#applicationgraphql-responsejson
+@Suite
+struct HTTPStatusCodeGraphQLJSONTests {
+    @Test func parsingFailureGivesBadRequest() async throws {
+        // https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#json-parsing-failure-1
+        let router = Router()
+        router.graphql(schema: helloWorldSchema) { _, _ in
+            EmptyContext()
+        }
+        let app = Application(router: router)
+
+        try await app.test(.router) { client in
+            try await client.execute(
+                uri: "/graphql",
+                method: .post,
+                headers: jsonGraphQLHeaders,
+                body: .init(data: #require(#"{"query":"#.data(using: .utf8)))
+            ) { response in
+                #expect(response.status == .badRequest)
+            }
+        }
+    }
+
+    @Test func invalidParametersGivesBadRequest() async throws {
+        // https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#invalid-parameters-1
+        let router = Router()
+        router.graphql(schema: helloWorldSchema) { _, _ in
+            EmptyContext()
+        }
+        let app = Application(router: router)
+
+        try await app.test(.router) { client in
+            try await client.execute(
+                uri: "/graphql",
+                method: .post,
+                headers: jsonGraphQLHeaders,
+                body: .init(data: #require(#"{"qeury": "{__typename}"}"#.data(using: .utf8)))
+            ) { response in
+                #expect(response.status == .badRequest)
+            }
+        }
+    }
+
+    @Test func documentParsingFailureGivesBadRequest() async throws {
+        // https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#document-parsing-failure-1
+        let router = Router()
+        router.graphql(schema: helloWorldSchema) { _, _ in
+            EmptyContext()
+        }
+        let app = Application(router: router)
+
+        try await app.test(.router) { client in
+            try await client.execute(
+                uri: "/graphql",
+                method: .post,
+                headers: jsonGraphQLHeaders,
+                body: .init(data: #require(#"{"query": "{"}"#.data(using: .utf8)))
+            ) { response in
+                #expect(response.status == .badRequest)
+            }
+        }
+    }
+
+    @Test func documentValidationFailureGivesBadRequest() async throws {
+        // https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#document-validation-failure-1
+        let router = Router()
+        router.graphql(schema: helloWorldSchema) { _, _ in
+            EmptyContext()
+        }
+        let app = Application(router: router)
+
+        try await app.test(.router) { client in
+            try await client.execute(
+                uri: "/graphql",
+                method: .post,
+                headers: jsonGraphQLHeaders,
+                // Fails "No Unused Variables" validation rule
+                body: .init(data: #require(#"{"query": "query A($name: String) { hello }"}"#.data(using: .utf8)))
+            ) { response in
+                #expect(response.status == .badRequest)
+            }
+        }
+    }
+
+    @Test func operationCannotBeDeterminedGivesBadRequest() async throws {
+        // https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#operation-cannot-be-determined-1
+        let router = Router()
+        router.graphql(schema: helloWorldSchema) { _, _ in
+            EmptyContext()
+        }
+        let app = Application(router: router)
+
+        try await app.test(.router) { client in
+            try await client.execute(
+                uri: "/graphql",
+                method: .post,
+                headers: jsonGraphQLHeaders,
+                body: .init(data: #require(#"{"query": "abc { hello }"}"#.data(using: .utf8)))
+            ) { response in
+                #expect(response.status == .badRequest)
+            }
+        }
+    }
+
+    @Test func variableCoercionFailureGivesBadRequest() async throws {
+        // https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#variable-coercion-failure-1
+        let schema = try GraphQLSchema(
+            query: GraphQLObjectType(
+                name: "Query",
+                fields: [
+                    "get": GraphQLField(
+                        type: GraphQLString,
+                        args: [
+                            "name": GraphQLArgument(type: GraphQLString),
+                        ],
+                        resolve: { _, args, _, _ in
+                            guard let name = args["name"].string else {
+                                throw GraphQLError(message: "Name arg is required")
+                            }
+                            return name
+                        }
+                    ),
+                ]
+            )
+        )
+        let router = Router()
+        router.graphql(schema: schema) { _, _ in
+            EmptyContext()
+        }
+        let app = Application(router: router)
+
+        try await app.test(.router) { client in
+            try await client.execute(
+                uri: "/graphql",
+                method: .post,
+                headers: jsonGraphQLHeaders,
+                body: .init(data: #require(
+                    #"{"query": "query getName($name: String!) { get(name: $name) }", "variables": { "name": null }}"#.data(using: .utf8)
+                ))
+            ) { response in
+                #expect(response.status == .badRequest)
+            }
+        }
+    }
+
+    @Test func fieldErrorGivesOk() async throws {
+        // https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#field-errors-encountered-during-execution-1
+        let schema = try GraphQLSchema(
+            query: GraphQLObjectType(
+                name: "Query",
+                fields: [
+                    "error": GraphQLField(
+                        type: GraphQLString,
+                        resolve: { _, _, _, _ in
+                            throw GraphQLError(message: "Something went wrong")
+                        }
+                    ),
+                ]
+            )
+        )
+        let router = Router()
+        router.graphql(schema: schema) { _, _ in
+            EmptyContext()
+        }
+        let app = Application(router: router)
+
+        try await app.test(.router) { client in
+            try await client.execute(
+                uri: "/graphql",
+                method: .post,
+                headers: jsonGraphQLHeaders,
+                body: .init(data: JSONEncoder().encode(GraphQLRequest(query: "{ error }")))
+            ) { response in
+                #expect(response.status == .ok)
+                #expect(response.headers[.contentType] == "application/graphql-response+json; charset=utf-8")
+
+                let result = try JSONDecoder().decode(GraphQLResult.self, from: response.body)
+                #expect(!result.errors.isEmpty)
+                #expect(result.errors.first?.message == "Something went wrong")
+            }
+        }
+    }
+}