IBM / openapi-to-graphql

Translate APIs described by OpenAPI Specifications (OAS) into GraphQL
https://developer.ibm.com/open/projects/openapi-to-graphql/
MIT License
1.61k stars 211 forks source link

Infinite loop on recursive oneOf component #451

Open smeyffret opened 2 years ago

smeyffret commented 2 years ago

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 commented 2 years ago

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

smeyffret commented 2 years ago

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 commented 2 years ago

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

smeyffret commented 2 years ago

Any update?