Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RangeError: Maximum call stack size exceeded in OverlappingFieldsCanBeMergedRule #3938

Closed
MarkKoester opened this issue Jul 17, 2023 · 8 comments

Comments

@MarkKoester
Copy link

MarkKoester commented Jul 17, 2023

I noticed during some testing that sending queries with 10000 fragments causes a
RangeError: Maximum call stack size exceeded in OverlappingFieldsCanBeMergedRule error to be thrown from OverlappingFieldsCanBeMergedRule. I was wondering if this is the intended behavior and if not, how it should be handled.

This was observed on version 16.6.0 and 16.7.2

Below is the stack trace of the error and the steps to reproduce

Error

RangeError: Maximum call stack size exceeded
      at getReferencedFieldsAndFragmentNames (/graphql/validation/rules/OverlappingFieldsCanBeMergedRule.js:678:45)
      at collectConflictsBetweenFieldsAndFragment (/graphql/validation/rules/OverlappingFieldsCanBeMergedRule.js:211:5)
      at collectConflictsBetweenFieldsAndFragment (/graphql/validation/rules/OverlappingFieldsCanBeMergedRule.js:250:5)
      at collectConflictsBetweenFieldsAndFragment (/graphql/validation/rules/OverlappingFieldsCanBeMergedRule.js:250:5)
      at collectConflictsBetweenFieldsAndFragment (/graphql/validation/rules/OverlappingFieldsCanBeMergedRule.js:250:5)
      at collectConflictsBetweenFieldsAndFragment (/graphql/validation/rules/OverlappingFieldsCanBeMergedRule.js:250:5)
      at collectConflictsBetweenFieldsAndFragment (/graphql/validation/rules/OverlappingFieldsCanBeMergedRule.js:250:5)
      at collectConflictsBetweenFieldsAndFragment (/graphql/validation/rules/OverlappingFieldsCanBeMergedRule.js:250:5)
      at collectConflictsBetweenFieldsAndFragment (/graphql/validation/rules/OverlappingFieldsCanBeMergedRule.js:250:5)
      at collectConflictsBetweenFieldsAndFragment (/graphql/validation/rules/OverlappingFieldsCanBeMergedRule.js:250:5)

Reproduction steps

import {
  parse,
  buildSchema,
  validate,
  OverlappingFieldsCanBeMergedRule,
} from 'graphql';

const n = 10000;
const fragments = Array.from(Array(n).keys()).reduce((fragments, next) => {
  return fragments.concat(`\n
    fragment X${next + 1} on Query {
      ...X${next}
    }
  `);
}, '');

const query = `
query Chat {
	...X${n}
}
${fragments}
fragment X0 on Query {
	__typename
}
`;

const schema = buildSchema(`
  type Query {
    test: String!
   }
`);

const ast = parse(query);
console.log(ast.definitions.length);
const result = validate(schema, ast, [OverlappingFieldsCanBeMergedRule]);
@Crizzooo
Copy link

Did you ever resolve this? We are just stumbling on this as well

@JoviDeCroock
Copy link
Member

Sorry I missed this in the past, I created a pull request that should circumvent this issue by rather than recursing for each level of discovered fragments to using an iterator #4116

@JoviDeCroock
Copy link
Member

@Crizzooo @MarkKoester in what circumstances are you hitting a depth of 10.000, would love to have more information on that.

@Crizzooo
Copy link

So in our case, we heavily use client preset and client side fragments. This led to the generated file being massive to the point where importing from ./gql was effectively OOMing the simulator hermes environment for React Native

To solve it, we now generate code into a ./base folder.
In there, we run a script that moves all the constants to its own file, and all the types to their own file.

We then copy the .gql file and replace its import line with:

import * as types from './base/consts.generated';

In our .graphql, we have this:

/**
 * This is not a generated file.
 * This is a hardcoded file mimicking what the generated/base/index does, but using our split files instead to export all consts and types.
 */
/* eslint-disable */
export * from './base/consts.generated';
/* @ts-expect-error - complaining about reexport */
export * from './base/types.generated';

This effectively fixes our simulators again, but took manual scripting over graphql-codegen outputs.

if curious, here is the default graphql file created by codegen

LMK if theres any other info I can provide @JoviDeCroock

graphql.txt

@JoviDeCroock
Copy link
Member

Hmm, that would not be related to this issue though as the issue here is a stack depth limit that would happen on the server that validates/executes your request. The issue you are faced with seems related to types/parsing, no?

@Crizzooo
Copy link

Crizzooo commented Jun 26, 2024

Correct, @JoviDeCroock . Originally I commented as it looked like a similar cause (React Native was displaying similar max call stack error)

Then I learned more about exactly what was breaking (reading the types, not generating them)

So just sharing more on our issue, but no, its not exactly the same as the OP here.

@MarkKoester
Copy link
Author

@JoviDeCroock I don’t remember why exactly but I was doing some tests to see if a ton of lots of fragments in a query would be handled gracefully. The example was contrived but it seemed like an edge case that would be worth accounting for

@yaacovCR
Copy link
Contributor

yaacovCR commented Oct 1, 2024

We can consider this if we have a bona fide use case, closing for now.

@yaacovCR yaacovCR closed this as completed Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants
@yaacovCR @JoviDeCroock @Crizzooo @MarkKoester and others