search

graphql / graphql-js

A reference implementation of GraphQL for JavaScript
http://graphql.org/graphql-js/
MIT License
20.05k stars 2.02k forks source link

Add null check to getAllImplementsInterfaceNodes() #2154

Open saerdnaer opened 5 years ago

saerdnaer commented 5 years ago

With some faulty schemas or responses iface in the following section can be null:

https://github.com/graphql/graphql-js/blob/6609d3942e2ce129ff44c3be24eb7917de444583/src/type/validate.js#L596-L603

If fails then with [error] Cannot read property 'name' of null. Adding a null check yields the much more understandable error message Type $typename must only implement Interface types, it cannot implement null.

IvanGoncharov commented 5 years ago

@saerdnaer Thanks for reporting 👍 It's definitely a bug 🐞

IvanGoncharov commented 5 years ago

@saerdnaer Looking at the sources I think we make a mistake allowing invalid values to propagate to schema validate they should be rejected in GraphQLObjectType constructor. As I understand this is not a blocker, right? At the moment, we trying to freeze our codebase since we are working on converting the entire project to TypeScript so can we postpone this issue?

ashutosh1919 commented 4 years ago

@IvanGoncharov and @saerdnaer , As I understand from the issue that we need to add check for iface and we need to raise error. How can we raise error? And how can I pass description. Can you plz guide me? I want to raise PR for this issue.