openapi-to-graphql icon indicating copy to clipboard operation
openapi-to-graphql copied to clipboard

Infinite loop on recursive oneOf component

Open smeyffret opened this issue 3 years ago • 4 comments

Describe the bug Given a schema that has a recursive oneOf element, openapi-to-graphql fails to generate the graphql schema with a Maximum call stack size exceeded error. I suspect there is an infinite loop while trying to convert the recursive element.

I'm using Smithy (union type) to generate the openapi file (see section below for openapi file). I came across that issue when trying to model arithmetic expressions. Here is an example with recursive oneOf:

union Expression {
    not: Expression,
    value: String
}

To Reproduce Steps to reproduce the behavior:

  1. Download this minimal openapi service definition example RecursionExample.swagger.api.json.txt
  2. Run openapi-to-graphql RecursionExample.swagger.api.json.txt (using "openapi-to-graphql-cli": "2.6.3")
  3. See error Maximum call stack size exceeded
RangeError: Maximum call stack size exceeded
    at Function.enabled (node_modules/@amzn/openapi-to-appsync/node_modules/debug/src/common.js:198:18)
    at Function.get [as enabled] (node_modules/@amzn/openapi-to-appsync/node_modules/debug/src/common.js:123:53)
    at debug (node_modules/@amzn/openapi-to-appsync/node_modules/debug/src/common.js:66:15)
    at createOrReuseOt (node_modules/@amzn/openapi-to-appsync/node_modules/openapi-to-graphql/dist/index.js:3558:13)
    at getGraphQLType (node_modules/@amzn/openapi-to-appsync/node_modules/openapi-to-graphql/dist/index.js:3478:20)
    at node_modules/@amzn/openapi-to-appsync/node_modules/openapi-to-graphql/dist/index.js:3649:20
    at Array.map (<anonymous>)
    at createOrReuseUnion (node_modules/@amzn/openapi-to-appsync/node_modules/openapi-to-graphql/dist/index.js:3648:60)
    at getGraphQLType (node_modules/@amzn/openapi-to-appsync/node_modules/openapi-to-graphql/dist/index.js:3488:20)
    at createFields (node_modules/@amzn/openapi-to-appsync/node_modules/openapi-to-graphql/dist/index.js:3846:28)

Expected behavior I'd expect openapi-to-graphql to not fail to generate the schema, and to, by order of preference:

  1. generate a union type (preferred)
  2. generate a plain structure (if not possible)
  3. generate a JSON blob (last resort)

Example of ideal union type that should be generated (if possible):

union Expression = ValueExpression | NotExpression
type NotExpression {
    not: Expression!
}
type ValueExpression {
    value: String!
}

Example of alternative that would still be acceptable:

type Expression {
    not: Expression
    value: String
}

This is not great as we lose the concept of union type, but it is better than having the whole schema generation failing.

Finally, if none of those are possible, JSON would be a last resort.

smeyffret avatar Mar 19 '22 16:03 smeyffret

Any idea / suggestion on how to work around this issue?

smeyffret avatar Apr 18 '22 18:04 smeyffret

I might have found the root cause, checkAmbiguousMemberTypes needs to be moved after assigning def.graphQLType (here) so that when fetching for the fields of the current def, it won't try to re-create the union type.

I tried with my schema and it worked with that change. I could submit a pull request but I'm not too familiar with github and I don't know how to do that (I tried creating a separate branch and pushing it but I don't have permission). I also couldn't write a proper unit tests for that, if the schema is too simple then GraphQL will complain:

Schema must contain uniquely named types but contains multiple types named "Expression".

I could spend more time trying to write a proper unit-test, but I'd like to have confirmation from someone first, since that issue doesn't seem to have much traction.

smeyffret avatar Apr 18 '22 23:04 smeyffret

I got some help and figured out how to submit a pull request (https://github.com/IBM/openapi-to-graphql/pull/457).

smeyffret avatar Apr 19 '22 15:04 smeyffret

Any update?

smeyffret avatar Jun 25 '22 21:06 smeyffret