Relay: [modern] Relay Compiler error: RangeError: Maximum call stack size exceeded

[dmbrowne]

I previously wrote a comment in the thread #1660 about an error I am receiving when trying to build with the relay compiler. I have managed to work out why I am receiving an error, and I think it may be a bug with the compiler.

Take the following schema which has been extended / modified from the todo-modern relay example (the + is what I have added to the todo-modern schema for demo purposes):

+ input AddUserInput {
+   username: String!
+   password: String!
+   todo: AddTodoInput
+ }

  input AddTodoInput {
    text: String!
+   user: AddUserInput
    clientMutationId: String
  }

+ type AddUserPayload {
+   changedUser: User
+   clientMutationId: String
+ }

  type Mutation {
+   addUser(input: AddUserInput!): AddUserPayload
    changeTodoStatus(input: ChangeTodoStatusInput!): ChangeTodoStatusPayload
    markAllTodos(input: MarkAllTodosInput!): MarkAllTodosPayload
    removeCompletedTodos(input: RemoveCompletedTodosInput!): RemoveCompletedTodosPayload
    removeTodo(input: RemoveTodoInput!): RemoveTodoPayload
    renameTodo(input: RenameTodoInput!): RenameTodoPayload
  }

Notice the dependancy of AddTodoInput on type AddUserInput, but also the dependency of AddUserInput on type AddTodoInput.

When I run the relay compiler, I get the error RangeError: Maximum call stack size exceeded and the compiler immediately quits its operation and does not write any __generated__ queries or mutations.

[mjmahone]

Interesting. I don't know if I've ever thought about recursive input types before, but they should work (i.e. it's a bug that they don't)! That would be a really good test case to add in to the compiler tests, but I'm curious where the error is actually happening (which module).

[dmbrowne]

I'm using scaphold.io for my graphql server, so unfortunately I'm not in control of the schema generation, so avoiding this error is impossible. My work around for now is to manually delete the recursive inputs before trying to compile.

[dmbrowne]

@mjmahone i'm not sure I understand your question, or was it rhetorical?

[josephsavona]

@dmbrowne If possible, the easiest way to move forward would be a PR that adds a failing test case. That would make the issue very clear and easy to diagnose (and make it clear exactly where the error occurs).

[alloy]

While working on adding custom Node ID support, I also ran into this exception. I’m not sure yet if our schema with custom Node ID lead to triggering this issue, so I can’t say that this fix is definitively correct, but FWIW this fixed the issue for me: https://github.com/alloy/relay/pull/1/commits/2e56d9c4a25ce857aedf0c9c9d63f816bca580e4

[alloy]

@josephsavona If, before I finish my migration work, you think that fix is reasonable in any case, let me know and I’ll PR that separately.

[marktani]

Here's a minimum example that reproduces the error: https://github.com/kguller/W3.harness.

You can run

yarn run schema
yarn run relay 

to receive

Writing default
Error writing modules:
RangeError: Maximum call stack size exceeded
Unchanged: 0 files
Written default in 0.37s
Done in 1.79s.

The working theory is that the recursive input type TesttestReferencesTestReference (definition) causes the stack size to exceed. It's used as an argument here for example.

As @mjmahone, my understanding is that this should be supported by the Relay Compiler.

[schickling]

I've went ahead and reduced the given reproduction to the minimum. Please see the following repository: https://github.com/schickling/relay-compiler-bug 🐛

I can confirm that the problem seems to be recursive input types like in this example:

input CreateTestInput {
  testField: String!
  test: CreateTestInput
  clientMutationId: String!
}

@josephsavona @alloy @leebyron any thoughts on this?

P.S. I wanted to create a PR with a failing test case as @josephsavona suggested, however the testing setup seems to be a bit overwhelming so I created this minimal repo instead.

[alloy]

@schickling @marktani Did my patch fix your specific cases too?

[schickling]

@alloy any instructions how I can easily try it out? Maybe it might be the quickest to quickly clone this test repo and run yarn relay.

[alloy]

Ok tried that and it’s definitely not the same bug, as my patch was for babel-plugin-relay which is only needed for classic/compat mode.

I’ll be doing some work to clean up some patches I have for Relay soon, if time permits I’ll see if I can easily figure out this issue, but no promises.

[serverlesspolska]

Unfortunately I hit the same problem as well

mutation createCompanyAssignmentMutation($input: CreateCompanyAssignmentInput!) {
createCompanyAssignment(input: $input) {
companyAssignment {
role
company {
id
companyId
companyName
}
user {
id
firstname
lastname
email
createdBy {
id
}
}
}
}
}

[phehaz]

+1, Experiencing this issue aswell - mine is related to the same issue as @marktani has described. 😕

[kguller]

+1 as well. I reported this issue over to @marktani with GraphCool who helped verify the problem and get it reported. Thank you for your help!

Just wanted to add that we tried to flatten out the recursive structure yesterday to work around the compiler error without success. Our team confirmed with GraphCool that the complexity multiplies as you attempt to make changes to the schema file, so it is quite a challenge. If anyone has had any success with a temporary work around, we'd love to hear about it. Unfortunately, our production project is at a standstill, so we are looking for any alternatives at this point.

[jhalborg]

+1 here, too - it's unfortunate, as we can't move on like @kguller mentions.

[serverlesspolska]

@kguller I'm using graph.cool as well, seems like a lot of people have same problem on that platform. Coincidence?

The real problem is that I'm really blocked with my work and there is no workaround solution :-(

[kguller]

@panzupa, definitely a related issue, it's an impact to anyone who needs to alter their schema. The graph.cool platform is fantastic, we are dying to get back to using it. This compiler error is a major showstopper since we've invested fully into react relay + graphql on graph.cool. We just rolled out to production and had our first patch planned last week. Unfortunately, we still have not figured a work around, so we are unable to service our client requests. We are at the mercy of this compile fix as the option to abandon a production system and rewrite a 6 month project isn't really sound. It doesn't seem like it's an easy or small fix either, so we're getting a bit worried about recovering from this as there's not a lot of insight or feedback yet from the relay team.

[alloy]

It doesn't seem like it's an easy or small fix either

@kguller Does that mean you tried to find and fix the cause? If so, please share any datapoint you have, e.g. where you think the issue in the code lies, what you tried and failed and why, etc. Any datapoint shared will make it easier for the next person (including the Relay team) to spend some of their scarce time on fixing this.

[jhalborg]

FYI, our compilation of the schema seems to work in spite of the warning/error from the compiler - the app can run anyway.

But as @panzupa mentions, it is worth noting that most people on this issue is using Graph.cool, and the issue has arisen since a compilation change they did on their backend system, although intentional according the the graph.cool team. I don't know if @marktani or anyone on your team might have some insight into specifically what changed in your way of generating the schema when this error started showing? If any is needed outside of the reproduction gist he already linked, @alloy ?

[alloy]

@jhalborg Sorry, I’m not asking for any specifics nor is my remark particularly tied to this issue, I’ll clarify more.

In the spirit of collaboration, I’m just mentioning that it would be beneficial if people would do brain dumps of their forays into trying to fix this, as it would be unfortunate if people have to spend time to investigate things that others have already done before them.

[kguller]

@jhalborg: Just to clarify what we are experiencing since you may still be effected, and I hope you aren't impacted further. I just want to note first off to avoid deleting any of your existing generated graph.ql files, in our scenario, mutation files with recursive arguments are never generated.

The compilation error we experience actually fails to create graphql files for mutations that have recursive arguments. If you already have working graph ql files generated for these mutations then you won't be effected as the compiler won't delete them, even if you have auto clean up set. But if you wipe out your generated graph.ql mutations that have recursive arguments, they will not be recreated. The compiler skips the mutations with recursive arguments and continues to try to process the remaining mutations. It seems to output the stack size error for just the mutations with recursive arguments. Our mutations didn't fail either, it was a deceiving warning because it looked like everything was just fine and we still had a full set of generated files. Once we needed to make an actual change to mutations that had recursive arguments, we realized the error as it didn't generate an updated file. If you are seeing mutation files actually being generated that have proper recursive arguments, then we have a difference in results, and it would be great to see your scenario further. I'm posting a detail of our failed work arounds in another comment as @alloy suggested, that should help explain the recursive argument problem.

[kguller]

@alloy We've been working on work arounds the past week and haven't had anything conclusive until yesterday. I wasn't able to identify if in fact the recursive inputs were the problem until this last change worked by removing a reference back to the recursive input 2-3 recursive steps down. I worked out for my teams app that 3 levels of recursive calls would be deep enough to support our routines. @marktani helped us realize that if we could halt the recursive pointers down the chain, we might get it to compile. I have to note that the work around below is not thoroughly tested or valid for most systems, it's just to prove the concept.

The work around with marginal success is as follows:

Work Around: Alter Schema File 2 Levels Down
Step 1) Consider an example mutation definition that includes the recursive input "testReferences" (input definition: TesttestReferencesTestReference).

input CreateTest {
testField: String!
testReferencesIds: [ID!]
testReferences: [TesttestReferencesTestReference!]
}

Step 2) The input definition TesttestReferencesTestReference is where the recursive loop begins. It includes a field named "tests" that is an input type of TestReferencetestsTest. The input definition TestReferencetestsTest has a field named "testReferences" that points back to TesttestReferencesTestReference input type and this is where we believe the recursion goes into an infinite loop.

input TesttestReferencesTestReference {
testsIds: [ID!]
tests: [TestReferencetestsTest!]
}
input TestReferencetestsTest {
testField: String!
testReferencesIds: [ID!]
testReferences: [TesttestReferencesTestReference!] <-- POINTS BACK TO CREATE THE LOOP
}

Step 3) We tried to flatten this out to prevent looping by going down 2-3 levels of recursion pointers and removing the reference as follows (these are in order of readability and not compilation order):

input CreateTest {
testField: String!
testReferencesIds: [ID!]
testReferences: [TesttestReferencesTestReference!]
}
input TesttestReferencesTestReference {
testsIds: [ID!]
tests: [TestReferencetestsTest!]
}
input TestReferencetestsTest {
testField: String!
testReferencesIds: [ID!]
testReferences: [TesttestReferencesTestReferenceEnd!] <-- POINTS TO A NEW TYPE
}
input TesttestReferencesTestReferenceEnd {
testsIds: [ID!]
tests: [TestReferencetestsTestEnd!]
}
input TestReferencetestsTestEnd {
testField: String!
testReferencesIds: [ID!]
// REMOVED REFERENCE POINT
}

Summary:
This ends the recursive call and avoids any stack error. The mutation looks like it works, but I need to do more work to make sure there aren't other status issues with the execution of these mutations. I think our previous attempts went too far down the stack and still ran into trouble. This scenario seemed to have worked and is based off the test environment provided by @schickling.

I just hope that this input opens up a path to address a need for a change in the compiler to only support recursive calls down to a particular level (and possibly allow a parameter to override the default and set a new level). The compiler would then stop traversing calls and create an end input/type of some sort like we did in the example below that does not have a recursive call and ends the chain. Again, a lot of assumptions and lack of insight into the compiler on my part. The work around above is highly error prone and messing with the schema of course is impractical on larger systems. Vendors could support locking out their schema recursive call levels at a certain number, so I guess either side could manage this, compiler vs. schema provider. Hope this is of help.

[benjaminogles]

The recursion that is exceeding the call stack seems to be here https://github.com/facebook/relay/blob/b0fe7f37c06745dfc1f32f17ad5242754acc9209/packages/relay-compiler/core/RelayFlowGenerator.js#L452 I haven't got around to making a failing test case yet for the RelayFlowGenerator, but my first impression is that there is going to be some way to detect that the generator has made a full circle in generating variable types when it encounters recursive inputs.

As a side note, I am also using graphcool and I have ironed out a few recursive definitions in my schema that only arose because of the lack of support of interfaces. (e.g. when I intend a file to be related to one and only one of three types but the graphcool generated schema assumes I might need a reference to all three types in every input which creates the recursive definition) Nonetheless, it seems at first glance that the compiler should be able to handle this case for when it is needed

[benjaminogles]

I was able to get the compiler to finish without exceeding the call stack by passing in a list of parent types when generating the mutation variable types and filtering parents out of the recursive map. But it doesn't look like an ideal solution when multiple types are all referencing each other because you end up with a mutation where at different levels, depending on the type's context, nested mutations are suddenly disallowed and the generated mutation definition is huge.

I guess I will try and make a good failing test case for the RelayFlowGenerator and then see what other solutions can be thought up. I think it would be great if the compiler could mark a mutation input as recursive at a certain point and then allow any depth of nested mutation as long as it repeated the recursive definition but it might be better to only generate the mutation to a predefined depth like @kguller suggested

[kguller]

I've attempted a number of different schema scripts and workarounds by stripping out recursive types from the schema and adding directly to relationship connections, as well as trying to end chains like my earlier post but more automation around it. Unfortunately, they are all too cumbersome to maintain since any number of changes can impact the exported schema. We can't realistically use these workarounds for production and keep pace, so marking the mutation input as recursive and identifying when the recursive loop repeats would be ideal as @benjaminogles suggested. Fixed level like I was suggesting before may be error prone, as I'm dealing with these real world configurations, I think we'd be switching levels exposing ourselves to more headaches.

[benjaminogles]

In case anyone in this thread is interested, there is a configuration option for the RelayFileWriter called inputFieldWhiteListForFlow. Any field names in this list will not have flow types generated for them (this is the part of the compiler that is crashing as it tries to generate flow types for objects that have recursive definitions in the schema). So white listing the field names that have recursive definitions solves the problem but results in less specific flow types.

I modified my local version of the compiler to first compute the strongly connected components in all mutation input definitions and add all the field names that appeared in a strongly connected component to the white list. Now the flow type is generated as specifically as possible, excluding those types that point back to themselves.

I am going to include this fix in my pull request with the failed test case and see if it is an acceptable solution because I am not entirely sure of the original intended use of the white list and I don't think it is available as an external option to the compiler so there isn't a comparable work around.

[jhalborg]

@benjaminogles Sounds reasonable.

Do you perhaps have some sample code for how you did it? Just tried adding the following whitelist, but it didn't seem to work - and I'm not sure if it's because I did it wrong, or because the fields I whitelisted are wrong/not all of them:

function RelayFileWriter(options) {
    (0, _classCallCheck3['default'])(this, RelayFileWriter);
    var config = options.config,
      onlyValidate = options.onlyValidate,
      baseDocuments = options.baseDocuments,
      documents = options.documents,
      schema = options.schema;

    this._baseDocuments = baseDocuments || ImmutableMap();
    this._baseSchema = schema;
    this._config = config;
    this._documents = documents;
    this._onlyValidate = onlyValidate;

    this._config.inputFieldWhiteListForFlow = [
      ...config.inputFieldWhiteListForFlow,
      'journalEntries',
      'user',
    ];
  }

Thanks :)

[benjaminogles]

@jhalborg Two things you may want to make sure of before trying my code is that you only apply the array spread to config.inputFieldWhiteListForFlow if it is defined and that you are modifying the correct file. I don't think you will see any changes unless you modify the bundled file relay-compiler directly or if you modify the source code RelayFileWriter.js and rebuild the project.

You can see exactly what I did in the pull request https://github.com/facebook/relay/pull/2080 just click on "Files Changed". It is a module that exports a function that finds groups of recursive field names in input types and then I use that in RelayFlowGenerator.js to add to the white list.

[chrish-21]

@marktani @schickling I do believe this problem does indeed come from changes made on the Graphcool backend. At least Graphcool used to generate a different schema file based on the same project.graphcool file.

I have an old project where relay-compiler has no problems with the schema.graphql file. However when I now run get-graphql-schema again (without having changed anything on the graphcool project), I get a different schema file generated and relay-compiler runs into the error mentioned above.

I created a repository (https://github.com/chrish-21/graphcool_maximum_call_stack_size_exceeded_error_schemas) with just the schema file that was generated before the graphcool changes (schemaOldWorking.graphql) and the schema that is generated now after the changes (schemaNewNotWorking.graphql) And here is a commit that shows the differences:

https://github.com/chrish-21/graphcool_maximum_call_stack_size_exceeded_error_schemas/commit/4f206d3fac6cad874090b0c2bdf5fb2c9b72f8c8?diff=split

Unfortunately the schema is quiet complex, but I hope it can help in finding the bug.

[marktani]

Hey @chrish-21, it's correct that changes in the way the Relay API is generated on Graphcool uncovered this problem.

However, the fact that the Relay compiler is not handling recursive input types correctly is still unexpected and should be considered a bug.

[chrish-21]

@marktani Ah alright got it, thx for the info!

[jhalborg]

@leebyron - I see you've added the 'Help Wanted' label. In spite of my best intentions, I'm pretty sure this bug transcends my fairly limited Relay knowledge. Do you have any plans internally to look at it, or it it entirely up to the community to fix it?

Also on a sidenote, good job on the licensing across the board :)

[patrick-samy]

@marktani I'm getting the same compiler error with Graphcool's generated schema for Relay: schema.graphql

Is there a workaround?

[benjaminogles]

@patrick-samy you either have to manually remove the unneeded recursive definitions from your schema temporarily or you can follow the work around I am using that I mention here in the PR https://github.com/facebook/relay/pull/2142#issuecomment-345327113

[patrick-samy]

@benjaminogles - Thanks, the workaround works like a charm! You saved me a painful weekend of work.

[jhalborg]

@benjaminogles , a hero indeed! Great work, thanks! Can also confirm that the workaround works great

[bitcoinporn]

solution not working for me...there some error. Any other solutions for this?

[renatonmendes]

Same problem. any idea of when the fix will be available in the current version, without a workaround?

[st0ffern]

Workaround.

For me the problem was that i had inputs users(first: $first, filter: $filter)
And the filter part was a sub-layer of variables i could pass, in example:
filter: {firstName: "test"}

Moving args out of filter like this:
users(first: $first, firstName: $firstName)
solved the problem, but this is just a workaround and you should use filter in the long run for cleaner code

[alloy]

As discussed in #2142, this has been fixed in master with 6d49a52050069dd45f422fdb054092b1bbdda9b8.