Custom resolvers killing server

[alizhdanov]

Dependencies

MacOS mojave
node - 10.12.0
graphql-tools - 4.0.3
graphql - 14.0.2"
apollo-server-express - 2.2.6

I have a remote schema which returns not relay compatible ID, I'm trying to write my own resolvers to generate the proper ID with help of mergeSchemas. But apollo server just dying with JavaScript heap out of memory error.

I was able to replicate the issue by using schema stitching basic example where I just trying to return id without modifying it and resolvers never even called.

id: {
  fragment: `... on Chirp { id }`,
  resolve(chirp) {
    // here I should be able to do some modifications
    return chirp.id;
  },
},

Intended outcome: It'd return id or throw some error if something done incorect
Actual outcome: Server dying with JavaScript heap out of memory error
How to reproduce the issue: clone repo and try to run this query:

{
  chirp {
    id
  }
}

P.S.: Probably there are bettwer aproaches to do it. But, anyways I think heap out of memory is not expected result :-)

[kommander]

This is the same issue with mergeSchemas trying to be smart, related to #1033, #1031, #1035. Have a look at this https://github.com/apollographql/graphql-tools/blob/master/src/stitching/mergeSchemas.ts#L446. Replace L446 like so:

          [resolverKey]: fields[fieldName][resolverKey],
          // [resolverKey]: createDelegatingResolver(
          //   schema,
          //   operationName,
          //   fieldName,
          // ),

No memory issues and no timeouts. A Cannot return null for non-nullable field Chirp.id. though, but I guess that's because of the reduced repro?

[yaacovCR]

The problem here is that you are replacing the id field with an id fragment that contains an id field, which causes an infinite loop. This could be fixed by replacing as one leaves a node rather than as one enters, but not sure if worth it, as fragments should not contain the containing field. Perhaps they should be validated as such, but probably easier just to allow without recursion. Rolling into #1306

[yaacovCR]

Moving to v5.1, reopening to move any relevant discussion here.

[yaacovCR]

We have moved to selectionSet hints rather than fragment hints, so we won't be pursuing a fix here, but PRs with one by the community will still be accepted if anyone has time to submit.