Merge pull request #124 from CrownedPhoenix/fix/serial-execution-strategy

fix: Serial Execution Strategy

Author: NeedleInAJayStack

Sources/GraphQL/Execution/Execute.swift

@@ -118,24 +118,29 @@ public struct SerialFieldExecutionStrategy: QueryFieldExecutionStrategy,
         path: IndexPath,
         fields: OrderedDictionary<String, [Field]>
     ) throws -> Future<OrderedDictionary<String, Any>> {
-        var results = OrderedDictionary<String, Future<Any>>()
-
-        try fields.forEach { field in
-            let fieldASTs = field.value
-            let fieldPath = path.appending(field.key)
-
-            let result = try resolveField(
-                exeContext: exeContext,
-                parentType: parentType,
-                source: sourceValue,
-                fieldASTs: fieldASTs,
-                path: fieldPath
-            )
-
-            results[field.key] = result.map { $0 ?? Map.null }
-        }
-
-        return results.flatten(on: exeContext.eventLoopGroup)
+        var results = OrderedDictionary<String, Any>()
+
+        return fields
+            .reduce(exeContext.eventLoopGroup.next().makeSucceededVoidFuture()) { prev, field in
+                // We use ``flatSubmit`` here to avoid a stack overflow issue with EventLoopFutures.
+                // See: https://github.com/apple/swift-nio/issues/970
+                exeContext.eventLoopGroup.next().flatSubmit {
+                    prev.tryFlatMap {
+                        let fieldASTs = field.value
+                        let fieldPath = path.appending(field.key)
+
+                        return try resolveField(
+                            exeContext: exeContext,
+                            parentType: parentType,
+                            source: sourceValue,
+                            fieldASTs: fieldASTs,
+                            path: fieldPath
+                        ).map { result in
+                            results[field.key] = result ?? Map.null
+                        }
+                    }
+                }
+            }.map { results }
     }
 }
 

Sources/GraphQL/Utilities/NIO+Extensions.swift

@@ -101,3 +101,31 @@ public protocol FutureType {
 extension Future: FutureType {
     public typealias Expectation = Value
 }
+
+// Copied from https://github.com/vapor/async-kit/blob/e2f741640364c1d271405da637029ea6a33f754e/Sources/AsyncKit/EventLoopFuture/Future%2BTry.swift
+// in order to avoid full package dependency.
+public extension EventLoopFuture {
+    func tryFlatMap<NewValue>(
+        file _: StaticString = #file, line _: UInt = #line,
+        _ callback: @escaping (Value) throws -> EventLoopFuture<NewValue>
+    ) -> EventLoopFuture<NewValue> {
+        /// When the current `EventLoopFuture<Value>` is fulfilled, run the provided callback,
+        /// which will provide a new `EventLoopFuture`.
+        ///
+        /// This allows you to dynamically dispatch new asynchronous tasks as phases in a
+        /// longer series of processing steps. Note that you can use the results of the
+        /// current `EventLoopFuture<Value>` when determining how to dispatch the next operation.
+        ///
+        /// The key difference between this method and the regular `flatMap` is  error handling.
+        ///
+        /// With `tryFlatMap`, the provided callback _may_ throw Errors, causing the returned `EventLoopFuture<Value>`
+        /// to report failure immediately after the completion of the original `EventLoopFuture`.
+        flatMap { [eventLoop] value in
+            do {
+                return try callback(value)
+            } catch {
+                return eventLoop.makeFailedFuture(error)
+            }
+        }
+    }
+}

Tests/GraphQLTests/FieldExecutionStrategyTests/FieldExecutionStrategyTests.swift

@@ -15,15 +15,10 @@ class FieldExecutionStrategyTests: XCTestCase {
                 "sleep": GraphQLField(
                     type: GraphQLString,
                     resolve: { _, _, _, eventLoopGroup, _ in
-                        let group = DispatchGroup()
-                        group.enter()
-
-                        DispatchQueue.global().asyncAfter(wallDeadline: .now() + 0.1) {
-                            group.leave()
+                        eventLoopGroup.next().makeSucceededVoidFuture().map {
+                            Thread.sleep(forTimeInterval: 0.1)
+                            return "z"
                         }
-
-                        group.wait()
-                        return eventLoopGroup.next().makeSucceededFuture("z")
                     }
                 ),
                 "bang": GraphQLField(
@@ -251,7 +246,7 @@ class FieldExecutionStrategyTests: XCTestCase {
             eventLoopGroup: eventLoopGroup
         ).wait())
         XCTAssertEqual(result.value, multiExpected)
-        // XCTAssertEqualWithAccuracy(1.0, result.seconds, accuracy: 0.5)
+//        XCTAssertEqualWithAccuracy(1.0, result.seconds, accuracy: 0.5)
     }
 
     func testSerialFieldExecutionStrategyWithMultipleFieldErrors() throws {
@@ -300,7 +295,7 @@ class FieldExecutionStrategyTests: XCTestCase {
             eventLoopGroup: eventLoopGroup
         ).wait())
         XCTAssertEqual(result.value, multiExpected)
-        // XCTAssertEqualWithAccuracy(0.1, result.seconds, accuracy: 0.25)
+//        XCTAssertEqualWithAccuracy(0.1, result.seconds, accuracy: 0.25)
     }
 
     func testConcurrentDispatchFieldExecutionStrategyWithMultipleFieldErrors() throws {