From b56c3ee14c6826c3ba6a60abec1a306ddbf68179 Mon Sep 17 00:00:00 2001 From: Hugo van Rijswijk Date: Mon, 31 Aug 2026 12:42:19 +0200 Subject: [PATCH] Handle partial data and error paths This is a complicated change, and open to discussion if it is actually wanted. Failed fields now propagate their `null` to the nearest nullable position, rather than a `Result.Failure` which discards all data. Example: Before: ```graphql query { ping viaEffect { name } } { "errors": [ { "message": "boom" } ] } ``` After: ```graphql query { ping viaEffect { name } } { "data": { "ping": "pong", "viaEffect": null }, "errors": [ { "message": "boom", "path": ["viaEffect"] } ] } ``` This is a big breaking change for users that expect data to be complete if there is `data` at the root. But it is in line with the GraphQL specification, which says that a failed field should not discard its siblings' data. This partially closes a few conformance suites, except for `location`s in errors, which are not (yet) implemented. --- The interpreter now tracks the response position of each value: - A failure at a nullable position completes as null and keeps the data of its siblings. - A failure at a non-null position propagates its null to the nearest enclosing nullable position. - A null which reaches the root leaves `data` as null. Each problem carries the response path of its own position, with the alias of the field and the index of the list entry. Both error policies now complete the deferred positions of a failed batch as null and keep the rest of the response. An internal error stays a request error and aborts the completion. BREAKING: `Problem.path` changes type from `List[String]` to `List[Problem.PathSegment]`, so that a path can hold list indexes. A segment is a `Name(String)` or an `Index(Int)`. --- .../scala/CirceEffectHandlerErrorData.scala | 125 +++++ .../scala/CirceEffectHandlerErrorSuite.scala | 91 +++- modules/core/src/main/scala/problem.scala | 37 +- .../src/main/scala/queryinterpreter.scala | 434 +++++++++++++----- modules/core/src/main/scala/result.scala | 12 + .../scala/compiler/EnvironmentSuite.scala | 3 +- .../test/scala/compiler/ProblemSuite.scala | 32 +- .../test/scala/errors/FieldErrorData.scala | 263 +++++++++++ .../test/scala/errors/FieldErrorSuite.scala | 212 +++++++++ 9 files changed, 1092 insertions(+), 117 deletions(-) create mode 100644 modules/core/src/test/scala/errors/FieldErrorData.scala create mode 100644 modules/core/src/test/scala/errors/FieldErrorSuite.scala diff --git a/modules/circe/src/test/scala/CirceEffectHandlerErrorData.scala b/modules/circe/src/test/scala/CirceEffectHandlerErrorData.scala index c7b3c28f..b12e69b2 100644 --- a/modules/circe/src/test/scala/CirceEffectHandlerErrorData.scala +++ b/modules/circe/src/test/scala/CirceEffectHandlerErrorData.scala @@ -24,6 +24,18 @@ import grackle.QueryInterpreter.EffectErrorPolicy import grackle.circe.CirceMapping import grackle.syntax._ +object FailingEffectHandler { + + /** + * An effect handler whose batch always fails. + */ + def apply[F[_]: Sync]: EffectHandler[F] = + new EffectHandler[F] { + def runEffects(queries: List[(Query, Cursor)]): F[Result[List[Cursor]]] = + Result.failure[List[Cursor]]("boom").pure[F] + } +} + class TestCirceEffectHandlerErrorMapping[F[_]: Sync]( ref: SignallingRef[F, Int], policy: EffectErrorPolicy) @@ -69,6 +81,119 @@ class TestCirceEffectHandlerErrorMapping[F[_]: Sync]( } +/** + * A failing effect handler beside a pure sibling field, both nullable. A failed batch is a + * field error, not a request error: the response keeps its `data` entry, with the effect field + * null and the sibling value intact. + */ +class TestCirceEffectHandlerSiblingMapping[F[_]: Sync] extends CirceMapping[F] { + val schema = + schema""" + type Query { + ping: String + viaEffect: String + } + """ + + val QueryType = schema.ref("Query") + + val typeMappings = List( + ObjectMapping( + tpe = QueryType, + fieldMappings = List( + CursorFieldJson("ping", _ => Result.success(Json.fromString("pong")), Nil), + EffectField("viaEffect", FailingEffectHandler[F], Nil) + ) + ) + ) +} + +/** + * A succeeding effect handler whose continuation fails at a non-null field. + * + * The batch succeeds, so the failure belongs to one position. `viaEffect` is non-null, so the + * null bubbles up to the `data` entry. + */ +class TestCirceFailingContinuationMapping[F[_]: Sync] extends CirceMapping[F] { + val schema = + schema""" + type Query { + ping: String + viaEffect: Child! + } + type Child { + name: String! + } + """ + + val QueryType = schema.ref("Query") + val ChildType = schema.ref("Child") + + val handler: EffectHandler[F] = + new EffectHandler[F] { + def runEffects(queries: List[(Query, Cursor)]): F[Result[List[Cursor]]] = + queries + .traverse { + case (query, parentCursor) => + Query + .childContext(parentCursor.context, query) + .map(ctx => CirceCursor(ctx, Json.obj(), Some(parentCursor), Env.empty): Cursor) + } + .pure[F] + } + + val typeMappings = List( + ObjectMapping( + tpe = QueryType, + fieldMappings = List( + CursorFieldJson("ping", _ => Result.success(Json.fromString("pong")), Nil), + EffectField("viaEffect", handler, Nil) + ) + ), + ObjectMapping( + tpe = ChildType, + fieldMappings = List( + CursorFieldJson("name", _ => Result.failure("boom"), Nil) + ) + ) + ) +} + +/** + * A failing effect handler at a non-null field of a nullable object. + * + * The failed field is non-null, so the null bubbles up to the `child` position. + */ +class TestCirceNestedNonNullEffectMapping[F[_]: Sync] extends CirceMapping[F] { + val schema = + schema""" + type Query { + ping: String + child: Child + } + type Child { + viaEffect: String! + } + """ + + val QueryType = schema.ref("Query") + val ChildType = schema.ref("Child") + + val typeMappings = List( + ObjectMapping( + tpe = QueryType, + fieldMappings = List( + CursorFieldJson("ping", _ => Result.success(Json.fromString("pong")), Nil), + CursorFieldJson("child", _ => Result.success(Json.obj()), Nil) + ) + ), + ObjectMapping( + tpe = ChildType, + fieldMappings = List(EffectField("viaEffect", FailingEffectHandler[F], Nil)) + ) + ) +} + /** * As `TestCirceEffectHandlerErrorMapping`, but both fields are backed by a *single, shared* * handler. Because effects are batched by `(mapping, handler)`, this means both fields end up diff --git a/modules/circe/src/test/scala/CirceEffectHandlerErrorSuite.scala b/modules/circe/src/test/scala/CirceEffectHandlerErrorSuite.scala index 47c68f8b..57374e6b 100644 --- a/modules/circe/src/test/scala/CirceEffectHandlerErrorSuite.scala +++ b/modules/circe/src/test/scala/CirceEffectHandlerErrorSuite.scala @@ -48,9 +48,10 @@ final class CirceEffectHandlerErrorSuite extends CatsEffectSuite { val expected = json""" { "errors" : [ - { "message": "value: hi" }, - { "message": "value: 42" } - ] + { "message": "value: hi", "path": ["s"] }, + { "message": "value: 42", "path": ["n"] } + ], + "data" : null } """ @@ -61,8 +62,9 @@ final class CirceEffectHandlerErrorSuite extends CatsEffectSuite { val expected = json""" { "errors" : [ - { "message": "value: hi" } - ] + { "message": "value: hi", "path": ["s"] } + ], + "data" : null } """ @@ -82,7 +84,8 @@ final class CirceEffectHandlerErrorSuite extends CatsEffectSuite { "errors" : [ { "message": "value: s" }, { "message": "value: n" } - ] + ], + "data" : null } """ @@ -97,6 +100,30 @@ final class CirceEffectHandlerErrorSuite extends CatsEffectSuite { assertIO(prg, (expected, 2)) } + test("circe effect handler failure is a field error, sibling data is retained") { + val query = """ + query { + ping, + viaEffect + } + """ + + val expected = json""" + { + "errors" : [ + { "message": "boom", "path": ["viaEffect"] } + ], + "data" : { + "ping" : "pong", + "viaEffect" : null + } + } + """ + + val map = new TestCirceEffectHandlerSiblingMapping[IO] + assertIO(map.compileAndRun(query), expected) + } + test("circe nested effect handler errors are accumulated in document order") { val query = """ query { @@ -118,7 +145,8 @@ final class CirceEffectHandlerErrorSuite extends CatsEffectSuite { { "message": "nested: a/y" }, { "message": "nested: b/x" }, { "message": "nested: b/y" } - ] + ], + "data" : null } """ @@ -126,4 +154,53 @@ final class CirceEffectHandlerErrorSuite extends CatsEffectSuite { assertIO(map.compileAndRun(query), expected) } + test("a failed continuation of a succeeding effect handler nulls its own position") { + val query = """ + query { + ping + viaEffect { + name + } + } + """ + + val expected = json""" + { + "errors" : [ + { "message": "boom", "path": ["viaEffect", "name"] } + ], + "data" : null + } + """ + + val map = new TestCirceFailingContinuationMapping[IO] + assertIO(map.compileAndRun(query), expected) + } + + test("a null from a nested effect handler stops at the nearest nullable position") { + val query = """ + query { + ping + child { + viaEffect + } + } + """ + + val expected = json""" + { + "errors" : [ + { "message": "boom", "path": ["child", "viaEffect"] } + ], + "data" : { + "ping" : "pong", + "child" : null + } + } + """ + + val map = new TestCirceNestedNonNullEffectMapping[IO] + assertIO(map.compileAndRun(query), expected) + } + } diff --git a/modules/core/src/main/scala/problem.scala b/modules/core/src/main/scala/problem.scala index 142e7770..d2450979 100644 --- a/modules/core/src/main/scala/problem.scala +++ b/modules/core/src/main/scala/problem.scala @@ -25,9 +25,17 @@ import io.circe.syntax._ final case class Problem( message: String, locations: List[(Int, Int)] = Nil, - path: List[String] = Nil, + path: List[Problem.PathSegment] = Nil, extensions: Option[JsonObject] = None ) { + + /** + * Yields this problem with `path` as its response path, if it has none. A path set deeper in + * the response is more precise, so it wins. + */ + def atPath(path: List[Problem.PathSegment]): Problem = + if (this.path.isEmpty) copy(path = path) else this + override def toString = { lazy val pathText: String = @@ -56,6 +64,33 @@ final case class Problem( object Problem { + /** + * A segment of a response path: a field name, or an index into a list. + * + * @see + * https://spec.graphql.org/September2025/#sec-Response-Position + */ + sealed trait PathSegment + + object PathSegment { + + final case class Name(name: String) extends PathSegment { + override def toString: String = name + } + + final case class Index(index: Int) extends PathSegment { + assert(index >= 0, s"Index must be non-negative: $index") + override def toString: String = index.toString + } + + implicit val PathSegmentEncoder: Encoder[PathSegment] = { + case Name(name) => name.asJson + case Index(index) => index.asJson + } + + implicit val eqPathSegment: Eq[PathSegment] = Eq.fromUniversalEquals + } + implicit val ProblemEncoder: Encoder[Problem] = { p => val locationsField: List[(String, Json)] = if (p.locations.isEmpty) Nil diff --git a/modules/core/src/main/scala/queryinterpreter.scala b/modules/core/src/main/scala/queryinterpreter.scala index 1faedeaf..510d457e 100644 --- a/modules/core/src/main/scala/queryinterpreter.scala +++ b/modules/core/src/main/scala/queryinterpreter.scala @@ -27,7 +27,7 @@ import io.circe.Json import grackle.Cursor.ListTransformCursor import grackle.Query._ -import grackle.QueryInterpreter.ProtoJson +import grackle.QueryInterpreter.{ProtoJson, ResponsePosition} import grackle.QueryInterpreter.ProtoJson._ import grackle.syntax._ @@ -49,10 +49,14 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) { else Stream.eval(runOneShot(query, rootTpe, rootCursor)) + // The `data` entry is nullable, so a null which reaches the root lands there. (for { pvalue <- ResultT(mergedResults) value <- ResultT(Stream.eval(QueryInterpreter.complete[F](pvalue))) - } yield value).value + } yield value).value.map { + case Result.Failure(ps) => Result.Warning(ps, Json.Null) + case other => other + } } /** @@ -154,14 +158,11 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) { for { pr <- pureResults er <- effectfulResults - } yield ((pr ++ er) match { + } yield (pr ++ er) match { case Nil => Result(ProtoJson.fromJson(Json.Null)) case List(r) => r case hd :: tl => tl.foldLeft(hd) { case (acc, elem) => acc |+| elem } - }) match { - case Result.Failure(errs) => Result.Warning(errs, ProtoJson.fromJson(Json.Null)) - case other => other } } } @@ -191,25 +192,52 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) { (strip(tpe) nominal_=:= strip(cursorTpe)) } + /** + * Marks `value` when it stands at a non-null response position of type `tpe`. + */ + private def atPosition(value: ProtoJson, tpe: Type): ProtoJson = + if (tpe.isNullable) value else ProtoJson.nonNull(value) + + /** + * Handles a failure of the field `name` at the position `pos` as a field error: the problems + * carry the path of `pos`, and a nullable field completes as null. + * + * @see + * https://spec.graphql.org/September2025/#sec-Handling-Field-Errors + */ + private def fieldError(tpe: Type, pos: ResponsePosition, name: String)( + res: Result[List[(String, ProtoJson)]]): Result[List[(String, ProtoJson)]] = + res.atPath(pos.path) match { + case Result.Failure(ps) if tpe.isNullable => + Result.Warning(ps, List((name, ProtoJson.fromJson(Json.Null)))) + case other => other + } + /** * Interpret `query` against `cursor`, yielding a collection of fields. * * If the query is valid, the field subqueries will all be valid fields of the enclosing type * `tpe` and the resulting fields may be used to build a Json object of type `tpe`. If the * query is invalid errors will be returned on the left hand side of the result. + * + * `path` is the response position of the enclosing object. */ - def runFields(query: Query, tpe: Type, cursor: Cursor): Result[List[(String, ProtoJson)]] = + def runFields( + query: Query, + tpe: Type, + cursor: Cursor, + path: ResponsePosition = ResponsePosition.root): Result[List[(String, ProtoJson)]] = if (!cursorCompatible(tpe, cursor.tpe)) Result.internalError(s"Mismatched query and cursor type in runFields: $tpe ${cursor.tpe}") else { query match { case g: Group if groupWithTypeCase(g) => ungroup(g) - .flatTraverse(query => runFields(query, tpe, cursor)) + .flatTraverse(query => runFields(query, tpe, cursor, path)) .map(fs => mergeFields(fs).toList) case Group(siblings) => - siblings.flatTraverse(query => runFields(query, tpe, cursor)) + siblings.flatTraverse(query => runFields(query, tpe, cursor, path)) case Introspect(schema, s @ Select("__typename", _, Empty)) if tpe.isNamed => val fail = @@ -245,36 +273,48 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) { .map { rc => for { c <- rc - fields <- runFields(sel, tpe, c) + fields <- runFields(sel, tpe, c, path) } yield fields } .getOrElse(List((sel.resultName, ProtoJson.fromJson(Json.Null))).success) - case sel @ Select(_, _, Count(Select(countName, _, _))) => + case sel @ Select(fieldName, _, Count(Select(countName, _, _))) => def size(c: Cursor): Result[Int] = if (c.isList) c.asList(Iterator).map(_.size) else 1.success - for { - c0 <- cursor.field(countName, None) - count <- - if (c0.isNullable) c0.asNullable.flatMap(_.map(size).getOrElse(0.success)) - else size(c0) - } yield List((sel.resultName, ProtoJson.fromJson(Json.fromInt(count)))) + val fieldTpe = tpe.field(fieldName).getOrElse(ScalarType.AttributeType) + val fieldPos = path.field(sel.resultName) + fieldError(fieldTpe, fieldPos, sel.resultName) { + for { + c0 <- cursor.field(countName, None) + count <- + if (c0.isNullable) c0.asNullable.flatMap(_.map(size).getOrElse(0.success)) + else size(c0) + } yield List((sel.resultName, ProtoJson.fromJson(Json.fromInt(count)))) + } - case sel @ Select(_, _, Effect(handler, cont)) => - for { - value <- ProtoJson - .effect(mapping, handler.asInstanceOf[EffectHandler[F]], cont, cursor) - .success - } yield List((sel.resultName, value)) + case sel @ Select(fieldName, _, Effect(handler, cont)) => + val fieldTpe = tpe.field(fieldName).getOrElse(ScalarType.AttributeType) + val fieldPos = path.field(sel.resultName) + val value = + ProtoJson.effect( + mapping, + handler.asInstanceOf[EffectHandler[F]], + cont, + cursor, + fieldPos) + List((sel.resultName, atPosition(value, fieldTpe))).success case sel @ Select(fieldName, resultName, child) => val fieldTpe = tpe.field(fieldName).getOrElse(ScalarType.AttributeType) - for { - c <- cursor.field(fieldName, resultName) - value <- runValue(child, fieldTpe, c) - } yield List((sel.resultName, value)) + val fieldPos = path.field(sel.resultName) + fieldError(fieldTpe, fieldPos, sel.resultName) { + for { + c <- cursor.field(fieldName, resultName) + value <- runValue(child, fieldTpe, c, fieldPos) + } yield List((sel.resultName, atPosition(value, fieldTpe))) + } case Narrow(tp1, child) => cursor.narrowsTo(tp1).flatMap { n => @@ -282,24 +322,29 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) { else for { c <- cursor.narrow(tp1) - fields <- runFields(child, tp1, c) + fields <- runFields(child, tp1, c, path) } yield fields } case c @ Component(_, _, cont) => - for { - componentName <- resultName(cont).toResultOrError( - "Join continuation has unexpected shape") - value <- runValue(c, tpe, cursor) - } yield List((componentName, ProtoJson.select(value, componentName))) + rootName(cont).toResultOrError("Join continuation has unexpected shape").flatMap { + case (fieldName, alias) => + val componentName = alias.getOrElse(fieldName) + val fieldTpe = tpe.field(fieldName).getOrElse(ScalarType.AttributeType) + val fieldPos = path.field(componentName) + runValue(c, tpe, cursor, fieldPos).map { value => + List( + (componentName, atPosition(ProtoJson.select(value, componentName), fieldTpe))) + } + } case Environment(childEnv: Env, child: Query) => - runFields(child, tpe, cursor.withEnv(childEnv)) + runFields(child, tpe, cursor.withEnv(childEnv), path) case TransformCursor(f, child) => for { ct <- f(cursor) - fields <- runFields(child, tpe, ct) + fields <- runFields(child, tpe, ct, path) } yield fields case _ => @@ -312,7 +357,8 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) { tpe: Type, parent: Cursor, unique: Boolean, - nullable: Boolean): Result[ProtoJson] = { + nullable: Boolean, + path: ResponsePosition = ResponsePosition.root): Result[ProtoJson] = { val (query0, f) = query match { case TransformCursor(f, child) => (child, Some(f)) @@ -363,19 +409,34 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) { def mkResult(child: Query, ic: Iterator[Cursor]): Result[ProtoJson] = { val builder = Vector.newBuilder[ProtoJson] var problems = Chain.empty[Problem] + var index = 0 builder.sizeHint(ic.knownSize) + + // A unique list yields one value, which stands at the position of the list itself. + def elemPosition(i: Int): ResponsePosition = if (unique) path else path.index(i) + def markElem(v: ProtoJson): ProtoJson = if (unique) v else atPosition(v, tpe) + while (ic.hasNext) { val c = ic.next() if (!cursorCompatible(tpe, c.tpe)) return Result.internalError( s"Mismatched query and cursor type in runList: $tpe ${c.tpe}") - runValue(child, tpe, c) match { - case err @ Result.InternalError(_) => return err - case fail @ Result.Failure(_) => return fail - case Result.Success(v) => builder.addOne(v) + val elemPos = elemPosition(index) + index += 1 + + runValue(child, tpe, c, elemPos) match { + case err: Result.InternalError => return err + // A nullable element completes as null, so the other elements survive. + case Result.Failure(ps) if !unique && tpe.isNullable => + val elemPath = elemPos.path + builder.addOne(ProtoJson.fromJson(Json.Null)) + problems = problems.concat(ps.map(_.atPath(elemPath)).toChain) + case fail: Result.Failure => return fail + case Result.Success(v) => + builder.addOne(markElem(v)) case Result.Warning(ps, v) => - builder.addOne(v) + builder.addOne(markElem(v)) problems = problems.concat(ps.toChain) } } @@ -409,23 +470,32 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) { * Interpret `query` against `cursor` with expected type `tpe`. * * If the query is invalid errors will be returned on the left hand side of the result. + * + * `path` is the response position of the value. */ - def runValue(query: Query, tpe: Type, cursor: Cursor): Result[ProtoJson] = { + def runValue( + query: Query, + tpe: Type, + cursor: Cursor, + path: ResponsePosition = ResponsePosition.root): Result[ProtoJson] = { if (!cursorCompatible(tpe, cursor.tpe)) Result.internalError(s"Mismatched query and cursor type in runValue: $tpe ${cursor.tpe}") else { (query, tpe.dealias) match { case (Environment(childEnv: Env, child: Query), tpe) => - runValue(child, tpe, cursor.withEnv(childEnv)) + runValue(child, tpe, cursor.withEnv(childEnv), path) case (Component(_, _, _), ListType(tpe)) => cursor.asList(Iterator) match { case Result.Success(ic) => val builder = Vector.newBuilder[ProtoJson] + var index = 0 builder.sizeHint(ic.knownSize) while (ic.hasNext) { val c = ic.next() - runValue(query, tpe, c) match { + val elemPos = path.index(index) + index += 1 + runValue(query, tpe, c, elemPos) match { case Result.Success(v) => builder.addOne(v) case notRight => return notRight } @@ -442,13 +512,13 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) { for { childName <- resultName(child).toResultOrError( "Join child has unexpected shape") - elems <- conts.traverse { - case cont => + elems <- conts.zipWithIndex.traverse { + case (cont, index) => for { componentName <- resultName(cont).toResultOrError( "Join continuation has unexpected shape") } yield ProtoJson.select( - ProtoJson.component(mapping, cont, cursor), + ProtoJson.component(mapping, cont, cursor, path.index(index)), componentName ) } @@ -460,19 +530,21 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) { for { renamedCont <- alignResultName(child, cont).toResultOrError( "Join continuation has unexpected shape") - } yield ProtoJson.component(mapping, renamedCont, cursor) + } yield ProtoJson.component(mapping, renamedCont, cursor, path) } case (Unique(child), _) => - cursor.preunique.flatMap(c => runList(child, tpe.nonNull, c, true, tpe.isNullable)) + cursor + .preunique + .flatMap(c => runList(child, tpe.nonNull, c, true, tpe.isNullable, path)) case (_, ListType(tpe)) => - runList(query, tpe, cursor, false, false) + runList(query, tpe, cursor, false, false, path) case (TransformCursor(f, child), _) => for { ct <- f(cursor) - value <- runValue(child, tpe, ct) + value <- runValue(child, tpe, ct, path) } yield value case (_, NullableType(tpe)) => @@ -482,7 +554,7 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) { .map { rc => for { c <- rc - value <- runValue(query, tpe, c) + value <- runValue(query, tpe, c, path) } yield value } .getOrElse(ProtoJson.fromJson(Json.Null).success) @@ -491,7 +563,7 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) { cursor.asLeaf.map(ProtoJson.fromJson) case (_, _: ObjectType | _: InterfaceType | _: UnionType) => - runFields(query, tpe, cursor).map(ProtoJson.fromDisjointFields) + runFields(query, tpe, cursor, path).map(ProtoJson.fromDisjointFields) case _ => Result.internalError(s"Stuck at type $tpe for ${query.render}") @@ -502,6 +574,42 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) { object QueryInterpreter { + /** + * The position of a value in the response. + * + * `segments` holds the path from the root of the response, in reverse order. + * + * @see + * https://spec.graphql.org/September2025/#sec-Response-Position + */ + final case class ResponsePosition(segments: List[Problem.PathSegment]) { + + /** + * The position of the field `name` inside this position. + */ + def field(name: String): ResponsePosition = + ResponsePosition(Problem.PathSegment.Name(name) :: segments) + + /** + * The position of the list entry `index` inside this position. + */ + def index(index: Int): ResponsePosition = + ResponsePosition(Problem.PathSegment.Index(index) :: segments) + + /** + * The response path of this position, from the root. + */ + def path: List[Problem.PathSegment] = segments.reverse + } + + object ResponsePosition { + + /** + * The position of the `data` entry. + */ + val root: ResponsePosition = ResponsePosition(Nil) + } + /** * Policy determining how errors arising from batches of deferred effects (effect handlers and * delegated components) are combined during result completion. @@ -509,6 +617,9 @@ object QueryInterpreter { * When batches from multiple mappings are completed together, accumulation only applies if * every contributing mapping opts in: any `FailFast` mapping makes the whole completion fail * fast, since fail fast is a promise not to run further effects after a failure. + * + * Either way a failed batch is a field error: it completes as null and the response keeps its + * `data` entry. Neither policy applies to internal errors, which abort the completion. */ sealed trait EffectErrorPolicy @@ -516,11 +627,15 @@ object QueryInterpreter { /** * Stop at the first failed effect batch; subsequent batches' effects are not run. + * + * The failed batch and the batches which do not run all complete as null. */ case object FailFast extends EffectErrorPolicy /** * Run every effect batch and accumulate errors from all of them, in document order. + * + * Each failed batch completes as null. */ case object Accumulate extends EffectErrorPolicy @@ -546,7 +661,8 @@ object QueryInterpreter { mapping: Mapping[F], handler: Option[EffectHandler[F]], query: Query, - cursor: Cursor) + cursor: Cursor, + position: ResponsePosition) extends DeferredJson // A partially constructed object which has at least one deferred subtree. private[QueryInterpreter] case class ProtoObject(fields: Seq[(String, ProtoJson)]) @@ -554,6 +670,11 @@ object QueryInterpreter { private[QueryInterpreter] case class ProtoArray(elems: Seq[ProtoJson]) // A result which will yield a selection from its child private[QueryInterpreter] case class ProtoSelect(elem: ProtoJson, fieldName: String) + // A subtree at a non-null response position. A null from below propagates past it, to the + // nearest enclosing nullable position. + private[QueryInterpreter] case class ProtoNonNull(elem: ProtoJson) + // A null which propagates from the position at which it stands. + private[QueryInterpreter] case class ProtoNull() implicit val monoidInstance: Monoid[ProtoJson] = new Monoid[ProtoJson] { @@ -566,18 +687,37 @@ object QueryInterpreter { * Delegate `query` to the interpreter `interpreter`. When evaluated by that interpreter the * query will have expected type `rootTpe`. */ - def component[F[_]](mapping: Mapping[F], query: Query, cursor: Cursor): ProtoJson = - wrap(EffectJson(mapping, None, query, cursor)) + def component[F[_]]( + mapping: Mapping[F], + query: Query, + cursor: Cursor, + position: ResponsePosition = ResponsePosition.root): ProtoJson = + wrap(EffectJson(mapping, None, query, cursor, position)) def effect[F[_]]( mapping: Mapping[F], handler: EffectHandler[F], query: Query, - cursor: Cursor): ProtoJson = - wrap(EffectJson(mapping, Some(handler), query, cursor)) + cursor: Cursor, + position: ResponsePosition = ResponsePosition.root): ProtoJson = + wrap(EffectJson(mapping, Some(handler), query, cursor, position)) def fromJson(value: Json): ProtoJson = wrap(value) + /** + * Marks `pj` as a value at a non-null response position. A complete Json value holds no + * null which propagates, so it needs no mark. + */ + def nonNull(pj: ProtoJson): ProtoJson = + if (pj.isInstanceOf[Json]) pj else wrap(ProtoNonNull(pj)) + + /** + * A null which propagates to the nearest enclosing nullable position. + * + * A failed value completes as such a null. A Json null stops at its own position instead. + */ + val propagatingNull: ProtoJson = wrap(ProtoNull()) + /** * Combine possibly partial fields to create a possibly partial object. * @@ -629,23 +769,39 @@ object QueryInterpreter { * Yields `true` if the argument contains any component or staged subtrees, false otherwise. */ def isDeferred(p: ProtoJson): Boolean = - p.isInstanceOf[DeferredJson] + p match { + case _: DeferredJson => true + case ProtoNonNull(elem) => isDeferred(elem) + case _ => false + } /** * Recursively merge a list of ProtoJson values. */ def mergeProtoJson(elems: Seq[ProtoJson]): ProtoJson = { - elems match { - case Seq(elem) => elem - case Seq(_: ProtoObject, _*) => mergeProtoObjects(elems) - case Seq(j: Json, _*) if j.isObject => mergeProtoObjects(elems) - case Seq(_: ProtoArray, _*) => mergeProtoArrays(elems) - case Seq(j: Json, _*) if j.isArray => mergeProtoArrays(elems) - case Seq(hd, _*) => hd - case _ => wrap(Json.Null) - } + // The merge matches on the shape of a value, so the non-null marks come off first and go + // back on the merged value. + val marked = elems.exists(_.isInstanceOf[ProtoNonNull]) + val stripped = if (marked) elems.map(stripNonNull) else elems + val merged = + stripped match { + case Seq(elem) => elem + case Seq(_: ProtoObject, _*) => mergeProtoObjects(stripped) + case Seq(j: Json, _*) if j.isObject => mergeProtoObjects(stripped) + case Seq(_: ProtoArray, _*) => mergeProtoArrays(stripped) + case Seq(j: Json, _*) if j.isArray => mergeProtoArrays(stripped) + case Seq(hd, _*) => hd + case _ => wrap(Json.Null) + } + if (marked) nonNull(merged) else merged } + private def stripNonNull(pj: ProtoJson): ProtoJson = + pj match { + case ProtoNonNull(elem) => elem + case other => other + } + /** * Recursively merge a list of ProtoJson objects. */ @@ -717,9 +873,10 @@ object QueryInterpreter { mergeProtoJson(objs.asInstanceOf[List[ProtoJson]]).asInstanceOf[Json] // Combine a list of ProtoJson results, collecting all errors on the left and preserving - // the order and number of elements by inserting Json Nulls for Lefts. + // the order and number of elements by inserting nulls for the failures. A failed element + // has no value, so its null propagates to the nearest enclosing nullable position. def combineResults(ress: List[Result[ProtoJson]]): Result[List[ProtoJson]] = - Result.combineAllWithDefault(ress, ProtoJson.fromJson(Json.Null)) + Result.combineAllWithDefault(ress, propagatingNull) private def wrap(j: AnyRef): ProtoJson = j.asInstanceOf[ProtoJson] } @@ -751,9 +908,26 @@ object QueryInterpreter { * Complete results are substituted back into the corresponding enclosing Json. * * Errors are aggregated across all the results and are accumulated on the `Left` of the - * result. + * result: + * - A failed effect batch is a field error. Its problems become warnings and its positions + * complete as null, so sibling data survives and the response keeps its `data` entry. + * - A null at a non-null position propagates to the nearest enclosing nullable position. + * - An internal error aborts the completion. + */ + def completeAll[F[_]: Monad](pjs: List[ProtoJson]): F[Result[List[Json]]] = + completeAllOrNull[F](pjs).map(_.map(_.map(_.getOrElse(Json.Null)))) + + // A null which stops at a nullable position. + private val SomeNull: Option[Json] = Some(Json.Null) + + /** + * Complete a collection of possibly deferred results, as `completeAll`. + * + * A result is `None` when a null propagates past the root of that result. The `data` entry is + * nullable, so `completeAll` turns such a result into a Json null. */ - def completeAll[F[_]: Monad](pjs: List[ProtoJson]): F[Result[List[Json]]] = { + private def completeAllOrNull[F[_]: Monad]( + pjs: List[ProtoJson]): F[Result[List[Option[Json]]]] = { // Yields deferred fields in document order. def gatherDeferred(pj: ProtoJson): List[DeferredJson] = { @tailrec @@ -763,10 +937,12 @@ object QueryInterpreter { case Some((hd, tl)) => (hd: @unchecked) match { case _: Json => loop(tl, acc) + case _: ProtoNull => loop(tl, acc) case d: DeferredJson => loop(tl, acc :+ d) case ProtoObject(fields) => loop(Chain.fromSeq(fields.map(_._2)) ++ tl, acc) case ProtoArray(elems) => loop(Chain.fromSeq(elems) ++ tl, acc) case ProtoSelect(elem, _) => loop(elem +: tl, acc) + case ProtoNonNull(elem) => loop(elem +: tl, acc) } } @@ -776,21 +952,31 @@ object QueryInterpreter { } } - def scatterResults(pj: ProtoJson, subst: mutable.Map[DeferredJson, Json]): Json = { - def loop(pj: ProtoJson): Json = + def scatterResults( + pj: ProtoJson, + subst: mutable.Map[DeferredJson, Option[Json]]): Option[Json] = { + // Yields None when a null propagates past `pj`: a position below it completed as null, + // and no position in between is nullable. + def loop(pj: ProtoJson): Option[Json] = (pj: @unchecked) match { - case p: Json => p + case p: Json => Some(p) + case _: ProtoNull => None case d: DeferredJson => subst(d) + case ProtoNonNull(elem) => loop(elem) case ProtoObject(fields) => - val fields0 = fields.map { case (label, pvalue) => (label, loop(pvalue)) } - Json.fromFields(fields0) + fields + .traverse { case (label, pvalue) => position(pvalue).tupleLeft(label) } + .map(Json.fromFields) case ProtoArray(elems) => - val elems0 = elems.map(loop) - Json.fromValues(elems0) + elems.traverse(position).map(Json.fromValues) case ProtoSelect(elem, fieldName) => - loop(elem).asObject.flatMap(_(fieldName)).getOrElse(Json.Null) + loop(elem).map(_.asObject.flatMap(_(fieldName)).getOrElse(Json.Null)) } + // A position without a non-null mark stops the propagation with a null. + def position(pj: ProtoJson): Option[Json] = + if (pj.isInstanceOf[ProtoNonNull]) loop(pj) else loop(pj).orElse(SomeNull) + loop(pj) } @@ -806,7 +992,7 @@ object QueryInterpreter { def runBatch( mapping: Mapping[F], handler: Option[EffectHandler[F]], - batch: List[EffectJson[F]]): F[Result[List[(EffectJson[F], Json)]]] = { + batch: List[EffectJson[F]]): F[Result[List[(EffectJson[F], Option[Json])]]] = { val queries = batch.map(e => (e.query, e.cursor)) (for { pnext <- @@ -825,44 +1011,84 @@ object QueryInterpreter { .toResultOrError("Continuation query has the wrong shape") } .pure[F]) - res <- ResultT(combineResults((conts, cs).parMapN { - case (query, cursor) => - mapping.interpreter.runValue(query, cursor.tpe, cursor) - }).pure[F]) + res <- ResultT.fromResult[F, List[ProtoJson]]( + combineResults((batch, conts, cs).parMapN { (e, query, cursor) => + mapping + .interpreter + .runValue(query, cursor.tpe, cursor, e.position) + .atPath(e.position.path) + })) } yield res } - next <- ResultT(completeAll[F](pnext)) + next <- ResultT(completeAllOrNull[F](pnext)) } yield batch.zip(next)).value } val policy = deferred.map(_.mapping).distinct.foldMap(_.effectErrorPolicy) - val batchedResults = + type Batch = ((Mapping[F], Option[EffectHandler[F]]), List[EffectJson[F]]) + type Completed = List[(EffectJson[F], Option[Json])] + + // Completes every deferred position of a batch as null. Failed and unrun batches are nulled + // rather than dropped, so that the substitution in `scatterResults` stays total. + def nullBatch(batch: List[EffectJson[F]]): Completed = + batch.tupleRight(None) + + // Handles a failed batch as a field error: its problems become warnings and its positions + // complete as null. A batch which covers exactly one position carries the path of that position. + def batchFieldError( + batch: List[EffectJson[F]], + ps: NonEmptyChain[Problem]): Result[Completed] = { + val ps0 = + batch match { + case List(e) => + val path = e.position.path + ps.map(_.atPath(path)) + case _ => ps + } + Result.Warning(ps0, nullBatch(batch)) + } + + // The completions of every batch, in document order. + val runBatches: F[List[Result[Completed]]] = policy match { case EffectErrorPolicy.FailFast => - // Monadic sequencing via `ResultT.flatMap` short-circuits at the first failed - // batch; subsequent batches' effects are not run. (`ResultT.traverse` would not - // do: its `Applicative` combines the underlying `F` actions with `map2`, running - // every batch's effects regardless of failures.) - batchedEffects - .foldLeft(ResultT(List.empty[List[(EffectJson[F], Json)]].success.pure[F])) { - case (acc, ((mapping, handler), batch)) => - acc.flatMap(results => - ResultT(runBatch(mapping, handler, batch)).map(_ :: results)) + def loop( + pending: Chain[Batch], + acc: Chain[Result[Completed]]): F[Chain[Result[Completed]]] = + pending.uncons match { + case None => acc.pure[F] + case Some((((mapping, handler), batch), tl)) => + runBatch(mapping, handler, batch).flatMap { + case Result.Failure(ps) => + // Stop here. This batch and the batches which do not run all complete as + // null. + val unrun = tl.map { case (_, b) => nullBatch(b).success } + ((acc :+ batchFieldError(batch, ps)) ++ unrun).pure[F] + case res if res.hasValue => loop(tl, acc :+ res) + // An internal error is not a field error. It aborts the completion. + case err => (acc :+ err).pure[F] + } } - .map(_.reverse) - .value + loop(Chain.fromSeq(batchedEffects), Chain.empty).map(_.toList) case EffectErrorPolicy.Accumulate => - // Run every batch independently, then combine the per-batch `Result`s with an - // accumulating combinator so that failures from *all* batches are preserved. - batchedEffects - .traverse { case ((mapping, handler), batch) => runBatch(mapping, handler, batch) } - .map(_.parSequence) + // Run every batch, so that the problems of *all* of them are preserved. + batchedEffects.traverse { + case ((mapping, handler), batch) => + runBatch(mapping, handler, batch).map { + case Result.Failure(ps) => batchFieldError(batch, ps) + case other => other + } + } } + // No batch is left as a `Failure`, so this accumulates the problems of all batches and + // propagates any internal error. + val batchedResults = runBatches.map(_.parSequence) + batchedResults.map(_.map { results => val subst = { - val m = new java.util.IdentityHashMap[DeferredJson, Json] + val m = new java.util.IdentityHashMap[DeferredJson, Option[Json]] Monoid.combineAll(results).foreach { case (d, j) => m.put(d, j) } m.asScala } diff --git a/modules/core/src/main/scala/result.scala b/modules/core/src/main/scala/result.scala index 86808e76..7ad0ede3 100644 --- a/modules/core/src/main/scala/result.scala +++ b/modules/core/src/main/scala/result.scala @@ -107,6 +107,18 @@ sealed trait Result[+T] { case _ => None } + /** + * Yields this result with `path` as the response path of each problem which has none. + */ + def atPath(path: List[Problem.PathSegment]): Result[T] = + if (path.isEmpty) this + else + this match { + case Result.Failure(ps) => Result.Failure(ps.map(_.atPath(path))) + case Result.Warning(ps, value) => Result.Warning(ps.map(_.atPath(path)), value) + case other => other + } + def withProblems(problems: NonEmptyChain[Problem]): Result[T] = this match { case Result.Success(value) => Result.Warning(problems, value) diff --git a/modules/core/src/test/scala/compiler/EnvironmentSuite.scala b/modules/core/src/test/scala/compiler/EnvironmentSuite.scala index d7d1d795..64737701 100644 --- a/modules/core/src/test/scala/compiler/EnvironmentSuite.scala +++ b/modules/core/src/test/scala/compiler/EnvironmentSuite.scala @@ -222,7 +222,8 @@ final class EnvironmentSuite extends CatsEffectSuite { { "errors" : [ { - "message" : "Missing argument" + "message" : "Missing argument", + "path" : [ "nested", "url" ] } ], "data" : null diff --git a/modules/core/src/test/scala/compiler/ProblemSuite.scala b/modules/core/src/test/scala/compiler/ProblemSuite.scala index 31eb2cab..89b45646 100644 --- a/modules/core/src/test/scala/compiler/ProblemSuite.scala +++ b/modules/core/src/test/scala/compiler/ProblemSuite.scala @@ -21,12 +21,13 @@ import io.circe.syntax._ import munit.CatsEffectSuite import grackle.Problem +import grackle.Problem.PathSegment.{Index, Name} final class ProblemSuite extends CatsEffectSuite { test("encoding (full)") { assertEquals( - Problem("foo", List(1 -> 2, 5 -> 6), List("bar", "baz")).asJson, + Problem("foo", List(1 -> 2, 5 -> 6), List(Name("bar"), Name("baz"))).asJson, json""" { "message" : "foo", @@ -72,7 +73,7 @@ final class ProblemSuite extends CatsEffectSuite { test("encoding (no locations)") { assertEquals( - Problem("foo", Nil, List("bar", "baz")).asJson, + Problem("foo", Nil, List(Name("bar"), Name("baz"))).asJson, json""" { "message" : "foo", @@ -85,6 +86,29 @@ final class ProblemSuite extends CatsEffectSuite { ) } + test("encoding (list index in path)") { + assertEquals( + Problem("foo", Nil, List(Name("bar"), Index(1), Name("baz"))).asJson, + json""" + { + "message" : "foo", + "path" : [ + "bar", + 1, + "baz" + ] + } + """ + ) + } + + test("toString (list index in path)") { + assertEquals( + Problem("foo", Nil, List(Name("bar"), Index(1), Name("baz"))).toString, + "foo (at bar/1/baz)" + ) + } + test("encoding (message only)") { assertEquals( Problem("foo", Nil, Nil).asJson, @@ -98,7 +122,7 @@ final class ProblemSuite extends CatsEffectSuite { test("toString (full)") { assertEquals( - Problem("foo", List(1 -> 2, 5 -> 6), List("bar", "baz")).toString, + Problem("foo", List(1 -> 2, 5 -> 6), List(Name("bar"), Name("baz"))).toString, "foo (at bar/baz: 1..2, 5..6)" ) } @@ -112,7 +136,7 @@ final class ProblemSuite extends CatsEffectSuite { test("toString (no locations)") { assertEquals( - Problem("foo", Nil, List("bar", "baz")).toString, + Problem("foo", Nil, List(Name("bar"), Name("baz"))).toString, "foo (at bar/baz)" ) } diff --git a/modules/core/src/test/scala/errors/FieldErrorData.scala b/modules/core/src/test/scala/errors/FieldErrorData.scala new file mode 100644 index 00000000..c366033f --- /dev/null +++ b/modules/core/src/test/scala/errors/FieldErrorData.scala @@ -0,0 +1,263 @@ +// Copyright (c) 2016-2025 Association of Universities for Research in Astronomy, Inc. (AURA) +// Copyright (c) 2016-2025 Grackle Contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package errors + +import cats.effect.IO + +import grackle._ +import grackle.Query._ +import grackle.QueryCompiler._ + +/** + * Mappings whose `name` field fails for one item of a list. + * + * The nullability of `name` and of the list decides where the null from that error lands. + */ +object FieldErrorMappings { + + case class Item(id: String, name: String) + + /** + * The item whose name the mapping cannot fetch. + */ + val failingId: String = "2" + + val message: String = s"Name for item $failingId could not be fetched." + + val items: List[Item] = List(Item("1", "one"), Item(failingId, "two"), Item("3", "three")) + + /** + * `name` is nullable, so the null stays at the `name` position. + */ + object NullableName extends ItemMapping(nullableName = true, nullableItems = true) + + /** + * `name` is non-null, so the null bubbles up to the entry of the `items` list. + */ + object NonNullName extends ItemMapping(nullableName = false, nullableItems = true) + + /** + * No position between `name` and the root is nullable, so the null bubbles up to `data`. + */ + object NonNullThroughout extends ItemMapping(nullableName = false, nullableItems = false) + + abstract class ItemMapping(nullableName: Boolean, nullableItems: Boolean) + extends ValueMapping[IO] { + + private val nameTpe = if (nullableName) "String" else "String!" + private val itemsTpe = if (nullableItems) "[Item]" else "[Item!]!" + + val schema: Schema = + mkSchema(s""" + type Query { + ping: String + items: $itemsTpe + } + type Item { + id: ID! + name: $nameTpe + } + """) + + val QueryType = schema.ref("Query") + val ItemType = schema.ref("Item") + + private def itemName(c: Cursor): Result[String] = + c.as[Item].flatMap { item => + if (item.id == failingId) Result.failure(message) + else Result(item.name) + } + + private val nameField: FieldMapping = + if (nullableName) CursorField[Option[String]]("name", c => itemName(c).map(Some(_))) + else CursorField[String]("name", itemName) + + private val itemsField: FieldMapping = + if (nullableItems) ValueField[Unit]("items", _ => Some(items.map(Some(_)))) + else ValueField[Unit]("items", _ => items) + + val typeMappings = + List( + ValueObjectMapping[Unit]( + tpe = QueryType, + fieldMappings = List(ValueField[Unit]("ping", _ => Some("pong")), itemsField)), + ValueObjectMapping[Item]( + tpe = ItemType, + fieldMappings = List(ValueField[Item]("id", _.id), nameField)) + ) + } + + /** + * A mapping whose `name` field yields its value beside a warning. + * + * The warning is raised at the `name` position, so it carries the path of that position. + */ + object WarningName extends ValueMapping[IO] { + val schema: Schema = + mkSchema(""" + type Query { + items: [Item!] + } + type Item { + id: ID! + name: String + } + """) + + val QueryType = schema.ref("Query") + val ItemType = schema.ref("Item") + + private def itemName(c: Cursor): Result[Option[String]] = + c.as[Item].flatMap { item => + if (item.id == failingId) Result.warning(message, Some(item.name)) + else Result(Some(item.name)) + } + + val typeMappings = + List( + ValueObjectMapping[Unit]( + tpe = QueryType, + fieldMappings = List(ValueField[Unit]("items", _ => Some(items)))), + ValueObjectMapping[Item]( + tpe = ItemType, + fieldMappings = + List(ValueField[Item]("id", _.id), CursorField[Option[String]]("name", itemName)) + ) + ) + } + + /** + * A mapping whose count field counts a field which fails. + * + * `tagCount` is nullable, so the null stays at the position of the count field. + */ + object FailingCount extends ValueMapping[IO] { + val schema: Schema = + mkSchema(""" + type Query { + ping: String + tags: [String!] + tagCount: Int + } + """) + + val QueryType = schema.ref("Query") + + val typeMappings = + List( + ValueObjectMapping[Unit]( + tpe = QueryType, + fieldMappings = List( + ValueField[Unit]("ping", _ => Some("pong")), + CursorField[Option[List[String]]]("tags", _ => Result.failure(message)), + ValueField[Unit]("tagCount", _ => 0) + ) + ) + ) + + override val selectElaborator = SelectElaborator { + case (QueryType, "tagCount", _) => + Elab.transformChild(_ => Count(Select("tags"))) + } + } + + /** + * The component of the mappings which delegate. Its `name` field fails. + */ + object FailingComponent extends ValueMapping[IO] { + val schema: Schema = + mkSchema(""" + type Query { + ping: String + delegated: Item! + } + type Item { + name: String! + } + """) + + val QueryType = schema.ref("Query") + val ItemType = schema.ref("Item") + + val typeMappings = + List( + ValueObjectMapping[Unit]( + tpe = QueryType, + fieldMappings = List( + ValueField[Unit]("ping", _ => Some("pong")), + ValueField[Unit]("delegated", _ => Item(failingId, "two")) + ) + ), + ValueObjectMapping[Item]( + tpe = ItemType, + fieldMappings = List(CursorField[String]("name", _ => Result.failure(message)))) + ) + } + + /** + * `delegated` is nullable, so the null stays at the position of the delegated field. + */ + object NullableDelegate extends DelegateMapping(nullableDelegate = true) + + /** + * `delegated` is non-null, so the null bubbles up to `data`. + */ + object NonNullDelegate extends DelegateMapping(nullableDelegate = false) + + /** + * A mapping which delegates both of its fields to [[FailingComponent]]. + * + * Both fields go into one batch, so the batch mixes a failed member with a successful one. + */ + abstract class DelegateMapping(nullableDelegate: Boolean) extends ComposedMapping[IO] { + + private val delegatedTpe = if (nullableDelegate) "Item" else "Item!" + + val schema: Schema = + mkSchema(s""" + type Query { + ping: String + delegated: $delegatedTpe + } + type Item { + name: String! + } + """) + + val QueryType = schema.ref("Query") + + val typeMappings = + List( + ObjectMapping( + tpe = QueryType, + fieldMappings = List( + Delegate("ping", FailingComponent), + Delegate("delegated", FailingComponent) + ) + ) + ) + } + + /** + * Builds a schema from `text`, or throws when `text` is not a valid schema. + */ + private def mkSchema(text: String): Schema = + Schema(text) match { + case Result.Success(s) => s + case Result.Warning(_, s) => s + case other => throw new IllegalArgumentException(other.toProblems.toList.mkString("; ")) + } +} diff --git a/modules/core/src/test/scala/errors/FieldErrorSuite.scala b/modules/core/src/test/scala/errors/FieldErrorSuite.scala new file mode 100644 index 00000000..5a8a685e --- /dev/null +++ b/modules/core/src/test/scala/errors/FieldErrorSuite.scala @@ -0,0 +1,212 @@ +// Copyright (c) 2016-2025 Association of Universities for Research in Astronomy, Inc. (AURA) +// Copyright (c) 2016-2025 Grackle Contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package errors + +import io.circe.Json +import io.circe.literal._ +import munit.CatsEffectSuite + +/** + * Tests for the handling of field errors. + * + * A field error does not discard the response. The failed position completes as null, the null + * bubbles up while the enclosing position is non-null, and the error carries the response path + * of its own position. + * + * @see + * https://spec.graphql.org/September2025/#sec-Handling-Field-Errors + */ +final class FieldErrorSuite extends CatsEffectSuite { + import FieldErrorMappings._ + + private val query = """ + query { + ping + items { + id + name + } + } + """ + + /** + * The response for `query`, with `data` as its data entry. + * + * Every case of `query` reports the same single error at the same position, so only the data + * entry tells the cases apart. + */ + private def expected(data: Json): Json = + json""" + { + "errors": [ + { + "message": $message, + "path": ["items", 1, "name"] + } + ], + "data": $data + } + """ + + test("a field error keeps the data of the positions which succeeded") { + val data = json""" + { + "ping": "pong", + "items": [ + { "id": "1", "name": "one" }, + { "id": "2", "name": null }, + { "id": "3", "name": "three" } + ] + } + """ + + assertIO(NullableName.compileAndRun(query), expected(data)) + } + + test("a null from a non-null position bubbles up to the nearest nullable position") { + val data = json""" + { + "ping": "pong", + "items": [ + { "id": "1", "name": "one" }, + null, + { "id": "3", "name": "three" } + ] + } + """ + + assertIO(NonNullName.compileAndRun(query), expected(data)) + } + + test("a null bubbles up to the data entry when no enclosing position is nullable") { + assertIO(NonNullThroughout.compileAndRun(query), expected(Json.Null)) + } + + test("a warning at a position carries the response path of that position") { + val query = """ + query { + items { + name + } + } + """ + + val expected = json""" + { + "errors": [ + { + "message": $message, + "path": ["items", 1, "name"] + } + ], + "data": { + "items": [ + { "name": "one" }, + { "name": "two" }, + { "name": "three" } + ] + } + } + """ + + assertIO(WarningName.compileAndRun(query), expected) + } + + test("a failed count field keeps the data of the positions which succeeded") { + val query = """ + query { + ping + tagCount + } + """ + + val expected = json""" + { + "errors": [ + { + "message": $message, + "path": ["tagCount"] + } + ], + "data": { + "ping": "pong", + "tagCount": null + } + } + """ + + assertIO(FailingCount.compileAndRun(query), expected) + } + + private val delegateQuery = """ + query { + ping + delegated { + name + } + } + """ + + /** + * The response for `delegateQuery`, with `data` as its data entry. + */ + private def delegateExpected(data: Json): Json = + json""" + { + "errors": [ + { + "message": $message, + "path": ["delegated", "name"] + } + ], + "data": $data + } + """ + + test("a failed delegated field keeps the data of the positions which succeeded") { + val data = json""" + { + "ping": "pong", + "delegated": null + } + """ + + assertIO(NullableDelegate.compileAndRun(delegateQuery), delegateExpected(data)) + } + + test("a null from a non-null delegated field bubbles up to the data entry") { + assertIO(NonNullDelegate.compileAndRun(delegateQuery), delegateExpected(Json.Null)) + } + + test("a response path uses the alias of the position") { + val aliased = """ + query { + entries: items { + name + } + } + """ + + val expected = json"""["entries", 1, "name"]""" + + assertIO( + NullableName + .compileAndRun(aliased) + .map(_.hcursor.downField("errors").downN(0).downField("path").focus), + Some(expected) + ) + } +}