-
Notifications
You must be signed in to change notification settings - Fork 93
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Batch exception fix when unwrapping exception #619
Conversation
Signed-off-by:Phillip Kruger <[email protected]>
Signed-off-by:Phillip Kruger <[email protected]>
@t1 - added a test and now also a fix ! I'll do a back port for 1.0.x |
@t1 this PR is waiting for you .... please review :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With some minor remarks, LGTM, but I don't understand a lot of the context (Uni, etc.)
@@ -48,10 +43,6 @@ public AbstractDataFetcher(Operation operation, Config config) { | |||
this.argumentHelper = new ArgumentHelper(operation.getArguments()); | |||
this.partialResultHelper = new PartialResultHelper(); | |||
this.batchLoaderHelper = new BatchLoaderHelper(); | |||
if (config != null && config.getUnwrapExceptions().isPresent()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would do this with an ifPresent
block, but that's just a matter of taste, I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is being deleted :)
server/implementation/src/main/java/io/smallrye/graphql/execution/error/ExceptionHandler.java
Show resolved
Hide resolved
server/tck/src/test/java/io/smallrye/graphql/test/apps/batch/api/BatchApi.java
Outdated
Show resolved
Hide resolved
…pi/BatchApi.java Co-authored-by: Rüdiger zu Dohna <[email protected]>
Fix #590
Signed-off-by:Phillip Kruger [email protected]