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

Redis critical failure when using GraphQL field aliases (bug) #2292

Open
jaggad opened this issue Jun 9, 2021 · 14 comments
Open

Redis critical failure when using GraphQL field aliases (bug) #2292

jaggad opened this issue Jun 9, 2021 · 14 comments

Comments

@jaggad
Copy link
Contributor

jaggad commented Jun 9, 2021

Describe the bug

When enabling mesh sdk caching using redis, or caching on any underlying service the mesh queries. If we alias a field then the Mesh SDK query itself fails with a null error for the particular aliased field.

GraphQL Mesh SDK Failed (1 errors): Cannot return null for non-nullable field component_configs_selector_field.selector_fields

The alias looks like this

selector {
  fields: selector_fields(order_by: { order_by: asc }) {
     type
     title
   }
}

After investigating it only seems to happen on Redis. The file and inmemory cache both seem to be working as expected with aliased fields returning no problem.

I confirmed it was an alias field issue by going into a Redis GUI and changing the cached response from something like

"selector":{
   "fields":[]
}

To the original unaliased name and that fixed the error (running into an error on the next aliased field though since we rely on aliases heavily.

"selector":{
   "selector_fields":[]
}

To Reproduce

Alias a GraphQL field and use a redis cache (in our case running on a docker pod inside kubernetes).

Expected behavior

Redis caching should work as expected when hooked up via the Mesh SDK Cache config, or when redis caching is enabled on the underlying service itself (i.e our underlyding federated ApolloServer pod)

Environment:

  • OS:
  • "@apollo/federation": "^0.14.1",
    "@apollo/gateway": "^0.14.1",
  • "@graphql-mesh/cache-file": "^0.5.10",
    "@graphql-mesh/cache-inmemory-lru": "^0.5.5",
    "@graphql-mesh/cache-redis": "^0.5.10",
    "@graphql-mesh/cli": "^0.2.15",
    "@graphql-mesh/config": "^0.4.1",
    "@graphql-mesh/graphql": "^0.2.15",
    "@graphql-mesh/runtime": "^0.2.15",
    "@graphql-mesh/transform-cache": "^0.9.0",
  • NodeJS: 13.14.0

Additional context

Pretty sure it's an issue directly related to aliases, it is working fine on all other tested caching solutions. Even though the cached response is exactly the same, for example I pulled this from the file cache

 {
  "selector":{
     "fields: [{... data ...}]
  }
]

I'm available to help fix the bug and support as required as this is a high priority issue for our company releasing GraphQL Mesh to production soon. I will just need to be pointed in the right direction, or if you are able to fix the issue no worries. Let me know if/how I can help.

@jaggad
Copy link
Contributor Author

jaggad commented Jun 9, 2021

Related #1130

@jaggad
Copy link
Contributor Author

jaggad commented Jun 9, 2021

Expanded error:

Error: Cannot return null for non-nullable field component_configs_selector.selector_fields.
    at completeValue (/node_modules/graphql/execution/execute.js:560:13)
    at /node_modules/graphql/execution/execute.js:492:16
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async Promise.all (index 0)
    at async Promise.all (index 0)
    at async Promise.all (index 0)
    at async Promise.all (index 0)
    at localRequester (/dist/runtime/src/get-mesh.js:114:33) {
  locations: [],
  path: [ 'component_configs_vertical_config', 0, 'selector', 'fields' ]
}

@ardatan
Copy link
Owner

ardatan commented Jun 9, 2021

@jackedgson Thank you for reporting this issue :)
Could you try with this alpha version?
@graphql-mesh/transform-cache@0.9.1-alpha-158b53d19.0

@jaggad
Copy link
Contributor Author

jaggad commented Jun 10, 2021

@ardatan using the latest version, unfortunately still getting the same error.

@ardatan
Copy link
Owner

ardatan commented Jun 10, 2021

@jackedgson I tried to fix the issue in this PR #2293 and released the alpha version to let you try and see if it works for you.

@jaggad
Copy link
Contributor Author

jaggad commented Jun 10, 2021

@ardatan yes sorry if I wasn't clear I updated to the latest canary alpha version you mentioned and retried.

I also double checked in the node modules on the kubernetes pod to make sure I was running the alpha version based on a review of the PR I had a look at and I was.

I didn't have luck unfortunately using the alpha version of the cache transform package.

Are there any possibilities you can think of other than this. It was the exact same error related to the ask error on the name of the aliased field.

@ardatan
Copy link
Owner

ardatan commented Jun 10, 2021

@jackedgson I need to reproduce it first. If you can create a reproduction repo and even a failing test would be the best for us because I am not able to reproduce it.

@jaggad
Copy link
Contributor Author

jaggad commented Jun 10, 2021

Okay, I wonder if it's within the context of a kubernetes pod. Though I can't see why.

I'll aim to create a repo tomorrow.

@ardatan
Copy link
Owner

ardatan commented Jun 10, 2021

@jackedgson That would be an awesome step! Thanks

@jaggad
Copy link
Contributor Author

jaggad commented Jun 11, 2021

Hey @ardatan, wasn't able to complete this today. We're using hasura under the hood and hasura doesn't play nicely with apollo-federation since it doesn't expose a federated schema.. we're doing a transform of old schema to federated schema however we can't include the actual resolver object in this..

Thinking about the issue as I go and hasura (aka non-native federated schema) could very well be the cause of the problem.

@ardatan
Copy link
Owner

ardatan commented Jun 11, 2021

@jackedgson Could you try this version as well?
@graphql-mesh/cache-redis@0.5.11-alpha-9cdc093bb.0

@jaggad
Copy link
Contributor Author

jaggad commented Jun 14, 2021

@ardatan Ahh, tried that thank you. I also migrated to make sure we were using the latest version of ioredis.

Unfortunately was seeing the same error, it's such an odd error to me and doesn't make sense no matter what way I look at it. I'm going to spend some more time creating a reproduction if I can of the issue, at the moment just heavy workload so hoping to have the reproduction done sometime this week.

The Mesh is awesome though by the way... kinda unrelated feedback just looking for an opinion.

So in our company we sometimes have 5+ devs working actively on development of the underlying graphql services and the graphql queries the mesh generates from... and there can be multiple changes done on different branches which when all are PRd leads to a bunch of confusing MESH SDK conflicts.

Have you got any ideas for how we can work around this or use the mesh more effectively in a large team environment? If not all good, I'll be spending some time to investigate this for our dev efficiency and am happy to report solutions here that may be able to help future users or mesh development

@jaggad
Copy link
Contributor Author

jaggad commented Jun 17, 2021

@ardatan We're currently going to use InMemoryCache as a workaround... We will circle back around to using redis cach and I'll work to create a reproduction branch probably sometimes in the next 3 - 4 weeks. It's in our company backlog and will be addressed, just been deprioritised for the next few weeks 👍

thanks for all the assistance so far

@aylesm
Copy link

aylesm commented Jun 22, 2022

I don't think this is related to just the cache feature. I see the same issue with just the gateway and an underlying graphql service. Any aliased field is coming back as null.

@theguild-bot theguild-bot mentioned this issue Aug 11, 2022
@theguild-bot theguild-bot mentioned this issue Sep 28, 2023
This was referenced Apr 30, 2024
This was referenced May 7, 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
Development

No branches or pull requests

3 participants