feat!: support partial responses#610
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #610 +/- ##
==========================================
+ Coverage 84.00% 84.09% +0.08%
==========================================
Files 152 151 -1
Lines 4978 4999 +21
Branches 861 861
==========================================
+ Hits 4182 4204 +22
Misses 495 495
+ Partials 301 300 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Adds support for proper error handling and partial responses as defined by the spec, cf. https://spec.graphql.org/September2025/#sec-Errors KGraphQL now distinguishes between _request_ and _execution_ errors, and the latter now result in a response that contains both, `data` and `errors`. Execution errors result in a value of `null` for nullable fields, and will bubble up to their parent node for non-nullable fields. Additionally, resolvers can now raise execution errors while still returning data. If data _and_ errors are present in the response, the `errors` key is serialized first to make it more apparent that errors are present. Behavior for request errors is unchanged, and `wrapErrors` configuration is still supported for now - but likely subject to change in the future. Resolves #114 BREAKING CHANGE: relying on exceptions being thrown from query execution may no longer work as before. It is advised to specify `wrapErrors = false` and report an issue with the respective use case.
acea407 to
c70d308
Compare
|
@grumpy-programmer Sorry, this took way longer than anticipated but - following our conversation in #437 - if you're still interested in discussing error handling then I'd be happy to receive your thoughts here and in the linked pull requests 🙂 |
|
@stuebingerb sure, happy to help. I'll check it out at the weekend, if you don't mind, because this is beefy PR 😄 |
Sure, take your time! This will likely stay open for a while anyway. |
|
Really enjoyed going through this. While reviewing, I also caught up on the changes from #560 - the separation between I particularly like the test case with @Test
fun `resolvers should be able to throw custom request errors`() {
data class Item(val data: String)
class ForbiddenError(node: Execution.Node, message: String) :
RequestError(message, node = node.selectionNode, extensions = mapOf("type" to "FORBIDDEN"))
val schema = KGraphQL.schema {
query("items") {
resolver<Item, Execution.Node> { node: Execution.Node ->
throw ForbiddenError(node, "Not allowed")
}
}
}
schema.executeBlocking("{ items { data }}") shouldBe """
{"errors":[{"message":"Not allowed","locations":[{"line":1,"column":3}],"extensions":{"type":"FORBIDDEN"}}]}
""".trimIndent()
}That illustrates how flexible error handling has become, because the developer takes responsibility for how a request can fail: globally Another nice flavor is I appreciate the tremendous amount of work that went into this entire PR series with the error handling changes. Great job! |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
There was a problem hiding this comment.
7 issues found across 42 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/content/Reference/errorHandling.md">
<violation number="1" location="docs/content/Reference/errorHandling.md:107">
P2: The docs incorrectly say the list entry would be “missing”; it should be `null` to match GraphQL semantics and the example response.</violation>
</file>
<file name="kgraphql/src/main/kotlin/com/apurebase/kgraphql/Context.kt">
<violation number="1" location="kgraphql/src/main/kotlin/com/apurebase/kgraphql/Context.kt:25">
P1: `Context.plus` shares the same mutable `errors` list, which can leak execution errors between derived contexts/requests.</violation>
</file>
<file name="kgraphql-ktor-stitched/src/main/kotlin/com/apurebase/kgraphql/stitched/schema/execution/AbstractRemoteRequestExecutor.kt">
<violation number="1" location="kgraphql-ktor-stitched/src/main/kotlin/com/apurebase/kgraphql/stitched/schema/execution/AbstractRemoteRequestExecutor.kt:72">
P2: Do not swallow coroutine cancellation here; rethrow `CancellationException` before converting failures to `ExecutionError`.</violation>
</file>
<file name="kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ArgumentTransformer.kt">
<violation number="1" location="kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ArgumentTransformer.kt:36">
P1: Unsupported argument coercion is now thrown as `ValidationException` (request error), which can incorrectly abort execution instead of returning a field-level execution error.</violation>
</file>
<file name="kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/scalar/Coercion.kt">
<violation number="1" location="kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/scalar/Coercion.kt:49">
P2: This catch path converts existing `ExecutionError`s into `InvalidInputValueException`, which changes error classification (e.g., `INTERNAL_SERVER_ERROR`/custom extensions become `BAD_USER_INPUT`). Preserve `ExecutionError` instead of re-wrapping it.</violation>
</file>
<file name="kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ParallelRequestExecutor.kt">
<violation number="1" location="kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ParallelRequestExecutor.kt:112">
P1: `data` nullability is derived from `resultMap` emptiness, so root-level non-null propagation can be serialized as partial object data instead of `data: null`.</violation>
<violation number="2" location="kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ParallelRequestExecutor.kt:308">
P2: Nested object field resolution no longer uses the configured coroutine dispatcher; it falls back to `Dispatchers.Default`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| inline fun <reified T : Any> get(): T? = get(T::class) | ||
|
|
||
| operator fun <T : Any> plus(value: T): Context = Context(map + (value::class to value)) | ||
| operator fun <T : Any> plus(value: T): Context = Context(map + (value::class to value), errors) |
There was a problem hiding this comment.
P1: Context.plus shares the same mutable errors list, which can leak execution errors between derived contexts/requests.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At kgraphql/src/main/kotlin/com/apurebase/kgraphql/Context.kt, line 25:
<comment>`Context.plus` shares the same mutable `errors` list, which can leak execution errors between derived contexts/requests.</comment>
<file context>
@@ -18,5 +22,7 @@ class Context(private val map: Map<KClass<*>, Any>) {
inline fun <reified T : Any> get(): T? = get(T::class)
- operator fun <T : Any> plus(value: T): Context = Context(map + (value::class to value))
+ operator fun <T : Any> plus(value: T): Context = Context(map + (value::class to value), errors)
+
+ fun raiseError(error: ExecutionError) = errors.add(error)
</file context>
| operator fun <T : Any> plus(value: T): Context = Context(map + (value::class to value), errors) | |
| operator fun <T : Any> plus(value: T): Context = Context(map + (value::class to value)) |
| throw ValidationException( | ||
| "'$funName' does support arguments: ${inputValues.map { it.name }}. Found arguments: ${args.keys}", | ||
| executionNode.selectionNode | ||
| ) |
There was a problem hiding this comment.
P1: Unsupported argument coercion is now thrown as ValidationException (request error), which can incorrectly abort execution instead of returning a field-level execution error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ArgumentTransformer.kt, line 36:
<comment>Unsupported argument coercion is now thrown as `ValidationException` (request error), which can incorrectly abort execution instead of returning a field-level execution error.</comment>
<file context>
@@ -31,7 +33,7 @@ open class ArgumentTransformer(val genericTypeResolver: GenericTypeResolver) {
if (unsupportedArguments?.isNotEmpty() == true) {
- throw InvalidInputValueException(
+ throw ValidationException(
"'$funName' does support arguments: ${inputValues.map { it.name }}. Found arguments: ${args.keys}",
executionNode.selectionNode
</file context>
| throw ValidationException( | |
| "'$funName' does support arguments: ${inputValues.map { it.name }}. Found arguments: ${args.keys}", | |
| executionNode.selectionNode | |
| ) | |
| throw InvalidInputValueException( | |
| "'$funName' does support arguments: ${inputValues.map { it.name }}. Found arguments: ${args.keys}", | |
| executionNode | |
| ) |
| plan.associate { operation -> executeOperation(operation) } | ||
| } | ||
|
|
||
| val data = if (resultMap.values.any { it != null }) { |
There was a problem hiding this comment.
P1: data nullability is derived from resultMap emptiness, so root-level non-null propagation can be serialized as partial object data instead of data: null.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ParallelRequestExecutor.kt, line 112:
<comment>`data` nullability is derived from `resultMap` emptiness, so root-level non-null propagation can be serialized as partial object data instead of `data: null`.</comment>
<file context>
@@ -74,63 +74,92 @@ class ParallelRequestExecutor(val schema: DefaultSchema) : RequestExecutor {
+ plan.associate { operation -> executeOperation(operation) }
+ }
+
+ val data = if (resultMap.values.any { it != null }) {
+ jsonNodeFactory.objectNode()
+ } else {
</file context>
| ``` | ||
|
|
||
| === "Response (HTTP 200)" | ||
| If the field `name` was declared as non-null, the whole list entry would be missing instead. However, the error itself would still be the same: |
There was a problem hiding this comment.
P2: The docs incorrectly say the list entry would be “missing”; it should be null to match GraphQL semantics and the example response.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/content/Reference/errorHandling.md, line 107:
<comment>The docs incorrectly say the list entry would be “missing”; it should be `null` to match GraphQL semantics and the example response.</comment>
<file context>
@@ -1,83 +1,179 @@
```
-=== "Response (HTTP 200)"
+If the field `name` was declared as non-null, the whole list entry would be missing instead. However, the error itself would still be the same:
+
+=== "SDL"
</file context>
| If the field `name` was declared as non-null, the whole list entry would be missing instead. However, the error itself would still be the same: | |
| If the field `name` was declared as non-null, the whole list entry would be `null` instead. However, the error itself would still be the same: |
| }.getOrElse { | ||
| ctx.raiseError( | ||
| ExecutionError(it.message ?: it.javaClass.simpleName, node, it, node.errorExtensions()) | ||
| ) | ||
| null | ||
| } |
There was a problem hiding this comment.
P2: Do not swallow coroutine cancellation here; rethrow CancellationException before converting failures to ExecutionError.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At kgraphql-ktor-stitched/src/main/kotlin/com/apurebase/kgraphql/stitched/schema/execution/AbstractRemoteRequestExecutor.kt, line 72:
<comment>Do not swallow coroutine cancellation here; rethrow `CancellationException` before converting failures to `ExecutionError`.</comment>
<file context>
@@ -34,59 +52,36 @@ abstract class AbstractRemoteRequestExecutor(private val objectMapper: ObjectMap
}
- return responseJson["data"]?.get(node.remoteOperation)
+ response.data?.get(node.remoteOperation)
+ }.getOrElse {
+ ctx.raiseError(
+ ExecutionError(it.message ?: it.javaClass.simpleName, node, it, node.errorExtensions())
</file context>
| }.getOrElse { | |
| ctx.raiseError( | |
| ExecutionError(it.message ?: it.javaClass.simpleName, node, it, node.errorExtensions()) | |
| ) | |
| null | |
| } | |
| }.getOrElse { | |
| if (it is kotlinx.coroutines.CancellationException) throw it | |
| ctx.raiseError( | |
| ExecutionError(it.message ?: it.javaClass.simpleName, node, it, node.errorExtensions()) | |
| ) | |
| null | |
| } |
| throw InvalidInputValueException( | ||
| message = "Cannot coerce '${value.valueNodeName}' to ${scalar.name}", | ||
| node = value, | ||
| message = (e as? GraphQLError)?.message ?: "Cannot coerce '${value.valueNodeName}' to ${scalar.name}", |
There was a problem hiding this comment.
P2: This catch path converts existing ExecutionErrors into InvalidInputValueException, which changes error classification (e.g., INTERNAL_SERVER_ERROR/custom extensions become BAD_USER_INPUT). Preserve ExecutionError instead of re-wrapping it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/scalar/Coercion.kt, line 49:
<comment>This catch path converts existing `ExecutionError`s into `InvalidInputValueException`, which changes error classification (e.g., `INTERNAL_SERVER_ERROR`/custom extensions become `BAD_USER_INPUT`). Preserve `ExecutionError` instead of re-wrapping it.</comment>
<file context>
@@ -39,17 +39,15 @@ fun <T : Any> deserializeScalar(scalar: Type.Scalar<T>, value: ValueNode): T {
throw InvalidInputValueException(
- message = "Cannot coerce '${value.valueNodeName}' to ${scalar.name}",
- node = value,
+ message = (e as? GraphQLError)?.message ?: "Cannot coerce '${value.valueNodeName}' to ${scalar.name}",
+ node = executionNode,
originalError = e
</file context>
| message = (e as? GraphQLError)?.message ?: "Cannot coerce '${value.valueNodeName}' to ${scalar.name}", | |
| message = when (e) { | |
| is ExecutionError -> throw e | |
| is GraphQLError -> e.message | |
| else -> "Cannot coerce '${value.valueNodeName}' to ${scalar.name}" | |
| }, |
| val objectNode = jsonNodeFactory.objectNode() | ||
| val deferreds = mutableListOf<Deferred<Map<String, Deferred<JsonNode?>?>>>() | ||
| for (child in node.children) { | ||
| val deferreds = node.children.mapIndexedParallel { _, child -> |
There was a problem hiding this comment.
P2: Nested object field resolution no longer uses the configured coroutine dispatcher; it falls back to Dispatchers.Default.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ParallelRequestExecutor.kt, line 308:
<comment>Nested object field resolution no longer uses the configured coroutine dispatcher; it falls back to `Dispatchers.Default`.</comment>
<file context>
@@ -248,35 +294,30 @@ class ParallelRequestExecutor(val schema: DefaultSchema) : RequestExecutor {
val objectNode = jsonNodeFactory.objectNode()
- val deferreds = mutableListOf<Deferred<Map<String, Deferred<JsonNode?>?>>>()
- for (child in node.children) {
+ val deferreds = node.children.mapIndexedParallel { _, child ->
when (child) {
- is Execution.Fragment -> deferreds.add(ctx.scope.async {
</file context>
| val deferreds = node.children.mapIndexedParallel { _, child -> | |
| val deferreds = node.children.mapIndexedParallel(dispatcher) { _, child -> |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/DefaultSchema.kt (1)
52-54: Consider catching JSON parsing exceptions.If
configuration.objectMapper.readTree(variables)throws (e.g., malformed JSON), it will propagate as a Jackson exception rather than a serialized GraphQL error. This could cause HTTP 500 responses instead of proper GraphQL error responses.♻️ Proposed fix to handle malformed variable JSON
try { + val parsedVariables = try { + variables + ?.let { VariablesJson.Defined(configuration.objectMapper.readTree(variables)) } + ?: VariablesJson.Empty() + } catch (e: Exception) { + throw ValidationException("Invalid JSON in variables: ${e.message}") + } - val parsedVariables = variables - ?.let { VariablesJson.Defined(configuration.objectMapper.readTree(variables)) } - ?: VariablesJson.Empty()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/DefaultSchema.kt` around lines 52 - 54, Wrap the call to configuration.objectMapper.readTree(variables) (used to build parsedVariables / VariablesJson.Defined) in a try-catch for com.fasterxml.jackson.core.JsonProcessingException (or general Jackson parsing exceptions); on catch, do not let the exception propagate as a raw exception — instead convert it into a GraphQL-friendly error path (for example return VariablesJson.Empty() and/or throw a graphql error type that your execution layer serializes, with a clear message like "Malformed variables JSON") so malformed JSON yields a serialized GraphQL error rather than an HTTP 500.docs/content/Reference/errorHandling.md (1)
13-13: Minor style suggestion: Consider "outside" instead of "outside of".Static analysis flagged "outside of" as potentially redundant. The phrase "outside the schema" is slightly more concise.
-Additionally, every error can contain an entry with the key `extensions` that is a map of server-specific additions outside of the schema. +Additionally, every error can contain an entry with the key `extensions` that is a map of server-specific additions outside the schema.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/Reference/errorHandling.md` at line 13, Update the sentence that describes the `extensions` entry to use the more concise phrasing "outside the schema" instead of "outside of the schema": locate the line discussing the `extensions` map and replace "that is a map of server-specific additions outside of the schema" with "that is a map of server-specific additions outside the schema" so the documentation for `extensions` reads more succinctly while preserving meaning.kgraphql-ktor-stitched/src/main/kotlin/com/apurebase/kgraphql/stitched/schema/execution/AbstractRemoteRequestExecutor.kt (1)
36-36: Typo in comment: "reponse" should be "response".- // reponse location is not mapped because we want the local location anyway + // response location is not mapped because we want the local location anyway🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kgraphql-ktor-stitched/src/main/kotlin/com/apurebase/kgraphql/stitched/schema/execution/AbstractRemoteRequestExecutor.kt` at line 36, Typo in the comment inside class AbstractRemoteRequestExecutor: change "reponse" to "response" in the inline comment ("// reponse location is not mapped because we want the local location anyway") so the comment reads "// response location is not mapped because we want the local location anyway".kgraphql/src/testFixtures/kotlin/com/apurebase/kgraphql/CommonTestUtils.kt (1)
36-41: Consider extendingerrorTypemapping for completeness.The current mapping covers the main exception types, but
ExecutionExceptionandExecutionErrorwould both fall through to theelsebranch returningINTERNAL_SERVER_ERROR. This works correctly but could be made more explicit.Also, if a caller uses
expectRequestError<RequestError>(the base class), it would incorrectly expectINTERNAL_SERVER_ERRORinstead ofBAD_USER_INPUT.Consider making base class handling explicit
inline fun <reified T : GraphQLError> errorType() = when (T::class) { InvalidInputValueException::class -> BuiltInErrorCodes.BAD_USER_INPUT.name ValidationException::class -> BuiltInErrorCodes.GRAPHQL_VALIDATION_FAILED.name InvalidSyntaxException::class -> BuiltInErrorCodes.GRAPHQL_PARSE_FAILED.name + ExecutionException::class -> BuiltInErrorCodes.INTERNAL_SERVER_ERROR.name + // Base classes default to their typical error codes + RequestError::class -> BuiltInErrorCodes.BAD_USER_INPUT.name else -> BuiltInErrorCodes.INTERNAL_SERVER_ERROR.name }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kgraphql/src/testFixtures/kotlin/com/apurebase/kgraphql/CommonTestUtils.kt` around lines 36 - 41, The when-mapping in errorType() misses explicit branches for RequestError, ExecutionException and ExecutionError so base-class callers fall through to INTERNAL_SERVER_ERROR; update the when in errorType() to add cases: RequestError::class -> BuiltInErrorCodes.BAD_USER_INPUT.name, ExecutionException::class -> BuiltInErrorCodes.INTERNAL_SERVER_ERROR.name, and ExecutionError::class -> BuiltInErrorCodes.INTERNAL_SERVER_ERROR.name (keeping existing mappings for InvalidInputValueException, ValidationException, InvalidSyntaxException) so behavior is explicit and callers using RequestError get BAD_USER_INPUT.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/EnumsSpecificationTest.kt`:
- Around line 36-38: The test currently asserts a runtime execution error by
using expectExecutionError<InvalidInputValueException> for passing a string
literal to an enum input; instead, update the assertion to expect a
request/validation-phase error (e.g., replace
expectExecutionError<InvalidInputValueException> with
expectRequestError<InvalidInputValueException>) so the failing call to
schema.executeBlocking("{cool(cool : \"COOL\")}") is validated as a request
error; locate and update the assertion that references expectExecutionError and
InvalidInputValueException in EnumsSpecificationTest.kt to the request-phase
assertion variant.
In
`@kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/ListsSpecificationTest.kt`:
- Around line 65-67: The test currently asserts a variable coercion failure as
an execution error; update the expectation to a request error by replacing
expectExecutionError<InvalidInputValueException> with
expectRequestError<InvalidInputValueException> for the case that runs
schema.executeBlocking("query(\$list: [String!]!) { list(list: \$list) }",
variables) so the null-in-list variable coercion is treated as a request
validation error rather than an execution error.
In
`@kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/ScalarsSpecificationTest.kt`:
- Around line 247-269: Tests in ScalarsSpecificationTest.kt assert
InvalidInputValueException for ID coercion but ID_COERCION in BuiltInScalars.kt
currently throws ValidationException; update the implementation to throw
InvalidInputValueException instead (or, if you prefer to change the tests,
update the expectExecutionError calls to expect ValidationException) so both
sides use the same exception type; modify the ID_COERCION handler in
BuiltInScalars.kt to construct/throw InvalidInputValueException with the same
message format used elsewhere to keep test messages consistent.
- Around line 148-150: The test fails because BuiltInScalars.kt currently throws
ValidationException for Int overflow but the test
(expectExecutionError<InvalidInputValueException>) expects an
InvalidInputValueException (BAD_USER_INPUT); update the scalar coercion path
that handles integer parsing/overflow (the Int scalar coercion function in
BuiltInScalars.kt — e.g., the coerce/parse branch that currently throws
ValidationException) to throw InvalidInputValueException with the same error
message so errors[index].extensions.type becomes BAD_USER_INPUT and the existing
expectExecutionError<InvalidInputValueException> assertion passes.
---
Nitpick comments:
In `@docs/content/Reference/errorHandling.md`:
- Line 13: Update the sentence that describes the `extensions` entry to use the
more concise phrasing "outside the schema" instead of "outside of the schema":
locate the line discussing the `extensions` map and replace "that is a map of
server-specific additions outside of the schema" with "that is a map of
server-specific additions outside the schema" so the documentation for
`extensions` reads more succinctly while preserving meaning.
In
`@kgraphql-ktor-stitched/src/main/kotlin/com/apurebase/kgraphql/stitched/schema/execution/AbstractRemoteRequestExecutor.kt`:
- Line 36: Typo in the comment inside class AbstractRemoteRequestExecutor:
change "reponse" to "response" in the inline comment ("// reponse location is
not mapped because we want the local location anyway") so the comment reads "//
response location is not mapped because we want the local location anyway".
In `@kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/DefaultSchema.kt`:
- Around line 52-54: Wrap the call to
configuration.objectMapper.readTree(variables) (used to build parsedVariables /
VariablesJson.Defined) in a try-catch for
com.fasterxml.jackson.core.JsonProcessingException (or general Jackson parsing
exceptions); on catch, do not let the exception propagate as a raw exception —
instead convert it into a GraphQL-friendly error path (for example return
VariablesJson.Empty() and/or throw a graphql error type that your execution
layer serializes, with a clear message like "Malformed variables JSON") so
malformed JSON yields a serialized GraphQL error rather than an HTTP 500.
In `@kgraphql/src/testFixtures/kotlin/com/apurebase/kgraphql/CommonTestUtils.kt`:
- Around line 36-41: The when-mapping in errorType() misses explicit branches
for RequestError, ExecutionException and ExecutionError so base-class callers
fall through to INTERNAL_SERVER_ERROR; update the when in errorType() to add
cases: RequestError::class -> BuiltInErrorCodes.BAD_USER_INPUT.name,
ExecutionException::class -> BuiltInErrorCodes.INTERNAL_SERVER_ERROR.name, and
ExecutionError::class -> BuiltInErrorCodes.INTERNAL_SERVER_ERROR.name (keeping
existing mappings for InvalidInputValueException, ValidationException,
InvalidSyntaxException) so behavior is explicit and callers using RequestError
get BAD_USER_INPUT.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cfd51f98-dcc0-484d-b73b-ff96f92ed2f7
📒 Files selected for processing (42)
docs/content/Reference/errorHandling.mdgradle/libs.versions.tomlkgraphql-ktor-stitched/api/kgraphql-ktor-stitched.apikgraphql-ktor-stitched/src/main/kotlin/com/apurebase/kgraphql/stitched/RemoteExecutionException.ktkgraphql-ktor-stitched/src/main/kotlin/com/apurebase/kgraphql/stitched/schema/execution/AbstractRemoteRequestExecutor.ktkgraphql-ktor-stitched/src/test/kotlin/com/apurebase/kgraphql/stitched/schema/execution/StitchedSchemaExecutionTest.ktkgraphql-ktor/src/test/kotlin/com/apurebase/kgraphql/KtorFeatureTest.ktkgraphql/api/kgraphql.apikgraphql/build.gradle.ktskgraphql/src/main/kotlin/com/apurebase/kgraphql/Context.ktkgraphql/src/main/kotlin/com/apurebase/kgraphql/GraphQLError.ktkgraphql/src/main/kotlin/com/apurebase/kgraphql/helpers/KGraphQLExtensions.ktkgraphql/src/main/kotlin/com/apurebase/kgraphql/request/Variables.ktkgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/DefaultSchema.ktkgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/builtin/BuiltInScalars.ktkgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ArgumentTransformer.ktkgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ParallelRequestExecutor.ktkgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/scalar/Coercion.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/access/AccessRulesTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/configuration/SchemaConfigurationTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/integration/MutationTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/integration/ParallelExecutionTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/integration/QueryTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/schema/SchemaBuilderTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/schema/SchemaInheritanceTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/introspection/IntrospectionSpecificationTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/language/FragmentsSpecificationTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/language/InputValuesSpecificationTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/language/ListInputCoercionTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/language/OperationsSpecificationTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/language/QueryDocumentSpecificationTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/language/SourceTextSpecificationTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/language/VariablesSpecificationTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/DirectivesSpecificationTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/EnumsSpecificationTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/InputObjectsSpecificationTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/InterfacesSpecificationTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/ListsSpecificationTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/NonNullSpecificationTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/ScalarsSpecificationTest.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/UnionsSpecificationTest.ktkgraphql/src/testFixtures/kotlin/com/apurebase/kgraphql/CommonTestUtils.kt
💤 Files with no reviewable changes (2)
- kgraphql-ktor-stitched/src/main/kotlin/com/apurebase/kgraphql/stitched/RemoteExecutionException.kt
- kgraphql-ktor-stitched/api/kgraphql-ktor-stitched.api
| expectExecutionError<InvalidInputValueException>("Cannot coerce string literal '\"COOL\"' to enum Coolness") { | ||
| schema.executeBlocking("{cool(cool : \"COOL\")}") | ||
| } |
There was a problem hiding this comment.
Enum literal type mismatch should be asserted as request error.
At Line 36, cool: "COOL" (string literal) for enum input is a validation/request-phase error, not an execution-phase field error. This expectation currently cements non-spec behavior.
Suggested test expectation update
-import com.apurebase.kgraphql.InvalidInputValueException
+import com.apurebase.kgraphql.ValidationException
import com.apurebase.kgraphql.expect
-import com.apurebase.kgraphql.expectExecutionError
+import com.apurebase.kgraphql.expectRequestError
@@
- expectExecutionError<InvalidInputValueException>("Cannot coerce string literal '\"COOL\"' to enum Coolness") {
+ expectRequestError<ValidationException>("Cannot coerce string literal '\"COOL\"' to enum Coolness") {
schema.executeBlocking("{cool(cool : \"COOL\")}")
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/EnumsSpecificationTest.kt`
around lines 36 - 38, The test currently asserts a runtime execution error by
using expectExecutionError<InvalidInputValueException> for passing a string
literal to an enum input; instead, update the assertion to expect a
request/validation-phase error (e.g., replace
expectExecutionError<InvalidInputValueException> with
expectRequestError<InvalidInputValueException>) so the failing call to
schema.executeBlocking("{cool(cool : \"COOL\")}") is validated as a request
error; locate and update the assertion that references expectExecutionError and
InvalidInputValueException in EnumsSpecificationTest.kt to the request-phase
assertion variant.
| expectExecutionError<InvalidInputValueException>("Cannot coerce 'null' to String") { | ||
| schema.executeBlocking("query(\$list: [String!]!) { list(list: \$list) }", variables) | ||
| } |
There was a problem hiding this comment.
This test locks in the wrong error category for variable coercion.
At Line 65, invalid variable value coercion (null inside [String!]!) should be asserted as a request error, not an execution error. Keeping it as execution error conflicts with spec-aligned request/execution separation and can mask the real engine bug.
Suggested test expectation update
-import com.apurebase.kgraphql.InvalidInputValueException
-import com.apurebase.kgraphql.expectExecutionError
+import com.apurebase.kgraphql.ValidationException
+import com.apurebase.kgraphql.expectRequestError
@@
- expectExecutionError<InvalidInputValueException>("Cannot coerce 'null' to String") {
+ expectRequestError<ValidationException>("Cannot coerce 'null' to String") {
schema.executeBlocking("query(\$list: [String!]!) { list(list: \$list) }", variables)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/ListsSpecificationTest.kt`
around lines 65 - 67, The test currently asserts a variable coercion failure as
an execution error; update the expectation to a request error by replacing
expectExecutionError<InvalidInputValueException> with
expectRequestError<InvalidInputValueException> for the case that runs
schema.executeBlocking("query(\$list: [String!]!) { list(list: \$list) }",
variables) so the null-in-list variable coercion is treated as a request
validation error rather than an execution error.
| expectExecutionError<InvalidInputValueException>("Cannot coerce '${Integer.MAX_VALUE.toLong() + 2L}' to Int as it is greater than (2^-31)-1") { | ||
| schema.executeBlocking("mutation { Int(int: ${Integer.MAX_VALUE.toLong() + 2L}) }") | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify what exception type INT_COERCION throws
rg -n "throw.*Exception" kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/builtin/BuiltInScalars.kt
# Check if InvalidInputValueException extends ExecutionError
rg -n "class InvalidInputValueException" --type=kt -A 3Repository: stuebingerb/KGraphQL
Length of output: 1638
🏁 Script executed:
rg -n "class ValidationException" --type-list --type python --type go | head -20Repository: stuebingerb/KGraphQL
Length of output: 453
🏁 Script executed:
rg -n "class ValidationException" -A 3Repository: stuebingerb/KGraphQL
Length of output: 540
🏁 Script executed:
rg -n "class InvalidInputValueException" -A 3Repository: stuebingerb/KGraphQL
Length of output: 529
🏁 Script executed:
rg -n "fun expectExecutionError" -A 10Repository: stuebingerb/KGraphQL
Length of output: 46
🏁 Script executed:
rg -n "expectExecutionError" kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/ScalarsSpecificationTest.kt | head -5Repository: stuebingerb/KGraphQL
Length of output: 560
🏁 Script executed:
rg -n "fun expectExecutionError" -B 2 -A 15Repository: stuebingerb/KGraphQL
Length of output: 46
🏁 Script executed:
rg -n "class RequestError" -A 5Repository: stuebingerb/KGraphQL
Length of output: 644
🏁 Script executed:
rg -n "class ExecutionError" -A 5Repository: stuebingerb/KGraphQL
Length of output: 695
🏁 Script executed:
find . -name "*.kt" -type f | xargs grep -l "fun expectExecutionError" 2>/dev/nullRepository: stuebingerb/KGraphQL
Length of output: 48
🏁 Script executed:
rg -n "inline fun expectExecutionError\|fun expectExecutionError" -B 2 -A 20Repository: stuebingerb/KGraphQL
Length of output: 46
🏁 Script executed:
rg -n "expectExecutionError" -lRepository: stuebingerb/KGraphQL
Length of output: 1371
🏁 Script executed:
fd "\.kt$" -type f | head -20Repository: stuebingerb/KGraphQL
Length of output: 234
🏁 Script executed:
cat kgraphql/src/testFixtures/kotlin/com/apurebase/kgraphql/CommonTestUtils.ktRepository: stuebingerb/KGraphQL
Length of output: 1981
Test will fail due to exception type mismatch.
The test expects InvalidInputValueException (error type BAD_USER_INPUT), but BuiltInScalars.kt throws ValidationException (error type GRAPHQL_VALIDATION_FAILED). The expectExecutionError helper checks that errors[index].extensions.type matches the expected error type. Since the thrown exception type differs from what the test asserts, the error type check will fail.
Either update the test to expect expectRequestError<ValidationException>() or change the implementation to throw InvalidInputValueException.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/ScalarsSpecificationTest.kt`
around lines 148 - 150, The test fails because BuiltInScalars.kt currently
throws ValidationException for Int overflow but the test
(expectExecutionError<InvalidInputValueException>) expects an
InvalidInputValueException (BAD_USER_INPUT); update the scalar coercion path
that handles integer parsing/overflow (the Int scalar coercion function in
BuiltInScalars.kt — e.g., the coerce/parse branch that currently throws
ValidationException) to throw InvalidInputValueException with the same error
message so errors[index].extensions.type becomes BAD_USER_INPUT and the existing
expectExecutionError<InvalidInputValueException> assertion passes.
| expectExecutionError<InvalidInputValueException>("Cannot coerce '4.0' to ID") { | ||
| testedSchema.executeBlocking("query(\$id: ID! = 4.0) { personById(id: \$id) { id, name } }") | ||
| } | ||
|
|
||
| // Boolean (should fail) | ||
| expect<InvalidInputValueException>("Cannot coerce 'true' to ID") { | ||
| expectExecutionError<InvalidInputValueException>("Cannot coerce 'true' to ID") { | ||
| testedSchema.executeBlocking("query(\$id: ID! = true) { personById(id: \$id) { id, name } }") | ||
| } | ||
|
|
||
| // List of strings (should fail) | ||
| expect<InvalidInputValueException>("Cannot coerce '[\"4\", \"5\"]' to ID") { | ||
| expectExecutionError<InvalidInputValueException>("Cannot coerce '[\"4\", \"5\"]' to ID") { | ||
| testedSchema.executeBlocking("query(\$id: ID! = [\"4\", \"5\"]) { personById(id: \$id) { id, name } }") | ||
| } | ||
|
|
||
| // Object (should fail) | ||
| expect<InvalidInputValueException>("Property 'value' on 'ID' does not exist") { | ||
| expectExecutionError<InvalidInputValueException>("Property 'value' on 'ID' does not exist") { | ||
| testedSchema.executeBlocking("query(\$id: ID! = {value: \"4\"}) { personById(id: \$id) { id, name } }") | ||
| } | ||
|
|
||
| // Null (should fail) | ||
| expect<InvalidInputValueException>("Cannot coerce 'null' to ID") { | ||
| expectExecutionError<InvalidInputValueException>("Cannot coerce 'null' to ID") { | ||
| testedSchema.executeBlocking("query(\$id: ID! = null) { personById(id: \$id) { id, name } }") | ||
| } |
There was a problem hiding this comment.
Same inconsistency applies to ID coercion tests.
These tests also expect InvalidInputValueException but ID_COERCION in BuiltInScalars.kt (line 178) throws ValidationException.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/ScalarsSpecificationTest.kt`
around lines 247 - 269, Tests in ScalarsSpecificationTest.kt assert
InvalidInputValueException for ID coercion but ID_COERCION in BuiltInScalars.kt
currently throws ValidationException; update the implementation to throw
InvalidInputValueException instead (or, if you prefer to change the tests,
update the expectExecutionError calls to expect ValidationException) so both
sides use the same exception type; modify the ID_COERCION handler in
BuiltInScalars.kt to construct/throw InvalidInputValueException with the same
message format used elsewhere to keep test messages consistent.
There was a problem hiding this comment.
3 issues found across 42 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/builtin/BuiltInScalars.kt">
<violation number="1" location="kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/builtin/BuiltInScalars.kt:164">
P2: Include `valueNode` when throwing this `ValidationException` to preserve GraphQL error locations.</violation>
</file>
<file name="kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/language/VariablesSpecificationTest.kt">
<violation number="1" location="kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/language/VariablesSpecificationTest.kt:49">
P2: Variable coercion failures are request errors, so these cases should not be asserted as execution errors.</violation>
</file>
<file name="kgraphql/src/main/kotlin/com/apurebase/kgraphql/helpers/KGraphQLExtensions.kt">
<violation number="1" location="kgraphql/src/main/kotlin/com/apurebase/kgraphql/helpers/KGraphQLExtensions.kt:145">
P2: `extensions` are serialized with `valueToTree`, which can fail for arbitrary `Map<String, Any?>` values and break error response serialization. Use the existing defensive conversion path so unknown extension value types are stringified instead of throwing.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client
participant Schema as DefaultSchema
participant Parser as Request Interpreter
participant Executor as ParallelRequestExecutor
participant Context as Execution Context
participant Resolver as Field Resolver / Data Loader
participant Remote as Remote Request Executor
Note over Client,Remote: KGraphQL Runtime Request Flow (Sept 2025 Spec Support)
Client->>Schema: execute(request, variables, context)
Schema->>Parser: parseDocument & createExecutionPlan
alt Request Error (Parsing/Validation/Introspection)
Parser-->>Schema: throw RequestError (e.g., ValidationException)
Schema->>Schema: CHANGED: serialize error without data key
Schema-->>Client: { "errors": [...] }
else Valid Request
Parser-->>Schema: ExecutionPlan
Schema->>Executor: suspendExecute(plan, variables, context)
Executor->>Context: NEW: Initialize shared thread-safe error list
loop For each field in ExecutionPlan
Executor->>Context: Check access rules / directives
alt Local Resolver
Executor->>Resolver: invoke()
alt Success
Resolver-->>Executor: data
else Execution Error (Caught)
Resolver-->>Executor: throw Exception / NEW: ctx.raiseError()
Executor->>Context: NEW: Add ExecutionError with path & location
Executor->>Executor: CHANGED: Apply nullability rules (null field or bubble up)
end
else Remote Resolver (Stitched)
Executor->>Remote: execute(node, ctx)
Remote->>Remote: Fetch external GraphQL
opt Remote Errors Present
Remote->>Context: NEW: Map and raise errors with adjusted paths
end
Remote-->>Executor: partial data
end
end
Executor->>Executor: NEW: Finalize result tree (handling null-bubbling)
Executor->>Executor: NEW: Serialize JSON (errors key FIRST, then data)
Executor-->>Schema: JSON Result String
Schema-->>Client: { "errors": [...], "data": {...} }
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| 1L -> true | ||
|
|
||
| else -> throw IllegalArgumentException("Cannot coerce '${valueNode.value}' to Boolean") | ||
| else -> throw ValidationException("Cannot coerce '${valueNode.value}' to Boolean") |
There was a problem hiding this comment.
P2: Include valueNode when throwing this ValidationException to preserve GraphQL error locations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/builtin/BuiltInScalars.kt, line 164:
<comment>Include `valueNode` when throwing this `ValidationException` to preserve GraphQL error locations.</comment>
<file context>
@@ -154,17 +154,17 @@ object BOOLEAN_COERCION : StringScalarCoercion<Boolean> {
1L -> true
- else -> throw IllegalArgumentException("Cannot coerce '${valueNode.value}' to Boolean")
+ else -> throw ValidationException("Cannot coerce '${valueNode.value}' to Boolean")
}
</file context>
| else -> throw ValidationException("Cannot coerce '${valueNode.value}' to Boolean") | |
| else -> throw ValidationException("Cannot coerce '${valueNode.value}' to Boolean", valueNode) |
| @Test | ||
| fun `query with int variable should not allow floating point numbers that are not whole`() { | ||
| expect<InvalidInputValueException>("Cannot coerce '1.01' to Int") { | ||
| expectExecutionError<InvalidInputValueException>("Cannot coerce '1.01' to Int") { |
There was a problem hiding this comment.
P2: Variable coercion failures are request errors, so these cases should not be asserted as execution errors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/language/VariablesSpecificationTest.kt, line 49:
<comment>Variable coercion failures are request errors, so these cases should not be asserted as execution errors.</comment>
<file context>
@@ -45,7 +46,7 @@ class VariablesSpecificationTest : BaseSchemaTest() {
@Test
fun `query with int variable should not allow floating point numbers that are not whole`() {
- expect<InvalidInputValueException>("Cannot coerce '1.01' to Int") {
+ expectExecutionError<InvalidInputValueException>("Cannot coerce '1.01' to Int") {
testedSchema.executeBlocking(
request = "query(\$rank: Int!) {filmByRank(rank: \$rank) {title}}",
</file context>
| } | ||
| set<JsonNode>("path", objectMapper.valueToTree(error.path)) | ||
| error.extensions?.let { | ||
| set<JsonNode>("extensions", objectMapper.valueToTree(it)) |
There was a problem hiding this comment.
P2: extensions are serialized with valueToTree, which can fail for arbitrary Map<String, Any?> values and break error response serialization. Use the existing defensive conversion path so unknown extension value types are stringified instead of throwing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At kgraphql/src/main/kotlin/com/apurebase/kgraphql/helpers/KGraphQLExtensions.kt, line 145:
<comment>`extensions` are serialized with `valueToTree`, which can fail for arbitrary `Map<String, Any?>` values and break error response serialization. Use the existing defensive conversion path so unknown extension value types are stringified instead of throwing.</comment>
<file context>
@@ -129,3 +130,21 @@ fun JsonNode?.toValueNode(expectedType: __Type): ValueNode = when (this) {
+ }
+ set<JsonNode>("path", objectMapper.valueToTree(error.path))
+ error.extensions?.let {
+ set<JsonNode>("extensions", objectMapper.valueToTree(it))
+ }
+ }
</file context>
| set<JsonNode>("extensions", objectMapper.valueToTree(it)) | |
| set<JsonNode>("extensions", objectMapper.readTree(it.toJsonElement().toString())) |
Adds support for proper error handling and partial responses as defined by the spec, cf. https://spec.graphql.org/September2025/#sec-Errors
KGraphQL now distinguishes between request and execution errors, and the latter now result in a response that contains both,
dataanderrors. Execution errors result in a value ofnullfor nullable fields, and will bubble up to their parent node for non-nullable fields. Additionally, resolvers can now raise execution errors while still returning data.If data and errors are present in the response, the
errorskey is serialized first to make it more apparent that errors are present.Behavior for request errors is unchanged, and
wrapErrorsconfiguration is still supported for now - but likely subject to change in the future.Resolves #114
BREAKING CHANGE: relying on exceptions being thrown from query execution may no longer work as before. It is advised to specify
wrapErrors = falseand report an issue with the respective use case.