refactor!: move error handler to schema#611
Merged
Merged
Conversation
stuebingerb
force-pushed
the
refactor/move-error-handler-to-schema
branch
from
3099697 to
969f4cb
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #611 +/- ##
==========================================
+ Coverage 84.10% 84.13% +0.03%
==========================================
Files 151 152 +1
Lines 5000 5006 +6
Branches 860 861 +1
==========================================
+ Hits 4205 4212 +7
Misses 495 495
+ Partials 300 299 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bencher Report![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 16 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/configuration.md">
<violation number="1" location="docs/content/Reference/configuration.md:14">
P3: The `errorHandler` documentation link points to a non-existent source path (`com/stuebingerb`) and should use the actual `com/apurebase` path.</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.
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR refactors exception handling in KGraphQL by introducing a schema-level, pluggable ChangesError Handler Integration and Ktor Plugin Refactoring
Sequence DiagramsequenceDiagram
participant User as User Code
participant Schema as SchemaConfiguration
participant Executor as ParallelRequestExecutor
participant Handler as ErrorHandler
participant Resp as Response
User->>Schema: configure(errorHandler = CustomHandler)
Schema->>Schema: store errorHandler instance
User->>Executor: execute query
Executor->>Executor: field resolution error occurs
Executor->>Handler: handleException(ctx, node, exception)
alt Custom mapping matches
Handler->>Handler: map to ExecutionError or RequestError
else No match
Handler->>Handler: call super (default mapping)
end
Handler-->>Executor: GraphQLError subclass
alt RequestError
Executor->>Resp: rethrow (halts request)
else ExecutionError + nullable field
Executor->>Resp: raise error, return null for field
else ExecutionError + non-nullable field
Executor->>Resp: propagate error upward
end
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/Reference/configuration.md`:
- Around line 12-14: Update the docs to show the correct default and link: add
the default value `true` for the wrapErrors entry to reflect
SchemaConfigurationDSL.wrapErrors being initialized to true, and fix the
ErrorHandler reference URL to point to the current package path (replace
com/stuebingerb/.../ErrorHandler.kt with com/apurebase/.../ErrorHandler.kt) so
the link resolves to the actual ErrorHandler source.
In
`@kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ParallelRequestExecutor.kt`:
- Around line 477-482: Call the schema errorHandler before deciding to rethrow
so custom handlers can remap exceptions even when
schema.configuration.wrapErrors is false: do not early-return when
!schema.configuration.wrapErrors; instead, first check for CancellationException
and skip handling for that case, otherwise invoke
schema.configuration.errorHandler.handleException(ctx.requestContext, node,
exception), then if that returns a RequestError throw it; finally, if wrapErrors
is false rethrow the original exception, else continue with the existing
wrapping logic.
🪄 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: f80ca7ab-d1e9-4a12-a707-a482238631b2
📒 Files selected for processing (16)
docs/content/Plugins/ktor.mddocs/content/Reference/configuration.mddocs/content/Reference/errorHandling.mdkgraphql-ktor-stitched/api/kgraphql-ktor-stitched.apikgraphql-ktor-stitched/src/main/kotlin/com/apurebase/kgraphql/stitched/schema/configuration/StitchedSchemaConfiguration.ktkgraphql-ktor-stitched/src/main/kotlin/com/apurebase/kgraphql/stitched/schema/dsl/StitchedSchemaConfigurationDSL.ktkgraphql-ktor/api/kgraphql-ktor.apikgraphql-ktor/src/main/kotlin/com/apurebase/kgraphql/KtorFeature.ktkgraphql-ktor/src/test/kotlin/com/apurebase/kgraphql/KtorFeatureTest.ktkgraphql-ktor/src/test/kotlin/com/apurebase/kgraphql/KtorTest.ktkgraphql/api/kgraphql.apikgraphql/src/main/kotlin/com/apurebase/kgraphql/configuration/SchemaConfiguration.ktkgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/dsl/SchemaConfigurationDSL.ktkgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ErrorHandler.ktkgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ParallelRequestExecutor.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/integration/QueryTest.kt
💤 Files with no reviewable changes (3)
- kgraphql-ktor/api/kgraphql-ktor.api
- kgraphql-ktor/src/test/kotlin/com/apurebase/kgraphql/KtorFeatureTest.kt
- kgraphql-ktor/src/test/kotlin/com/apurebase/kgraphql/KtorTest.kt
There was a problem hiding this comment.
No issues found across 16 files
Architecture diagram
sequenceDiagram
participant Client as Client (HTTP)
participant Ktor as Ktor Plugin
participant Exec as ParallelRequestExecutor
participant Handler as NEW: ErrorHandler (Schema)
participant Resolver as Resolver (User Code)
Note over Client,Resolver: GraphQL Execution Flow with Error Handling
Client->>Ktor: POST /graphql
Ktor->>Exec: execute(query)
Exec->>Resolver: Invoke resolver
alt Success Path
Resolver-->>Exec: Data
Exec-->>Ktor: Execution Result
Ktor-->>Client: 200 OK (JSON Data)
else Resolver throws Exception
Resolver-->>Exec: throw Throwable
Exec->>Handler: NEW: handleException(ctx, node, exception)
Note right of Handler: Maps Throwable to GraphQLError
Handler-->>Exec: Return GraphQLError (e.g. ExecutionError)
alt Field is Nullable
Exec->>Exec: Add error to result context
Exec-->>Ktor: Result (Data + Errors)
Ktor-->>Client: 200 OK (Partial JSON)
else Field is Non-Nullable
Exec-->>Ktor: CHANGED: throw GraphQLError
Ktor->>Ktor: Catch GraphQLError
Ktor-->>Client: 200 OK (JSON Errors only)
end
end
Note over Ktor,Handler: BREAKING: Error mapping moved from Ktor Plugin to Schema Config
stuebingerb
force-pushed
the
feature/support-partial-responses
branch
3 times, most recently
from
f71014d to
bdf5538
Compare
Moves the error handler from the Ktor plugin to the schema itself. The error handler can now be used to map any exception encountered during execution and delegate to the default implementation. BREAKING CHANGE: error handler is no longer supported as part of Ktor plugin configuration
stuebingerb
force-pushed
the
refactor/move-error-handler-to-schema
branch
from
969f4cb to
e3c9ee1
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
kgraphql/src/test/kotlin/com/apurebase/kgraphql/integration/QueryTest.kt (1)
1211-1235: ⚡ Quick winAdd an assertion for the
super.handleException(...)fallback path.Line 1211 delegates to default handling, but this path is currently untested. Add a third query that throws an unmapped exception and assert default error output.
Proposed test extension
val schema = KGraphQL.schema { configure { errorHandler = customErrorHandler } query("executionError") { resolver<String?> { throw IllegalArgumentException("Illegal argument") } } query("requestError") { resolver<String?> { throw IllegalAccessException() } } + query("defaultError") { + resolver<String?> { throw RuntimeException("Boom") } + } } @@ schema.executeBlocking("{ requestError }") shouldBe """ {"errors":[{"message":"You shall not pass!","locations":[{"line":1,"column":3}],"extensions":{"required_role":"ADMIN","reason":"Gandalf"}}]} """.trimIndent() + + schema.executeBlocking("{ defaultError }") shouldBe """ + {"errors":[{"message":"Boom","locations":[{"line":1,"column":3}],"path":["defaultError"],"extensions":{"type":"INTERNAL_SERVER_ERROR"}}],"data":{"defaultError":null}} + """.trimIndent() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@kgraphql/src/test/kotlin/com/apurebase/kgraphql/integration/QueryTest.kt` around lines 1211 - 1235, Add a test for the fallback path of customErrorHandler by adding a third query in the same schema (e.g., query "unmappedError" with a resolver that throws an unmapped exception like IllegalStateException) and assert that schema.executeBlocking("{ unmappedError }") returns the default error shape produced by super.handleException (including standard "message", "locations" and "path" fields and no custom "extensions"). Locate the customErrorHandler and its handleException override in the test and extend the schema block and assertions accordingly so the fallback path is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@kgraphql/src/test/kotlin/com/apurebase/kgraphql/integration/QueryTest.kt`:
- Around line 1211-1235: Add a test for the fallback path of customErrorHandler
by adding a third query in the same schema (e.g., query "unmappedError" with a
resolver that throws an unmapped exception like IllegalStateException) and
assert that schema.executeBlocking("{ unmappedError }") returns the default
error shape produced by super.handleException (including standard "message",
"locations" and "path" fields and no custom "extensions"). Locate the
customErrorHandler and its handleException override in the test and extend the
schema block and assertions accordingly so the fallback path is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 65545a8a-a026-4cec-bb57-c24cd4fd8852
📒 Files selected for processing (16)
docs/content/Plugins/ktor.mddocs/content/Reference/configuration.mddocs/content/Reference/errorHandling.mdkgraphql-ktor-stitched/api/kgraphql-ktor-stitched.apikgraphql-ktor-stitched/src/main/kotlin/com/apurebase/kgraphql/stitched/schema/configuration/StitchedSchemaConfiguration.ktkgraphql-ktor-stitched/src/main/kotlin/com/apurebase/kgraphql/stitched/schema/dsl/StitchedSchemaConfigurationDSL.ktkgraphql-ktor/api/kgraphql-ktor.apikgraphql-ktor/src/main/kotlin/com/apurebase/kgraphql/KtorFeature.ktkgraphql-ktor/src/test/kotlin/com/apurebase/kgraphql/KtorFeatureTest.ktkgraphql-ktor/src/test/kotlin/com/apurebase/kgraphql/KtorTest.ktkgraphql/api/kgraphql.apikgraphql/src/main/kotlin/com/apurebase/kgraphql/configuration/SchemaConfiguration.ktkgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/dsl/SchemaConfigurationDSL.ktkgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ErrorHandler.ktkgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ParallelRequestExecutor.ktkgraphql/src/test/kotlin/com/apurebase/kgraphql/integration/QueryTest.kt
💤 Files with no reviewable changes (3)
- kgraphql-ktor/src/test/kotlin/com/apurebase/kgraphql/KtorTest.kt
- kgraphql-ktor/api/kgraphql-ktor.api
- kgraphql-ktor/src/test/kotlin/com/apurebase/kgraphql/KtorFeatureTest.kt
✅ Files skipped from review due to trivial changes (1)
- kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ErrorHandler.kt
🚧 Files skipped from review as they are similar to previous changes (7)
- kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/dsl/SchemaConfigurationDSL.kt
- kgraphql-ktor-stitched/src/main/kotlin/com/apurebase/kgraphql/stitched/schema/dsl/StitchedSchemaConfigurationDSL.kt
- kgraphql/src/main/kotlin/com/apurebase/kgraphql/configuration/SchemaConfiguration.kt
- kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ParallelRequestExecutor.kt
- docs/content/Reference/errorHandling.md
- kgraphql/api/kgraphql.api
- kgraphql-ktor/src/main/kotlin/com/apurebase/kgraphql/KtorFeature.kt
There was a problem hiding this comment.
No issues found across 16 files
Architecture diagram
sequenceDiagram
participant Client
participant Ktor as Ktor Plugin
participant Schema as Schema
participant Config as Schema Configuration
participant Executor as ParallelRequestExecutor
participant ErrorHandler as ErrorHandler
participant Resolver as Resolver
Note over Client,Resolver: NEW: Error handling moved from Ktor plugin to Schema
Client->>Ktor: POST /graphql
Ktor->>Schema: executeQuery(query, context)
Schema->>Executor: execute(request)
Executor->>Resolver: resolveField()
alt Exception thrown by resolver
Resolver-->>Executor: Throwable
Executor->>Executor: handleException(ctx, node, type, throwable)
alt wrapErrors = false OR CancellationException
Executor->>Executor: throw exception (no handling)
else NEW: Call error handler on Schema
Executor->>ErrorHandler: handleException(ctx, node, exception)
alt Result is RequestError
ErrorHandler-->>Executor: RequestError
Executor->>Executor: throw RequestError
else Result is ExecutionError AND field nullable
ErrorHandler-->>Executor: ExecutionError
Executor->>Executor: raiseError + return null node
else Result is ExecutionError AND field non-nullable
ErrorHandler-->>Executor: ExecutionError
Executor->>Executor: throw ExecutionError
end
end
end
alt Exception propagated to Ktor
Ktor->>Ktor: catch GraphQLError
Ktor->>Client: 200 + serialized error
else Normal execution
Schema-->>Ktor: result
Ktor->>Client: 200 + response
end
Note over ErrorHandler: Default: GraphQLError -> pass through<br/>Other -> wrap as ExecutionError
Note over Config: Configurable via schema configuration<br/>errorHandler property
There was a problem hiding this comment.
1 issue found across 16 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:227">
P2: The new guidance is internally inconsistent: it tells users to re-throw mapped exceptions, but the documented error-handler API returns mapped `GraphQLError`s. This should instruct returning mapped errors instead of re-throwing exceptions.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client
participant Ktor as Ktor Plugin
participant Schema as Schema
participant Config as Schema Configuration
participant Executor as ParallelRequestExecutor
participant ErrorHandler as ErrorHandler
participant Resolver as Resolver
Note over Client,Resolver: NEW: Error handling moved from Ktor plugin to Schema
Client->>Ktor: POST /graphql
Ktor->>Schema: executeQuery(query, context)
Schema->>Executor: execute(request)
Executor->>Resolver: resolveField()
alt Exception thrown by resolver
Resolver-->>Executor: Throwable
Executor->>Executor: handleException(ctx, node, type, throwable)
alt wrapErrors = false OR CancellationException
Executor->>Executor: throw exception (no handling)
else NEW: Call error handler on Schema
Executor->>ErrorHandler: handleException(ctx, node, exception)
alt Result is RequestError
ErrorHandler-->>Executor: RequestError
Executor->>Executor: throw RequestError
else Result is ExecutionError AND field nullable
ErrorHandler-->>Executor: ExecutionError
Executor->>Executor: raiseError + return null node
else Result is ExecutionError AND field non-nullable
ErrorHandler-->>Executor: ExecutionError
Executor->>Executor: throw ExecutionError
end
end
end
alt Exception propagated to Ktor
Ktor->>Ktor: catch GraphQLError
Ktor->>Client: 200 + serialized error
else Normal execution
Schema-->>Ktor: result
Ktor->>Client: 200 + response
end
Note over ErrorHandler: Default: GraphQLError -> pass through<br/>Other -> wrap as ExecutionError
Note over Config: Configurable via schema configuration<br/>errorHandler property
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| Because thrown exceptions are re-thrown, `wrapErrors = false` will not produce partial responses from thrown exceptions, | ||
| but resolvers can still return partial responses by calling `Context.raiseError`. `wrapErrors = false` will also not | ||
| invoke a custom error handler. If you want to throw exceptions with custom mapping, use `wrapErrors = true` and re-throw | ||
| mapped exceptions from the error handler. |
There was a problem hiding this comment.
P2: The new guidance is internally inconsistent: it tells users to re-throw mapped exceptions, but the documented error-handler API returns mapped GraphQLErrors. This should instruct returning mapped errors instead of re-throwing exceptions.
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 227:
<comment>The new guidance is internally inconsistent: it tells users to re-throw mapped exceptions, but the documented error-handler API returns mapped `GraphQLError`s. This should instruct returning mapped errors instead of re-throwing exceptions.</comment>
<file context>
@@ -221,4 +221,42 @@ Those re-thrown exceptions could then be handled with the [`StatusPages` Ktor pl
+Because thrown exceptions are re-thrown, `wrapErrors = false` will not produce partial responses from thrown exceptions,
+but resolvers can still return partial responses by calling `Context.raiseError`. `wrapErrors = false` will also not
+invoke a custom error handler. If you want to throw exceptions with custom mapping, use `wrapErrors = true` and re-throw
+mapped exceptions from the error handler.
+
+## Error Handler
</file context>
mervyn-mccreight
approved these changes
May 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Moves the error handler from the Ktor plugin to the schema itself. The error handler can now be used to map any exception encountered during execution and delegate to the default implementation.
BREAKING CHANGE: error handler is no longer supported as part of Ktor plugin configuration