"TypeError: Cannot read property 'forEach' of undefined" when embedded object field is a reference

[yoadsn]

Overview of the Issue

Querying a schema field which is a reference and is a sub field of an embedded object fails graphql validation with error:

"errors": [
  "TypeError: Cannot read property 'forEach' of undefined"
]

The relevant parts of the stack trace of the internal graphql execution is:

  TypeError: Cannot read property 'forEach' of undefined

  - ProvidedNonNullArguments.js:61 Object.leave
    [react-starter-kit]/[graphql]/validation/rules/ProvidedNonNullArguments.js:6    1:22

  - visitor.js:312 Object.leave
    [react-starter-kit]/[graphql]/language/visitor.js:312:29

  - visitor.js:351 Object.leave
    [react-starter-kit]/[graphql]/language/visitor.js:351:21

  - visitor.js:227 visit
    [react-starter-kit]/[graphql]/language/visitor.js:227:26

  - validate.js:75 visitUsingRules
    [react-starter-kit]/[graphql]/validation/validate.js:75:22

  - validate.js:60 validate
    [react-starter-kit]/[graphql]/validation/validate.js:60:10

  - graphql.js:54 Promise.catch.error.errors
    [react-starter-kit]/[graphql]/graphql.js:54:51

  - es6.promise.js:193 new Promise
    [react-starter-kit]/[core-js]/modules/es6.promise.js:193:7

  - graphql.js:51 graphql
    [react-starter-kit]/[graphql]/graphql.js:51:10

  - gqlSchema.js:13 Object.<anonymous>
    D:/my-dev-path-here/build/webpack:/data/gqlSchema.js:13:1

Reproduce the Error

Steps to reproduce:

1. Create the following mongoose schema

var parentSchema = Schema({
  name: String,
  child: {
    description: String,
    childInner: { type: Schema.Types.ObjectId, ref: 'Child' }
  }
});

var childSchema = Schema({
  name: String
});

var Parent = mongoose.model('Parent', parentSchema);
var Child = mongoose.model('Child', childSchema);

export default [Parent, Child];

1. Create a graphql schema using graffiti-mongoose

const gqlScehma = getSchema(dbschema);

1. Execute the following query against the result schema

graphql(gqlScehma, `
  {
  parents {
    child {
      childInner {
        name
      }
    }
  }
}`)

Node version: 5.6.0 (but this also happens on older version)
Graffiti-Mongoose version: 5.2.0 (but happened in previous versions as well)

Related Issues

#3 Probably should have written the tests.

Suggest a Fix

The problem is a check done by the graphql runtime in:
src/validation/rules/ProvidedNonNullArguments.js -> ProvidedNonNullArguments()
This, checks that non-null args are supplied. The problem is that it always expects an 'args' field to exist on the fields checked by this method (even if empty)

...
fieldDef.args.forEach(argDef => {
...

For some reason - the embedded sub fields which are reference does not have an 'args' field. I suspect
this has to do with the getType() method in graffiti-mongoose's graffiti-mongoose/src/type/type.js.

There, on line 255 it is seen that the field definition is created without an args field.

See here

but on line 228 which handles the same case for array type of fields - it does.

See here

I tried adding an empty array as the `args' field there - it fixes the problem but fails some of the tests on the schema validation utils from garphql - so I assume that is not a good fix.

I have a PR with a failing test cases I can submit - but that would mean it will be failing until a fix is found. We can merge that to a failing branch if you like and try and analyze it there.

See my branch here for the changes

Thank you for your contribution!

[yoadsn]

I suspect this might be a bug in Graphql.js itself. since in the "type" docs of Graphql the "args" property of a field is optional. From here:

type GraphQLFieldConfig = {
  type: GraphQLOutputType;
  args?: GraphQLFieldConfigArgumentMap;
  resolve?: GraphQLFieldResolveFn;
  deprecationReason?: string;
  description?: ?string;
}

Should I open a bug in graphql?

[tothandras]

@yoadsn looking at it

[tothandras]

feel free to open the PR, I will try to fix the tests

[yoadsn]

After many hours of debugging- I think I have found the problem - deep inside the type.js source code.
I will update later when I have a suggestion for a fix.

[tothandras]

@yoadsn Thank you, I really appreciate!

[yoadsn]

Ok, For future reference here is a brief of the problem and how it was resolved.

Since ever, graphql was memoizing access to the getFields() method of a GraphQLObjectType so that the calculation is stored internally on the _fields property and returned from that field if already calculated.

 getFields(): GraphQLFieldDefinitionMap {
    return this._fields || (this._fields =
      defineFieldMap(this, this._typeConfig.fields)
    );
  }

code here;

Now, graffiti-mongoose generates fields with reference and attaches the correct types to those fields in a big method getTypes() in type.js.
During that process in order to get the fields of a parent field of a reference field it uses the graphql getFields() method unlike how it does it for other types of fields (such as an array of reference fields).

parent = parent[segment].type.getFields();

code here

The parent variable is the internal _fields property of the GraphQLObjectType and a few rows below it is modified (partially overridden) by the correct reference type field prepared beforehand. That pre-made field does not have an empty args property and it overrides the correct value in _fields generated by graphql which did contain the args property.

Later on, when the schema and inside it that specific GraphQLObjectType object is used by graphql it checks for the existence of the args field. (which is a mistake since the spec does not require it - see above).
But since graphql assumed internal properties like _fields are under the library control - it is a reasonable invariant to assert.

The fix was to enumerate the fields using an external method which does not call the internal memoizing leaving the object "clean".

parent = getTypeFields(parent[segment].type);

This is how other fields were populated in other parts of that method.
This bug was most likely caused by this commit