Skip to content

Commit 05ef9fc

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 05ef9fc

3 files changed

Lines changed: 298 additions & 2 deletions

File tree

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,8 +376,12 @@ 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)
384+
| Error errs when ctx.ExecutionInfo.IsNullable -> return Ok (KeyValuePair(name, null), None, errs @ additionalErrs)
381385
| Error errs -> return Error (errs @ additionalErrs)
382386
}
383387

‎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: 291 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,291 @@
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+
type TestContainer = {
19+
Title: string
20+
Tags: string seq
21+
Count: int
22+
}
23+
24+
let TestItemType =
25+
Define.Object<TestItem>(
26+
"TestItem", [
27+
Define.Field("name", StringType, fun _ x -> x.Name)
28+
])
29+
30+
let TestContainerType =
31+
Define.Object<TestContainer>(
32+
"TestContainer", [
33+
Define.Field("title", StringType, fun _ x -> x.Title)
34+
Define.Field(
35+
"tags",
36+
Nullable (ListOf StringType),
37+
fun _ x -> x.Tags |> Some
38+
)
39+
Define.Field("count", IntType, fun _ x -> x.Count)
40+
])
41+
42+
[<Fact>]
43+
let ``Execution must return null with field error when nullable list field throws during lazy enumeration`` () =
44+
let schema =
45+
Schema(
46+
Define.Object<unit>(
47+
"Query", [
48+
Define.Field(
49+
"tags",
50+
Nullable (ListOf StringType),
51+
fun _ _ ->
52+
seq {
53+
yield "first"
54+
failwith "Boom during enumeration"
55+
}
56+
|> Some
57+
)
58+
]))
59+
let expectedData =
60+
NameValueLookup.ofList [
61+
"tags", null
62+
]
63+
let expectedErrors =
64+
[
65+
GQLProblemDetails.CreateWithKind ("Boom during enumeration", Execution, [ box "tags" ])
66+
]
67+
let result = sync <| Executor(schema).AsyncExecute(parse "{ tags }", getMockInputContext, ())
68+
ensureDirect result <| fun data errors ->
69+
data |> equals (upcast expectedData)
70+
errors |> equals expectedErrors
71+
72+
[<Fact>]
73+
let ``Execution must return null with field error when struct nullable list field throws during lazy enumeration`` () =
74+
let schema =
75+
Schema(
76+
Define.Object<unit>(
77+
"Query", [
78+
Define.Field(
79+
"tags",
80+
StructNullable (ListOf StringType),
81+
fun _ _ ->
82+
seq {
83+
yield "first"
84+
failwith "Boom during enumeration"
85+
}
86+
|> ValueSome
87+
)
88+
]))
89+
let expectedData =
90+
NameValueLookup.ofList [
91+
"tags", null
92+
]
93+
let expectedErrors =
94+
[
95+
GQLProblemDetails.CreateWithKind ("Boom during enumeration", Execution, [ box "tags" ])
96+
]
97+
let result = sync <| Executor(schema).AsyncExecute(parse "{ tags }", getMockInputContext, ())
98+
ensureDirect result <| fun data errors ->
99+
data |> equals (upcast expectedData)
100+
errors |> equals expectedErrors
101+
102+
[<Fact>]
103+
let ``Execution must propagate error when non-nullable list field throws during lazy enumeration`` () =
104+
let schema =
105+
Schema(
106+
Define.Object<unit>(
107+
"Query", [
108+
Define.Field(
109+
"tags",
110+
ListOf StringType,
111+
fun _ _ ->
112+
seq {
113+
yield "first"
114+
failwith "Boom during enumeration"
115+
}
116+
)
117+
]))
118+
let expectedError = GQLProblemDetails.CreateWithKind ("Boom during enumeration", Execution, [ box "tags" ])
119+
let result = sync <| Executor(schema).AsyncExecute(parse "{ tags }", getMockInputContext, ())
120+
ensureRequestError result <| fun [ error ] -> error |> equals expectedError
121+
122+
[<Fact>]
123+
let ``Execution must return null with field error when nullable list of objects throws KeyNotFoundException during lazy enumeration`` () =
124+
let tagsById = dict [ "a", { Name = "Alpha" } ]
125+
let schema =
126+
Schema(
127+
Define.Object<unit>(
128+
"Query", [
129+
Define.Field(
130+
"tags",
131+
Nullable (ListOf TestItemType),
132+
fun _ _ ->
133+
[ "a"; "b" ]
134+
|> Seq.map (fun id -> tagsById[id])
135+
|> Some
136+
)
137+
]))
138+
let result = sync <| Executor(schema).AsyncExecute(parse "{ tags { name } }", getMockInputContext, ())
139+
ensureDirect result <| fun data errors ->
140+
let expectedData =
141+
NameValueLookup.ofList [
142+
"tags", null
143+
]
144+
data |> equals (upcast expectedData)
145+
equals 1 errors.Length
146+
hasError "was not present in the dictionary." errors
147+
148+
[<Fact>]
149+
let ``Execution must return partial result when sibling nullable field throws during lazy enumeration`` () =
150+
let schema =
151+
Schema(
152+
Define.Object<unit>(
153+
"Query", [
154+
Define.Field(
155+
"tags",
156+
Nullable (ListOf StringType),
157+
fun _ _ ->
158+
seq {
159+
yield "first"
160+
failwith "Boom during enumeration"
161+
}
162+
|> Some
163+
)
164+
Define.Field(
165+
"name",
166+
StringType,
167+
fun _ _ -> "Hello"
168+
)
169+
]))
170+
let expectedData =
171+
NameValueLookup.ofList [
172+
"tags", null
173+
"name", upcast "Hello"
174+
]
175+
let result = sync <| Executor(schema).AsyncExecute(parse "{ tags name }", getMockInputContext, ())
176+
ensureDirect result <| fun data errors ->
177+
data |> equals (upcast expectedData)
178+
equals 1 errors.Length
179+
hasError "Boom during enumeration" errors
180+
181+
[<Fact>]
182+
let ``Execution must return sibling fields on nested object when nullable list throws during lazy enumeration`` () =
183+
let container = {
184+
Title = "Meeting"
185+
Tags = seq {
186+
yield "first"
187+
failwith "Boom during enumeration"
188+
}
189+
Count = 42
190+
}
191+
let schema =
192+
Schema(
193+
Define.Object<unit>(
194+
"Query", [
195+
Define.Field("container", TestContainerType, fun _ _ -> container)
196+
]))
197+
let expectedData =
198+
NameValueLookup.ofList [
199+
"container", upcast NameValueLookup.ofList [
200+
"title", upcast "Meeting"
201+
"tags", null
202+
"count", upcast 42
203+
]
204+
]
205+
let result = sync <| Executor(schema).AsyncExecute(parse "{ container { title tags count } }", getMockInputContext, ())
206+
ensureDirect result <| fun data errors ->
207+
data |> equals (upcast expectedData)
208+
equals 1 errors.Length
209+
hasError "Boom during enumeration" errors
210+
211+
[<Fact>]
212+
let ``Execution must return sibling fields on nested struct nullable list that throws during lazy enumeration`` () =
213+
let TestContainerStructType =
214+
Define.Object<TestContainer>(
215+
"TestContainerStruct", [
216+
Define.Field("title", StringType, fun _ x -> x.Title)
217+
Define.Field(
218+
"tags",
219+
StructNullable (ListOf StringType),
220+
fun _ x -> x.Tags |> ValueSome
221+
)
222+
Define.Field("count", IntType, fun _ x -> x.Count)
223+
])
224+
let container = {
225+
Title = "Meeting"
226+
Tags = seq {
227+
yield "first"
228+
failwith "Boom during enumeration"
229+
}
230+
Count = 42
231+
}
232+
let schema =
233+
Schema(
234+
Define.Object<unit>(
235+
"Query", [
236+
Define.Field("container", TestContainerStructType, fun _ _ -> container)
237+
]))
238+
let expectedData =
239+
NameValueLookup.ofList [
240+
"container", upcast NameValueLookup.ofList [
241+
"title", upcast "Meeting"
242+
"tags", null
243+
"count", upcast 42
244+
]
245+
]
246+
let result = sync <| Executor(schema).AsyncExecute(parse "{ container { title tags count } }", getMockInputContext, ())
247+
ensureDirect result <| fun data errors ->
248+
data |> equals (upcast expectedData)
249+
equals 1 errors.Length
250+
hasError "Boom during enumeration" errors
251+
252+
[<Fact>]
253+
let ``Execution must return sibling objects when one nested nullable list throws during lazy enumeration`` () =
254+
let goodContainer = {
255+
Title = "Good"
256+
Tags = seq { yield "alpha"; yield "beta" }
257+
Count = 1
258+
}
259+
let badContainer = {
260+
Title = "Bad"
261+
Tags = seq {
262+
yield "first"
263+
failwith "Boom during enumeration"
264+
}
265+
Count = 2
266+
}
267+
let schema =
268+
Schema(
269+
Define.Object<unit>(
270+
"Query", [
271+
Define.Field("good", TestContainerType, fun _ _ -> goodContainer)
272+
Define.Field("bad", TestContainerType, fun _ _ -> badContainer)
273+
]))
274+
let expectedData =
275+
NameValueLookup.ofList [
276+
"good", upcast NameValueLookup.ofList [
277+
"title", upcast "Good"
278+
"tags", upcast [| box "alpha"; box "beta" |]
279+
"count", upcast 1
280+
]
281+
"bad", upcast NameValueLookup.ofList [
282+
"title", upcast "Bad"
283+
"tags", null
284+
"count", upcast 2
285+
]
286+
]
287+
let result = sync <| Executor(schema).AsyncExecute(parse "{ good { title tags count } bad { title tags count } }", getMockInputContext, ())
288+
ensureDirect result <| fun data errors ->
289+
data |> equals (upcast expectedData)
290+
equals 1 errors.Length
291+
hasError "Boom during enumeration" errors

0 commit comments

Comments
 (0)