Skip to content

Commit 28631fd

Browse files
xperiandrixperiandri
committed
Fixed issue with the whole GraphQL request failure if exception is thrown in a collection field
Exceptions thrown during lazy sequence evaluation (e.g., Seq.toArray in the List case of `direct`) escaped the GraphQL error handling mechanism. This caused entire requests to fail instead of returning partial results with field-level errors for nullable fields (violating GraphQL spec §6.4.4). Wrapped the `onSuccess` call in `resolveWith` with try-catch, converting exceptions to field errors via `resolverError`. The Nullable case in `direct` already converts Error results to null + errors, so nullable field semantics now work correctly for post-processing failures.
1 parent e27f055 commit 28631fd

3 files changed

Lines changed: 167 additions & 2 deletions

File tree

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,10 @@ and private executeResolvers (inputContext : InputExecutionContextProvider) (ctx
376376
| Error errs -> return Error (errs @ additionalErrs)
377377
| Ok None -> return Error ((nullResolverError name path ctx) @ additionalErrs)
378378
| Ok (Some v) ->
379-
match! onSuccess ctx path parent v with
379+
let! onSuccessResult =
380+
try onSuccess ctx path parent v
381+
with e -> resolverError path ctx e |> Error |> AsyncVal.wrap
382+
match onSuccessResult with
380383
| Ok (res, deferred, errs) -> return Ok (res, deferred, errs @ additionalErrs)
381384
| Error errs -> return Error (errs @ additionalErrs)
382385
}

‎tests/FSharp.Data.GraphQL.Tests/FSharp.Data.GraphQL.Tests.fsproj‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
<Compile Include="MiddlewareTests.fs" />
7373
<Compile Include="FileTests.fs" />
7474
<Compile Include="AstExtensionsTests.fs" />
75+
<Compile Include="LazyEnumerationExceptionTests.fs" />
7576
<Compile Include="AspNetCore/TestSchema.fs" />
7677
<Compile Include="AspNetCore/InvalidMessageTests.fs" />
7778
<Compile Include="AspNetCore/SerializationTests.fs" />
@@ -89,4 +90,4 @@
8990
<ProjectReference Include="..\..\src\FSharp.Data.GraphQL.Server.AspNetCore\FSharp.Data.GraphQL.Server.AspNetCore.fsproj" />
9091
<ProjectReference Include="..\..\src\FSharp.Data.GraphQL.Server.Middleware\FSharp.Data.GraphQL.Server.Middleware.fsproj" />
9192
</ItemGroup>
92-
</Project>
93+
</Project>

‎tests/FSharp.Data.GraphQL.Tests/LazyEnumerationExceptionTests.fs‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
// The MIT License (MIT)
2+
// Copyright (c) 2016 Bazinga Technologies Inc
3+
4+
module FSharp.Data.GraphQL.Tests.LazyEnumerationExceptionTests
5+
6+
open System.Collections.Generic
7+
open Xunit
8+
9+
#nowarn "0025"
10+
11+
open FSharp.Data.GraphQL
12+
open FSharp.Data.GraphQL.Types
13+
open FSharp.Data.GraphQL.Parser
14+
open FSharp.Data.GraphQL.Execution
15+
16+
type TestItem = { Name: string }
17+
18+
let TestItemType =
19+
Define.Object<TestItem>(
20+
"TestItem", [
21+
Define.Field("name", StringType, fun _ x -> x.Name)
22+
])
23+
24+
[<Fact>]
25+
let ``Execution must return null with field error when nullable list field throws during lazy enumeration`` () =
26+
let schema =
27+
Schema(
28+
Define.Object<unit>(
29+
"Query", [
30+
Define.Field(
31+
"tags",
32+
Nullable (ListOf StringType),
33+
fun _ _ ->
34+
seq {
35+
yield "first"
36+
failwith "Boom during enumeration"
37+
}
38+
|> Some
39+
)
40+
]))
41+
let expectedData =
42+
NameValueLookup.ofList [
43+
"tags", null
44+
]
45+
let expectedErrors =
46+
[
47+
GQLProblemDetails.CreateWithKind ("Boom during enumeration", Execution, [ box "tags" ])
48+
]
49+
let result = sync <| Executor(schema).AsyncExecute(parse "{ tags }", getMockInputContext, ())
50+
ensureDirect result <| fun data errors ->
51+
data |> equals (upcast expectedData)
52+
errors |> equals expectedErrors
53+
54+
[<Fact>]
55+
let ``Execution must return null with field error when struct nullable list field throws during lazy enumeration`` () =
56+
let schema =
57+
Schema(
58+
Define.Object<unit>(
59+
"Query", [
60+
Define.Field(
61+
"tags",
62+
StructNullable (ListOf StringType),
63+
fun _ _ ->
64+
seq {
65+
yield "first"
66+
failwith "Boom during enumeration"
67+
}
68+
|> ValueSome
69+
)
70+
]))
71+
let expectedData =
72+
NameValueLookup.ofList [
73+
"tags", null
74+
]
75+
let expectedErrors =
76+
[
77+
GQLProblemDetails.CreateWithKind ("Boom during enumeration", Execution, [ box "tags" ])
78+
]
79+
let result = sync <| Executor(schema).AsyncExecute(parse "{ tags }", getMockInputContext, ())
80+
ensureDirect result <| fun data errors ->
81+
data |> equals (upcast expectedData)
82+
errors |> equals expectedErrors
83+
84+
[<Fact>]
85+
let ``Execution must propagate error when non-nullable list field throws during lazy enumeration`` () =
86+
let schema =
87+
Schema(
88+
Define.Object<unit>(
89+
"Query", [
90+
Define.Field(
91+
"tags",
92+
ListOf StringType,
93+
fun _ _ ->
94+
seq {
95+
yield "first"
96+
failwith "Boom during enumeration"
97+
}
98+
)
99+
]))
100+
let expectedError = GQLProblemDetails.CreateWithKind ("Boom during enumeration", Execution, [ box "tags" ])
101+
let result = sync <| Executor(schema).AsyncExecute(parse "{ tags }", getMockInputContext, ())
102+
ensureRequestError result <| fun [ error ] -> error |> equals expectedError
103+
104+
[<Fact>]
105+
let ``Execution must return null with field error when nullable list of objects throws KeyNotFoundException during lazy enumeration`` () =
106+
let tagsById = dict [ "a", { Name = "Alpha" } ]
107+
let schema =
108+
Schema(
109+
Define.Object<unit>(
110+
"Query", [
111+
Define.Field(
112+
"tags",
113+
Nullable (ListOf TestItemType),
114+
fun _ _ ->
115+
[ "a"; "b" ]
116+
|> Seq.map (fun id -> tagsById[id])
117+
|> Some
118+
)
119+
]))
120+
let result = sync <| Executor(schema).AsyncExecute(parse "{ tags { name } }", getMockInputContext, ())
121+
ensureDirect result <| fun data errors ->
122+
let expectedData =
123+
NameValueLookup.ofList [
124+
"tags", null
125+
]
126+
data |> equals (upcast expectedData)
127+
equals 1 errors.Length
128+
hasError "was not present in the dictionary." errors
129+
130+
[<Fact>]
131+
let ``Execution must return partial result when sibling nullable field throws during lazy enumeration`` () =
132+
let schema =
133+
Schema(
134+
Define.Object<unit>(
135+
"Query", [
136+
Define.Field(
137+
"tags",
138+
Nullable (ListOf StringType),
139+
fun _ _ ->
140+
seq {
141+
yield "first"
142+
failwith "Boom during enumeration"
143+
}
144+
|> Some
145+
)
146+
Define.Field(
147+
"name",
148+
StringType,
149+
fun _ _ -> "Hello"
150+
)
151+
]))
152+
let expectedData =
153+
NameValueLookup.ofList [
154+
"tags", null
155+
"name", upcast "Hello"
156+
]
157+
let result = sync <| Executor(schema).AsyncExecute(parse "{ tags name }", getMockInputContext, ())
158+
ensureDirect result <| fun data errors ->
159+
data |> equals (upcast expectedData)
160+
equals 1 errors.Length
161+
hasError "Boom during enumeration" errors

0 commit comments

Comments
 (0)