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

Supergraph handler fails with same field name #6671

Closed
4 tasks
nk-coding opened this issue Mar 16, 2024 · 2 comments · Fixed by ardatan/graphql-tools#5998
Closed
4 tasks

Supergraph handler fails with same field name #6671

nk-coding opened this issue Mar 16, 2024 · 2 comments · Fixed by ardatan/graphql-tools#5998

Comments

@nk-coding
Copy link

Issue workflow progress

Progress of the issue based on the
Contributor Workflow

Make sure to fork this template and run yarn generate in the terminal.

Please make sure Mesh package versions under package.json matches yours.

  • 2. A failing test has been provided
  • 3. A local solution has been provided
  • 4. A pull request is pending review

Describe the bug

Using the supergraph handler, if two fields on different types on the same subgraph have the same signature and are fetched identically, an error (Cannot return null for non-nullable field [...] occurs

To Reproduce Steps to reproduce the behavior:

  1. Clone https://github.com/nk-coding/graphql-mesh-supergraph-bug
  2. Start the server
    npm install
    npm start
    
  3. Open the browser and go to http://localhost:4000/graphql
  4. Run the following query:
    query {
        products(first: 1) {
            id
            name
            discounts {
                id
            }
            categories {
                id
                discounts {
                    id
                }
            }
        }
    }
  5. You will receive the following error:
    {
        "data": {
            "products": [
            null
            ]
        },
        "errors": [
            {
                "message": "Cannot return null for non-nullable field Category.discounts.",
                "path": [
                    "products",
                    0,
                    "categories",
                    0,
                    "discounts"
                ]
            }
        ]
    }

Expected behavior

Not getting the error and instead getting the data

Environment:

  • OS: Windows 11
  • @graphql-mesh/cli: 0.89.4,
  • @graphql-mesh/supergraph: 0.2.4,
  • graphql: 16.8.1
  • NodeJS: v18.19.0

Additional context

After some debugging, I found out that the error most likely has something to do getLoader
Tldr: is seems like both Product.discounts and Category.discounts seem to use the same dataloader, maybe due to wrong cache key

First, when enabling debug logging, I see the following requests:

[start-services] [start-gateway-delayed] Executing DISCOUNT with args: {
[start-services] [start-gateway-delayed]   document: 'query ($_v0_representations: [_Any!]!) {\n' +
[start-services] [start-gateway-delayed]     '  __typename\n' +
[start-services] [start-gateway-delayed]     '  _entities(representations: $_v0_representations) {\n' +
[start-services] [start-gateway-delayed]     '    ... on Product {\n' +
[start-services] [start-gateway-delayed]     '      __typename\n' +
[start-services] [start-gateway-delayed]     '      id\n' +
[start-services] [start-gateway-delayed]     '      discounts {\n' +
[start-services] [start-gateway-delayed]     '        id\n' +
[start-services] [start-gateway-delayed]     '      }\n' +
[start-services] [start-gateway-delayed]     '    }\n' +
[start-services] [start-gateway-delayed]     '    __typename\n' +
[start-services] [start-gateway-delayed]     '  }\n' +
[start-services] [start-gateway-delayed]     '}',
[start-services] [start-gateway-delayed]   variables: { _v0_representations: [ [Object] ] }
[start-services] [start-gateway-delayed] }
[start-services] [start-gateway-delayed] Executing DISCOUNT with args: {
[start-services] [start-gateway-delayed]   document: 'query ($_v0_representations: [_Any!]!) {\n' +
[start-services] [start-gateway-delayed]     '  __typename\n' +
[start-services] [start-gateway-delayed]     '  _entities(representations: $_v0_representations) {\n' +
[start-services] [start-gateway-delayed]     '    ... on Product {\n' +
[start-services] [start-gateway-delayed]     '      __typename\n' +
[start-services] [start-gateway-delayed]     '      id\n' +
[start-services] [start-gateway-delayed]     '      discounts {\n' +
[start-services] [start-gateway-delayed]     '        id\n' +
[start-services] [start-gateway-delayed]     '      }\n' +
[start-services] [start-gateway-delayed]     '    }\n' +
[start-services] [start-gateway-delayed]     '    __typename\n' +
[start-services] [start-gateway-delayed]     '  }\n' +
[start-services] [start-gateway-delayed]     '}',
[start-services] [start-gateway-delayed]   variables: { _v0_representations: [ [Object] ] }
[start-services] [start-gateway-delayed] }

Note that the first one should be for Product.discounts, while the second one should be for Category.discounts.
Even though the second one should be for the Discount entity, however it still does ... on Product, thus no data is returned and the error occurs.
With some debugging, I found out that both use the data loader under the cache key

_entities{
   discounts {
     id
  }
}

(and the cache map is also the same). I suspect that this causes both using the same data loader, and thus the second one uses the wrong query.
When I provide an alias for one of the two discount fields, it starts working, and two different data loaders are used.

I think this could potentially be fixed by including the "parent" type name (I mean Product/Category in the example) in the cache key, however I do not have sufficient knowledge of this project to be sure.

@ardatan
Copy link
Owner

ardatan commented Mar 19, 2024

This might fix it;
ardatan/graphql-tools#5998
Could you try it by adding @graphql-tools/batch-delegate as 9.0.2-alpha-20240319205135-b2f28663ded1506586af727ffda76b36e3016094 to resolutions in your package.json file?
Thanks for the issue!

@nk-coding
Copy link
Author

works as expected, thx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants