search

ardatan / graphql-mesh

🕸️ GraphQL Mesh - The Graph of Everything - Federated architecture for any API service
https://the-guild.dev/graphql/mesh
MIT License
3.22k stars 331 forks source link

@graphql-mesh/grpc: Invalid value used as weak map key #6424

Open michaelmohamed opened 6 months ago

michaelmohamed commented 6 months ago

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.

Describe the bug

To Reproduce Steps to reproduce the behavior:

Expected behavior

When I use "@graphql-mesh/grpc": "^0.97.2",, I receive the following error:

Failed to generate the schema TypeError: Invalid value used as weak map key
    at WeakMap.set (<anonymous>)
    at GrpcHandler.getServiceClient (/node_modules/@graphql-mesh/grpc/cjs/index.js:209:53)
    at GrpcHandler.processDirectives (/node_modules/@graphql-mesh/grpc/cjs/index.js:276:53)
    at GrpcHandler.getMeshSource (/node_modules/@graphql-mesh/grpc/cjs/index.js:626:14)
    at async /node_modules/@graphql-mesh/cli/node_modules/@graphql-mesh/runtime/cjs/get-mesh.js:118:28
    at async Promise.allSettled (index 0)
    at async getMesh (/node_modules/@graphql-mesh/cli/node_modules/@graphql-mesh/runtime/cjs/get-mesh.js:113:5)
    at async Object.handler (/node_modules/@graphql-mesh/cli/cjs/index.js:337:53)

When I use `"@graphql-mesh/grpc": "^1.0.0", everything seems to work.

I am using GRPC reflection.

It seems that "@graphql-mesh/grpc": "^0.97.2", is the latest version. What is the different between this and the 1.0.0 version?

Environment:

Additional context

mbrowne commented 6 months ago

I created a repo earlier with a reproduction of this issue. Steps to reproduce:

git clone git@github.com:mbrowne/graphql-mesh-bug-repro.git --single-branch --branch grpc-source-issue
cd graphql-mesh-bug-repro
yarn
yarn grpc:start

Then in another terminal window:

yarn mesh:start

I also already fixed this error in a fork of this repo: https://github.com/ardatan/graphql-mesh/compare/master...mbrowne:graphql-mesh:grpc-sdl-fixes?expand=1

Unfortunately there is still a second issue that will likely be encountered by anyone who encounters this first issue, as I described at https://github.com/ardatan/graphql-mesh/issues/5438, but I suppose it makes sense to start by getting a bug fix for this error merged.

michaelmohamed commented 6 months ago

@mbrowne is there a way to use your fix in my project by modifying my package.json?

How do we get these changes merged in?

mbrowne commented 6 months ago

@michaelmohamed Unfortunately it's not that simple... TLDR if you just want to get it working, then I recommend downgrading to an older version of the grpc handler for now; I think 0.93.2 might be the last version without the bug (which was introduced in https://github.com/ardatan/graphql-mesh/pull/5420).

Explanation: the bug is happening because of a new feature that was added to include the grpc metadata in a .graphql file using a graphql directive, and then read the metadata from the .graphql file instead of directly from the grpc API. In theory, that will make it possible to publish the schema for grpc APIs to GraphQL Hive as explained in the docs, although not everyone needs that feature. The problem is that there's a bug in the way that the grpc metadata is saved in the .graphql file, meaning that graphql-mesh isn't always able to read it properly. Specifically, this seems to happen as soon as you're using a grpc API that has more than one protobuf package.

The WeakMap error is just a symptom of this issue, and my "fix" is just a better error message for it, because even when the underlying issue is fixed it would be good to have better error handling for that case. (My PR gives it this error message: "Protobuf metadata not found in the GraphQL schema or is corrupted. The gRPC handler only accepts GraphQL schema files generated using GraphQL Mesh".)

I was also trying to fix the underlying issue, but got too busy and it was also a bigger task than I first anticipated, because there seems to be an upstream issue in @graphql-tools/stitch or one of its dependencies. I'm not one of the maintainers of this repo so the code is unfamiliar to me...hopefully someone like @ardatan can take a look. Relevant details here and here.

kotyara85 commented 4 months ago

Hey there, I faced the same issue >= 0.94.0 getting this error. < 0.94.0 dynamic vars are not working with :|