Declare that GraphQL response data can be optional and skippable

RELEASE_NOTES.md

@@ -291,7 +291,7 @@
 * **Breaking Change** `SubscriptionExecutionResult.Data` is now `obj Skippable`, and the record has new `Path` and `HasNext` fields for incremental delivery
 * **Breaking Change** `BufferedStreamOptions.Interval` and `BufferedStreamOptions.PreferredBatchSize` are now `int voption`
 * **Breaking Change** `ServerMessage.Error` and `ServerRawPayload.ErrorMessages` now carry `GQLProblemDetails list` instead of `NameValueLookup list`, so an `error` message's `payload` is a standard GraphQL error array as the `graphql-transport-ws` protocol requires
-* **Breaking Change** A query or mutation whose non-null root field fails during execution now produces a `Direct` (execution) result with `null` data instead of a `RequestError`, which is now only ever produced for a request rejected before execution (validation, planning, variable or inline argument coercion, a middleware, or the executor itself failing); HTTP and `graphql-transport-ws` responses for such a failure now carry `data: null` as the spec requires, instead of omitting `data` entirely
+* **Breaking Change** A query or mutation whose non-null root field fails during execution now produces a `Direct` (execution) result with `null` data instead of a `RequestError`, which is now only ever produced for a request rejected before execution (validation, planning, variable or inline argument coercion, a middleware, or the executor itself failing); HTTP and `graphql-transport-ws` responses for such a failure now carry `data: null` as the spec requires, instead of omitting `data` entirely. This also changes the public `GQLResponse.Data`, `GQLResponseContent.Direct.Data`, `DeferredErrors.Data`, and `SubscriptionErrors.Data` signatures to use `voption`
 * Added case-insensitive string comparison support to `ObjectListFilter`, including comparer-aware filter cases and GraphQL filter suffix handling
 * Improved Relay XML documentation comments
 * Changed query planning to throw `MalformedGQLQueryException` for invalid queries, `NotSupportedException` for unsupported type definition implementations and `InvalidOperationException` for internal planning errors instead of `System.Exception`, with messages naming the affected field, type and execution kind

src/FSharp.Data.GraphQL.Server.AspNetCore/GraphQLRequestHandler.fs

@@ -55,9 +55,9 @@ and [<AbstractClass>] GraphQLRequestHandler<'Root>
             logger.LogDebug ("Produced direct GraphQL response with documentId = '{documentId}' and metadata:\n{metadata}", documentId, metadata)
 
             if logger.IsEnabled LogLevel.Trace then
-                logger.LogTrace ("GraphQL response data:\n{data}", serializeIndented data)
+                logger.LogTrace ("GraphQL response data:\n{data}", serializeIndented (data |> ValueOption.toObj))
 
-            GQLResponse.Direct (documentId, data, errs)
+            GQLResponse.Direct (documentId, data |> ValueOption.toObj, errs)
         | Deferred (data, errs, deferred) ->
             logger.LogDebug ("Produced deferred GraphQL response with documentId = '{documentId}' and metadata:\n{metadata}", documentId, metadata)
 
@@ -69,12 +69,12 @@ and [<AbstractClass>] GraphQLRequestHandler<'Root>
 
                         if logger.IsEnabled LogLevel.Trace then
                             logger.LogTrace ("GraphQL deferred data:\n{data}", serializeIndented data)
-                    | DeferredErrors (null, errors, path) ->
+                    | DeferredErrors (ValueNone, errors, path) ->
                         logger.LogDebug ("Produced GraphQL deferred errors for path: {path}", path |> Seq.map string |> Seq.toArray |> Path.Join)
 
                         if logger.IsEnabled LogLevel.Trace then
                             logger.LogTrace ("GraphQL deferred errors:\n{errors}", errors)
-                    | DeferredErrors (data, errors, path) ->
+                    | DeferredErrors (ValueSome data, errors, path) ->
                         logger.LogDebug (
                             "Produced GraphQL deferred result with errors for path: {path}",
                             path |> Seq.map string |> Seq.toArray |> Path.Join
@@ -96,12 +96,12 @@ and [<AbstractClass>] GraphQLRequestHandler<'Root>
 
                         if logger.IsEnabled LogLevel.Trace then
                             logger.LogTrace ("GraphQL subscription data:\n{data}", serializeIndented data)
-                    | SubscriptionErrors (null, errors) ->
+                    | SubscriptionErrors (ValueNone, errors) ->
                         logger.LogDebug ("Produced GraphQL subscription errors")
 
                         if logger.IsEnabled LogLevel.Trace then
                             logger.LogTrace ("GraphQL subscription errors:\n{errors}", errors)
-                    | SubscriptionErrors (data, errors) ->
+                    | SubscriptionErrors (ValueSome data, errors) ->
                         logger.LogDebug ("Produced GraphQL subscription result with errors")
 
                         if logger.IsEnabled LogLevel.Trace then

src/FSharp.Data.GraphQL.Server.AspNetCore/GraphQLWebsocketMiddleware.fs

@@ -246,8 +246,8 @@ type GraphQLWebSocketMiddleware<'Root>
                 logger.LogWarning ("Subscription errors: {subscriptionErrors}", (String.Join ('\n', errors |> Seq.map (fun x -> $"- %s{x.Message}"))))
                 // The executor may still have resolved partial data alongside the field errors; forward it as-is
                 match output with
-                | null -> SubscriptionExecutionResult.CreateErrors errors |> sendOutput id
-                | output -> SubscriptionExecutionResult.Create (output, errors) |> sendOutput id
+                | ValueNone -> SubscriptionExecutionResult.CreateErrors errors |> sendOutput id
+                | ValueSome output -> SubscriptionExecutionResult.Create (output, errors) |> sendOutput id
 
         // Incremental payloads are sent as soon as they are produced, with their path inside the initial result,
         // so a client can merge them. The completion marker becomes a final payload with hasNext set to false.
@@ -260,7 +260,7 @@ type GraphQLWebSocketMiddleware<'Root>
                     do! SubscriptionExecutionResult.CreateIncremental (itemData, [], itemPath) |> sendOutput id
             | ValueSome (DeferredResult (data, path)) ->
                 do! SubscriptionExecutionResult.CreateIncremental (data, [], path) |> sendOutput id
-            | ValueSome (DeferredErrors (data, errors, BatchPath (fieldPath, indices))) ->
+            | ValueSome (DeferredErrors (ValueSome data, errors, BatchPath (fieldPath, indices))) ->
                 logger.LogWarning (
                     "Deferred response errors: {deferredErrors}",
                     (String.Join ('\n', errors |> Seq.map (fun x -> $"- %s{x.Message}")))
@@ -272,7 +272,7 @@ type GraphQLWebSocketMiddleware<'Root>
                     "Deferred response errors: {deferredErrors}",
                     (String.Join ('\n', errors |> Seq.map (fun x -> $"- %s{x.Message}")))
                 )
-                do! SubscriptionExecutionResult.CreateIncremental (data, errors, path) |> sendOutput id
+                do! SubscriptionExecutionResult.CreateIncremental (data |> ValueOption.toObj, errors, path) |> sendOutput id
             | ValueNone -> do! SubscriptionExecutionResult.CreateCompleted () |> sendOutput id
         }
 
@@ -291,7 +291,7 @@ type GraphQLWebSocketMiddleware<'Root>
                 // message below
                 if not errors.IsEmpty then
                     logger.LogWarning ("Execution errors:\n{errors}", errors)
-                do! SubscriptionExecutionResult.Create (data, errors) |> sendOutput id
+                do! SubscriptionExecutionResult.Create (data |> ValueOption.toObj, errors) |> sendOutput id
                 // The graphql-transport-ws protocol requires Complete after the single Next of a query or mutation
                 do! sendMsg (Complete id)
             | RequestError problemDetails ->

src/FSharp.Data.GraphQL.Server/Execution.fs

@@ -161,10 +161,10 @@ let deferResults path (res : ResolverResult<obj>) : IObservable<GQLDeferredRespo
         let deferredData =
             match errs with
             | [] -> DeferredResult (data, formattedPath)
-            | _ -> DeferredErrors (data, errs, formattedPath)
+            | _ -> DeferredErrors (data |> ValueOption.ofObj, errs, formattedPath)
             |> Observable.singleton
         Option.foldBack Observable.concat deferred deferredData
-    | Error errs -> Observable.singleton <| DeferredErrors (null, errs, formattedPath)
+    | Error errs -> Observable.singleton <| DeferredErrors (ValueNone, errs, formattedPath)
 
 /// Collect together an array of results using the appropriate execution strategy.
 let collectFields (strategy : ExecutionStrategy) (rs : AsyncVal<ResolverResult<KeyValuePair<string, obj>>> []) : AsyncVal<ResolverResult<KeyValuePair<string, obj> []>> = asyncVal {
@@ -327,7 +327,7 @@ and private streamed (options : BufferedStreamOptions) (innerDef : OutputDef) (i
             ||> List.foldBack (fun event struct (items, failures) ->
                 match event with
                 | StreamedItem (index, result) -> struct (index, result) :: items, failures
-                | StreamFailure error -> items, DeferredErrors (null, resolverError path ctx error, normalizeErrorPath path) :: failures)
+                | StreamFailure error -> items, DeferredErrors (ValueNone, resolverError path ctx error, normalizeErrorPath path) :: failures)
         match failures with
         | [] -> collectItems items
         | failures -> collectItems items |> Observable.concat (Observable.ofSeq failures)
@@ -587,9 +587,9 @@ let private executeSubscription (resultSet: (string * ExecutionInfo) []) (inputC
     let onValue v = asyncVal {
             match! executeResolvers inputContext fieldCtx fieldPath value (toOption v |> AsyncVal.wrap) with
             | Ok (data, None, []) -> return SubscriptionResult (NameValueLookup.ofList [nameOrAlias, data.Value])
-            | Ok (data, None, errs) -> return SubscriptionErrors (NameValueLookup.ofList [nameOrAlias, data.Value], errs)
+            | Ok (data, None, errs) -> return SubscriptionErrors (ValueSome (NameValueLookup.ofList [nameOrAlias, data.Value]), errs)
             | Ok (_, Some _, _) -> return failwith "Deferred/Streamed/Live are not supported for subscriptions!"
-            | Error errs -> return SubscriptionErrors (null, errs)
+            | Error errs -> return SubscriptionErrors (ValueNone, errs)
         }
     return
         ctx.Schema.SubscriptionProvider.Add fieldCtx value subDef

src/FSharp.Data.GraphQL.Server/Executor.fs

@@ -104,7 +104,7 @@ type Executor<'Root>(schema: ISchema<'Root>, middlewares : IExecutorMiddleware s
         let prepareOutput res =
             match res with
             | RequestError errs -> GQLExecutionResult.Error (documentId, errs, res.Metadata)
-            | Direct (data, errors) -> GQLExecutionResult.Direct (documentId, data, errors, res.Metadata)
+            | Direct (data, errors) -> GQLExecutionResult.Direct (documentId, data |> ValueOption.toObj, errors, res.Metadata)
             | Deferred (data, errors, deferred) -> GQLExecutionResult.Deferred (documentId, data, errors, deferred, res.Metadata)
             | Stream (stream) -> GQLExecutionResult.Stream (documentId, stream, res.Metadata)
         async {

src/FSharp.Data.GraphQL.Server/IO.fs

@@ -9,83 +9,72 @@ open FSharp.Data.GraphQL.Types
 
 type Output = IDictionary<string, obj>
 
-type GQLResponse =
-    { DocumentId: int
-      Data : Output Skippable
-      Errors : GQLProblemDetails list Skippable }
-    static member Direct(documentId, data, errors) =
-        { DocumentId = documentId
-          Data = Include data
-          Errors = Skippable.ofList errors }
-    static member Stream(documentId) =
-        { DocumentId = documentId
-          Data = Include null
-          Errors = Skip }
-    static member RequestError(documentId, errors) =
-        { DocumentId = documentId
-          Data = Skip
-          Errors = Include errors }
+type GQLResponse = {
+    DocumentId : int
+    Data : Skippable<Output voption>
+    Errors : Skippable<GQLProblemDetails list>
+} with
 
-type GQLExecutionResult =
-    { DocumentId: int
-      Content : GQLResponseContent
-      Metadata : Metadata }
-    static member Direct(documentId, data, errors, meta) =
-        { DocumentId = documentId
-          Content = Direct (data, errors)
-          Metadata = meta }
-    static member Deferred(documentId, data, errors, deferred, meta) =
-        { DocumentId = documentId
-          Content = Deferred (data, errors, deferred)
-          Metadata = meta }
-    static member Stream(documentId, data, meta) =
-        { DocumentId = documentId
-          Content = Stream data
-          Metadata = meta }
-    static member RequestError(documentId, errors, meta) =
-        { DocumentId = documentId
-          Content = RequestError errors
-          Metadata = meta }
-    static member Empty(documentId, meta) =
-        GQLExecutionResult.Direct(documentId, Map.empty, [], meta)
-    static member Error(documentId, errors, meta) =
-        GQLExecutionResult.RequestError(documentId, errors, meta)
-    static member Error(documentId, error, meta) =
-        GQLExecutionResult.RequestError(documentId, [ error ], meta)
-    static member Error(documentId, error, meta) =
-        GQLExecutionResult.RequestError(documentId, [ GQLProblemDetails.OfError error ], meta)
-    static member Error(documentId, errors, meta) =
-        GQLExecutionResult.RequestError(documentId, errors |> List.map GQLProblemDetails.OfError, meta)
-    static member Error(documentId, msg, meta) =
-        GQLExecutionResult.RequestError(documentId, [ GQLProblemDetails.Create msg ], meta)
+    static member Direct (documentId, data : Output | null, errors) = {
+        DocumentId = documentId
+        Data = Include (Option.ofObj data |> ValueOption.ofOption)
+        Errors = Skippable.ofList errors
+    }
+    static member Stream (documentId) = { DocumentId = documentId; Data = Include ValueNone; Errors = Skip }
+    static member RequestError (documentId, errors) = { DocumentId = documentId; Data = Skip; Errors = Include errors }
 
-    static member ErrorFromException(documentId : int, ex : Exception, meta : Metadata) =
-        GQLExecutionResult.RequestError(documentId, [ GQLProblemDetails.Create (ex.Message, ex) ], meta)
+type GQLExecutionResult = {
+    DocumentId : int
+    Content : GQLResponseContent
+    Metadata : Metadata
+} with
 
-    static member Invalid(documentId, errors, meta) =
-        GQLExecutionResult.RequestError(documentId, errors, meta)
-    static member ErrorAsync(documentId, msg : string, meta) =
-        AsyncVal.wrap (GQLExecutionResult.Error (documentId, msg, meta))
-    static member ErrorAsync(documentId, error : IGQLError, meta) =
-        AsyncVal.wrap (GQLExecutionResult.Error (documentId, error, meta))
+    static member Direct (documentId, data : Output | null, errors, meta) = {
+        DocumentId = documentId
+        Content = Direct (Option.ofObj data |> ValueOption.ofOption, errors)
+        Metadata = meta
+    }
+    static member Deferred (documentId, data, errors, deferred, meta) = {
+        DocumentId = documentId
+        Content = Deferred (data, errors, deferred)
+        Metadata = meta
+    }
+    static member Stream (documentId, data, meta) = { DocumentId = documentId; Content = Stream data; Metadata = meta }
+    static member RequestError (documentId, errors, meta) = { DocumentId = documentId; Content = RequestError errors; Metadata = meta }
+    static member Empty (documentId, meta) = GQLExecutionResult.Direct (documentId, Map.empty, [], meta)
+    static member Error (documentId, errors, meta) = GQLExecutionResult.RequestError (documentId, errors, meta)
+    static member Error (documentId, error, meta) = GQLExecutionResult.RequestError (documentId, [ error ], meta)
+    static member Error (documentId, error, meta) =
+        GQLExecutionResult.RequestError (documentId, [ GQLProblemDetails.OfError error ], meta)
+    static member Error (documentId, errors, meta) =
+        GQLExecutionResult.RequestError (documentId, errors |> List.map GQLProblemDetails.OfError, meta)
+    static member Error (documentId, msg, meta) =
+        GQLExecutionResult.RequestError (documentId, [ GQLProblemDetails.Create msg ], meta)
+
+    static member ErrorFromException (documentId : int, ex : Exception, meta : Metadata) =
+        GQLExecutionResult.RequestError (documentId, [ GQLProblemDetails.Create (ex.Message, ex) ], meta)
+
+    static member Invalid (documentId, errors, meta) = GQLExecutionResult.RequestError (documentId, errors, meta)
+    static member ErrorAsync (documentId, msg : string, meta) = AsyncVal.wrap (GQLExecutionResult.Error (documentId, msg, meta))
+    static member ErrorAsync (documentId, error : IGQLError, meta) = AsyncVal.wrap (GQLExecutionResult.Error (documentId, error, meta))
 
 // TODO: Rename to PascalCase
 and GQLResponseContent =
     /// The request was rejected before execution started: validation, planning, variable or inline argument
     /// coercion, a middleware, or the executor itself failing. There is no data, unlike a Direct result whose
     /// data happens to be null.
-    | RequestError of Errors: GQLProblemDetails list
+    | RequestError of Errors : GQLProblemDetails list
     /// An execution result. Data is null when a non-null root field failed during execution and the error
     /// propagated to the root, exactly as it would for a non-null nested field, rather than being rejected as a
     /// RequestError.
-    | Direct of Data : Output * Errors: GQLProblemDetails list
+    | Direct of Data : Output voption * Errors : GQLProblemDetails list
     | Deferred of Data : Output * Errors : GQLProblemDetails list * Defer : IObservable<GQLDeferredResponseContent>
     | Stream of Stream : IObservable<GQLSubscriptionResponseContent>
 
 and GQLDeferredResponseContent =
     | DeferredResult of Data : obj * Path : FieldPath
-    | DeferredErrors of Data : obj * Errors: GQLProblemDetails list * Path : FieldPath
+    | DeferredErrors of Data : obj voption * Errors : GQLProblemDetails list * Path : FieldPath
 
 and GQLSubscriptionResponseContent =
     | SubscriptionResult of Data : Output
-    | SubscriptionErrors of Data : Output * Errors: GQLProblemDetails list
+    | SubscriptionErrors of Data : Output voption * Errors : GQLProblemDetails list

tests/FSharp.Data.GraphQL.Tests/DeferredTests.fs

@@ -275,7 +275,7 @@ let ``Resolver error`` () =
         ]
     let expectedDeferred =
         DeferredErrors (
-            null,
+            ValueNone,
             [ GQLProblemDetails.CreateWithKind ("Resolver error!", Execution, [ box "testData"; "resolverError"; "value" ]) ],
             [ "testData"; "resolverError" ]
         )
@@ -304,13 +304,13 @@ let ``Resolver list error`` () =
         ]
     let expectedDeferred1 =
         DeferredErrors (
-            null,
+            ValueNone,
             [ GQLProblemDetails.CreateWithKind ("Resolver error!", Execution, [ box "testData"; "resolverListError"; 0; "value" ]) ],
             [ box "testData"; "resolverListError"; 0 ]
         )
     let expectedDeferred2 =
         DeferredErrors (
-            null,
+            ValueNone,
             [ GQLProblemDetails.CreateWithKind ("Resolver error!", Execution, [ box "testData"; "resolverListError"; 1; "value" ]) ],
             [ box "testData"; "resolverListError"; 1 ]
         )
@@ -343,7 +343,7 @@ let ``Nullable error`` () =
         ]
     let expectedDeferred =
         DeferredErrors (
-            null,
+            ValueNone,
             [ GQLProblemDetails.CreateWithKind ("Non-Null field value resolved as a null!", Execution, [ box "testData"; "nullableError"; "value" ]) ],
             [ "testData"; "nullableError" ]
         )