Skip to content

#268 - now follows spec in regard to null error handling#353

Merged
bbakerman merged 3 commits into
graphql-java:masterfrom
bbakerman:268-nonnull-error-handling
Apr 16, 2017
Merged

#268 - now follows spec in regard to null error handling#353
bbakerman merged 3 commits into
graphql-java:masterfrom
bbakerman:268-nonnull-error-handling

Conversation

@bbakerman

Copy link
Copy Markdown
Member

#268 has problems with error handling re: http://facebook.github.io/graphql/#sec-Errors

This API breaking change fixes that. It now follows the rules on non null ness

The key change is a new "TypeInfo" object to track parent child types and their null ness.

The current type system cant do this (and probably should not).

The rules on making parent objects null have been followed

@bbakerman bbakerman self-assigned this Apr 3, 2017
…-handling

# Conflicts:
#	src/main/java/graphql/ErrorType.java
#	src/main/java/graphql/execution/Execution.java
#	src/test/groovy/graphql/GraphQLTest.groovy
@andimarek

Copy link
Copy Markdown
Member

@bbakerman Could you update you PR? Thanks!

…-handling

# Conflicts:
#	src/main/java/graphql/execution/ExecutionStrategy.java
#	src/main/java/graphql/execution/ExecutorServiceExecutionStrategy.java
#	src/main/java/graphql/execution/SimpleExecutionStrategy.java
#	src/main/java/graphql/execution/batched/BatchedExecutionStrategy.java
@bbakerman

Copy link
Copy Markdown
Member Author

base updated. Can I get a tick for this before I merge so the other dependent PRs can merege please namely #369

@andimarek andimarek added this to the 3.0.0 milestone Apr 11, 2017

@andimarek andimarek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job! :-)

I am not totally happy about using Exceptions in this way to be honest ... did you thought about using a special ExecutionResult?

return new Builder();
}

private static GraphQLType unwrap(GraphQLType type) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also implemented in NoUnbrokenInputCycles .. .maybe we can make it dry?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants