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

Getting multiple instances of graphql error despite only having a single version #2801

Open
link2cory opened this issue Sep 17, 2020 · 33 comments
Assignees

Comments

@link2cory
Copy link

I wasn't sure whether or not to make a new issue or comment on #594 but that one is so old I figured it would be preferable to start anew

In my project I get the "ensure that there are not multiple versions of GraphQL installed in your node_modules directory" error despite having done so.

yarn list output:

yarn list --pattern graphql                   
yarn list v1.22.5
├─ @apollographql/apollo-tools@0.4.8
├─ @apollographql/graphql-playground-html@1.6.26
├─ @types/graphql-upload@8.0.4
├─ apollo-graphql@0.5.0
├─ graphql-extensions@0.12.4
├─ graphql-scalars@1.2.7
├─ graphql-tag@2.11.0
├─ graphql-tools@4.0.8
├─ graphql-upload@8.1.0
└─ graphql@15.3.0
Done in 0.87s.

and for posterity:

find node_modules -name graphql
node_modules/graphql

My tech stack includes apollo-server-lambda and nexus-schema. Not sure if/how one of them could be behind this, but the issue only cropped up after switching from apollo-server-express to apollo-server-lambda. Nevertheless, it would seem that graphql-js is reporting this error erroneously as I have proven there is only one instance of graphql in my node_modules directory.

exact error message:

Error: Cannot use GraphQLObjectType "Bio" from another module or realm.                                  
                                                                                                         
Ensure that there is only one instance of "graphql" in the node_modules                                  
directory. If different versions of "graphql" are the dependencies of other                              
relied on modules, use "resolutions" to ensure only one version is installed.                            
                                                                                                         
https://yarnpkg.com/en/docs/selective-version-resolutions                                                
                                                                                                         
Duplicate "graphql" modules cannot be used at the same time since different                              
versions may have different capabilities and behavior. The data from one                                 
version used in the function from another could produce confusing and                                    
spurious results.

Link to my project repo: https://github.com/link2cory/portfolio-backend/tree/serverless
a quick note: although it is not in the current version of my repo, I have tried making use of the resolutions yarn with exactly the same results. Plus the evidence I have provided above suggests to me that it should not be necessary based on my project dependencies.

Please correct me if I am wrong in any of these assumptions!

@onhate
Copy link

onhate commented Sep 17, 2020

hi @link2cory I'll check what is going on

@onhate
Copy link

onhate commented Sep 17, 2020

@link2cory I was afraid it was the bundler that was making the mess and on runtime despite having a single version graphql it was creating different instances of the imports, so I've made a simple test by replacing the serverless-plugin-typescript to use serverless-plugin-parcel by running:

yarn add -D parcel-bundler serverless-plugin-parcel
yarn remove serverless-plugin-typescript

and changing the plugins config on serverless.ts to:

 plugins: ['serverless-plugin-parcel', 'serverless-offline']

and now it works great and it has also reduced the bundle size from 32MB to a few Kbs, which is great for your lambda performance.

I'll continue investigate further on how to improve the version collision detection.

@link2cory
Copy link
Author

Wow thank you so much for your help!

@IvanGoncharov
Copy link
Member

@onhate Thanks for detailed investigation 🕵️
Can we improve error messages to help users to detect such issues faster?

@link2cory
Copy link
Author

@onhate

Sorry for the delay here, but I do have some updates.

It turns out that the parcel solution didn't actually work in deployment. I think because the dependencies weren't actually being bundled which while it circumvented the problem, doesn't really solve it for the serverless deployment that I am trying to achieve.

So I kept trying different webpack configurations until I came across this issue #1174

So I tried setting the NODE_ENV to 'production' and that seems to work. So I believe the real issue comes down to false positives introduced by the minification process. Not sure if this can be improved within graphql-js or just means I need to adjust my development setup for use with serverless-offline. I will report back if I come up with something.

@link2cory
Copy link
Author

Well it looks like my NODE_ENV=production workaround led me right back to a dead end. All but the first request I make to my offline lambda result in an error saying that we expected a GraphQLSchema.

I have traced this error back to assertSchema in src/type/schema.js, which calls the util function instanceOf Which is exactly the function that was edited in the issue I posted in my previous comment.

I added some console logging to the assertSchema function in src/type/schema.js so that it looks like as follows:

function assertSchema(schema) {
  console.log(
    `this is the supposed schema's constructor: ${schema.constructor.name}`
  );
  if (!isSchema(schema)) {
    console.log("it is NOT a GraphQLSchema")
    throw new Error(
      "Expected ".concat(
        (0, _inspect.default)(schema),
        " to be a GraphQL schema."
      )
    );
  }
  console.log("it IS a GraphQLSchema");

  return schema;
}

So now when I run my lambda in production but offline i get the following output from 2 identical requests to the local endpoint:

this is the supposed schema's constructor: GraphQLSchema
it IS a GraphQLSchema
this is the supposed schema's constructor: GraphQLSchema
it IS a GraphQLSchema
this is the supposed schema's constructor: GraphQLSchema
it IS a GraphQLSchema
offline: (λ: graphql) RequestId: ckfbsdo3c00020ozm9kud8nxo  Duration: 1001.04 ms  Billed Duration: 1100 ms
offline: [200] {"body":"{\"data\":{\"username\":{\"name\":\"cory\"}}}\n","statusCode":200,"headers":{"Content-Type":"application/json","Content-Length":"38","access-control-allow-credentials":"true","access-control-allow-headers":"Authorization","access-control-expose-headers":"Authorization"}}

offline: POST /dev/graphql (λ: graphql)
this is the supposed schema's constructor: GraphQLSchema
it IS a GraphQLSchema
this is the supposed schema's constructor: GraphQLSchema
it is NOT a GraphQLSchema
offline: (λ: graphql) RequestId: ckfbsdrpe00050ozma1be43tp  Duration: 222.42 ms  Billed Duration: 300 ms
offline: [500] {"body":"{\"errors\":[{\"message\":\"Expected { __validationErrors: [], description: undefined, extensions: { nexus: [NexusSchemaExtension] }, astNode: undefined, extensionASTNodes: undefined, _queryType: Query, _mutationType: undefined, _subscriptionType: undefined, _directives: [@include, @skip, @deprecated, @specifiedBy], _typeMap: { Bio: Bio, String: String, Query: Query, Boolean: Boolean, __Schema: __Schema, __Type: __Type, __TypeKind: __TypeKind, __Field: __Field, __InputValue: __InputValue, __EnumValue: __EnumValue, __Directive: __Directive, __DirectiveLocation: __DirectiveLocation }, _subTypeMap: {}, _implementationsMap: {}, _extensionsEnabled: true } to be a GraphQL schema.\",\"extensions\":{\"code\":\"INTERNAL_SERVER_ERROR\"}}]}\n","statusCode":500,"headers":{"Content-Type":"application/json","access-control-allow-credentials":"true","access-control-allow-headers":"Authorization","access-control-expose-headers":"Authorization"}}

I find this all very odd. It would seem that the schema I am providing is sometimes (all but the first request) being replaced by a similar one with a different prototype of the same name: GraphQLSchema. Since everything after my lambda handler creation is handled by Apollo-server-lambda, I suppose it is worth digging through their source as well to see if I can figure out what is going on.

@onhate
Copy link

onhate commented Sep 21, 2020

hi @link2cory , thanks for the detailed investigation, when I had run the lambda offline bundled with parcel it has worked fine, but indeed there's something else in the equation, I'll take some time to go deep in the scenarios you provided and see how to improve the false positive cases.

@onhate
Copy link

onhate commented Sep 22, 2020

@link2cory I think I finally discovered the root cause, it happens that serverless-offline uses worker threads to run the lambdas by default. Well, what that means, it means that the the lambda is instantiated in one worker thread and run in a different execution context so the Nexus schemas that are generated in the setup time uses one instance of graphql while the runtime checks after the lambda is setup uses a different instance of graphql.

By setting the following on serverless.ts it tells the serverless offline plugin that each request should run in a unique process, from setup to execution and in that case it will have the same graphql instance when nexus builds the schema and when graphql-js runs schema. This is similar to what happens in production, the same execution context is used on lambda setup and when requests are processed.

  custom: {
    'serverless-offline': {
      useChildProcesses: true
    }
  }

image

@yaacovCR
Copy link
Contributor

Of course, the instanceOf check is kind of complex....

what the user is trying to do is to make sure that the object is a schema.

But instead what the user does is check that the object is a schema instantiated with the same instance of the constructor for a schema.

but what we want the user to do is to check that the object is a schema instantiated with the same version of the constructor for a schema regardless of instance.

Maybe it makes sense instead to store metadata within the object that can use duck typing to easily check for whether it is a schema or not.

Same goes for the other type system objects...

And metadata can be added to determine what version of GraphQL the schema / type system object was created with...

Just thinking out loud...

@IvanGoncharov
Copy link
Member

@yaacovCR Fully agree that current approach is problematic however there are two problems with checking metadata:

  1. Naive implementation will result in a measurable slowdown on production build, not sure but maybe it is solvable.
  2. It will uncover a bunch of other issues. Example from top of my head:
    export const BREAK: { ... } = Object.freeze({});

    Visitors created by one instance and passed to visit from other instances will lose the ability to use BREAK.
    We can rewrite all such places to use Symbol.for or something else but it very easy to write such code in the future.

@yaacovCR
Copy link
Contributor

Not sure why there would be production slowdown, in production we are doing:

value instanceof constructor

Could do

value.kind === SCHEMA

Is that really slower if use symbol?

In development could do:

value.kind === SCHEMA && value.version === VERSION

@link2cory
Copy link
Author

@onhate thanks so much for diving deep into this. I have verified your solution in my dev environment, and now I wish I had taken a closer look at my deployed lambda because it works just fine (using stock serverless bundler) as your comment lead me to believe it would.

this seems to be quite an edge-case, so I imagine a true solution wont be super fast in the making. In the meantime I will post a link to the commit where I have this working for those who stumble upon this issue before that happens.

https://github.com/link2cory/portfolio-backend/tree/5a450bc1b2fc0e7bddd9192648a3991569273556

Thanks again for your help!

@frankyveras2
Copy link

@onhate THANK YOU so much, you just saved my day! :D :D :D

@Hole313779

This comment has been minimized.

@yaacovCR
Copy link
Contributor

@onhate -- just trying to make sure that i understand where the error comes from and if it is immune to switching the instanceof check to a Symbol.for check -- from my quick look through the serverless-offline code, i see that it can use workers to run the lambdas, but it seems to do so when configured on every execution -- why is the first execution successful and the others not? Are you able to point me in the right direction in the codebase in terms of where the setup/initial run veers off from the follow-up runs?

Appreciate your time -- just a bit puzzled.

@cjnoname
Copy link

I got the same error, anyone can help?

@thekevinbrown
Copy link

I'm getting the same error, even with useChildProcesses: true.

We've seen similar problems in other modules. As soon as they're put through webpack, instanceof stops working. The only solution I know of is to use a setup similar to @yaacovCR's, checking metadata on the object instead of using instanceof.

I'll work on a reproduction.

@thekevinbrown
Copy link

Here's the reproduction. Very simple setup, using apollo-server-lambda with webpack causes this error:

https://github.com/thekevinbrown/graphqljs-instanceof-reproduction

@thekevinbrown
Copy link

Found a workaround, seems like an Apollo Server can fix it, but also it'd be nice if this package could make it a bit easier for people using @yaacovCR's suggestion.

@daveykane
Copy link

@onhate thank you!

@scottdallamura
Copy link

scottdallamura commented Mar 7, 2021

I hit the same issue. my setup is a little different since i'm using graphql-codegen instead of apollo, but it's the same deal. there's only one copy of graphql:

dir -Recurse graphql


    Directory: C:\myrepo\ClientApp\node_modules


Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
d-----          3/7/2021   2:41 PM                graphql

and yarn only knows about one version of graphql:

yarn list --pattern graphql
yarn list v1.22.5
├─ @graphql-codegen/add@2.0.2
├─ @graphql-codegen/cli@1.21.2
├─ @graphql-codegen/core@1.17.9
│  └─ @graphql-tools/utils@6.2.4
├─ @graphql-codegen/introspection@1.18.1
├─ @graphql-codegen/plugin-helpers@1.18.3
├─ @graphql-codegen/typescript-graphql-files-modules@1.18.1
├─ @graphql-codegen/typescript-operations@1.17.15
├─ @graphql-codegen/typescript-react-query@1.2.3
├─ @graphql-codegen/typescript@1.21.1
├─ @graphql-codegen/visitor-plugin-common@1.19.0
├─ @graphql-tools/apollo-engine-loader@6.2.5
├─ @graphql-tools/batch-execute@7.0.0
├─ @graphql-tools/code-file-loader@6.3.1
├─ @graphql-tools/delegate@7.0.10
├─ @graphql-tools/git-loader@6.2.6
├─ @graphql-tools/github-loader@6.2.5
├─ @graphql-tools/graphql-file-loader@6.2.7
├─ @graphql-tools/graphql-tag-pluck@6.5.1
├─ @graphql-tools/import@6.3.0
├─ @graphql-tools/json-file-loader@6.2.6
├─ @graphql-tools/load@6.2.7
├─ @graphql-tools/merge@6.2.10
├─ @graphql-tools/optimize@1.0.1
├─ @graphql-tools/prisma-loader@6.2.7
├─ @graphql-tools/relay-operation-optimizer@6.3.0
├─ @graphql-tools/schema@7.1.3
├─ @graphql-tools/url-loader@6.8.1
├─ @graphql-tools/utils@7.5.0
├─ @graphql-tools/wrap@7.0.5
├─ graphql-config@3.2.0
│  └─ @graphql-tools/utils@6.2.4
├─ graphql-request@3.4.0
├─ graphql-tag@2.11.0
├─ graphql-upload@11.0.0
├─ graphql-ws@4.1.5
└─ graphql@15.5.0

my package.json includes

  "resolutions": {
    "graphql": "15.5.0"
  },

setting NODE_ENV to production changes the error but doesn't fix it - it still fails due to the instanceOf check not succeeding. and yeah it's probably because of some bundling, and since i'm creating a web app i kind of want to keep that.

so my hacky workaround is to create a second node project with just the graphql-codegen stuff. in my "real project" i added a "prebuild" command that runs a script (yes i'm cool and running on windows):

pushd ..\graphql
call yarn graphql-codegen --config codegen.yml
popd
copy ..\graphql\generated\*.* generated

hopefully that helps someone else :)

@ericwooley
Copy link

I'm having the same issue as @scottdallamura

Here is a commit where it happens.

https://github.com/ericwooley/graphql-code-generator/tree/4adaae6dbcad6859d8b280aa1dbe2f3a93d459d8

to trigger the issue, run yarn build && yarn generate:examples

@wesselvdv
Copy link

Maybe for anyone who's encountering this issue where you're seemingly getting this error for no reason. I've managed to track down why it was failing for me, and I've fixed it.

I got these errors because for some reason Webpack was importing both *.mjs files from graphql, and *.js files. This causes graphql to be instantiated twice, which results in this error. My error was due to the fact that the graphql-parse-resolve-info package is importing getArgumentValues from graphql/execution/values which Webpack decided to import from the *.js variation instead of the *.mjs one:

/***/ "../../../node_modules/graphql-parse-resolve-info/build-turbo/index.js":
/*!*****************************************************************************!*\
  !*** ../../../node_modules/graphql-parse-resolve-info/build-turbo/index.js ***!
  \*****************************************************************************/
/***/ ((__unused_webpack_module, exports, __webpack_require__) => {

"use strict";

Object.defineProperty(exports, "__esModule", ({ value: true }));
exports.getAlias = exports.simplify = exports.parse = exports.simplifyParsedResolveInfoFragmentWithType = exports.parseResolveInfo = exports.getAliasFromResolveInfo = void 0;
const assert = __webpack_require__(/*! assert */ "assert");
const graphql_1 = __webpack_require__(/*! graphql */ "../../../node_modules/graphql/index.mjs");
const values_1 = __webpack_require__(/*! graphql/execution/values */ "../../../node_modules/graphql/execution/values.js");
const debugFactory = __webpack_require__(/*! debug */ "../../../node_modules/debug/src/index.js");
const debug = debugFactory("graphql-parse-resolve-info");
// <SNIP MORE CODE IRRELEVANT>

As can be seen above it was importing the index.mjs variation from graphql but the values.js variation from graphql/execution/values. The fix is to replace the graphql/execution/values with the *.mjs variation: graphql/execution/values.mjs. Which can be done in Webpack by putting the following in your plugins array in your config:

plugins: [
  // other plugins maybe,
  new NormalModuleReplacementPlugin(
     /graphql\/execution\/values$/,
     'graphql/execution/values.mjs',
  )
],

@Nicoowr
Copy link

Nicoowr commented Aug 11, 2021

In our case, we put .mjs before .js in our webpack.config.js:

  resolve: {
    extensions: [".ts", ".mjs", ".json", ".js"], // IMPORTANT: .mjs has to be BEFORE .js
    plugins: [PnpWebpackPlugin],
  },

And it worked. That's black magic.

@tetchel
Copy link

tetchel commented Apr 8, 2022

I thought I'd share my own solution here, so it may save someone else hours of headdesking.

My error message was 'Expected <my schema> to be a GraphQL Schema' which was traced to the 'instanceof' check in assertSchema.

I'd previously added an alias to my webpack config in line with this comment to work around the 'multiple instances of GraphQL' issue.

If you add this line, make sure the file extension is .mjs, not .js, as well as putting the .mjs before .js in the resolve.extensions. IE:

resolve: {
      // https://github.com/apollographql/apollo-server/issues/4637#issuecomment-830767224
      extensions: ['.ts', '.mjs', '.js'],
      alias: {
        // other aliases ...
        // https://github.com/apollographql/apollo-server/issues/4637#issuecomment-706813287
        graphql$: join(
            __dirname,
            'node_modules/graphql/index.mjs',    // NOT index.js !!!
          )
      },
}

EDIT: In fact, once you put .mjs before .js, you can just delete the alias! So that's probably the best solution. apollographql/apollo-server#4637 (comment)

@yaacovCR
Copy link
Contributor

see #3616 which attempts to replace instanceof with branding using symbols

@IvanGoncharov
Copy link
Member

Looking through the root cause of this issue I don't think we can ever make GraphQLSchema created in the main thread unusable for workers.
https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm

@yaacovCR Can you please test it with your solution?
If it wouldn't work I don't think we can really do anything here and this issue can be closed.

@yaacovCR
Copy link
Contributor

Yes looks impossible, unfortunately. Thanks for catching that!

yaacovCR added a commit to graphql/graphql-js-wg that referenced this issue Jun 29, 2022
= add time expectations
= remove references to cross-realm GraphQL (sadly), see graphql/graphql-js#2801 (comment)
saihaj pushed a commit to graphql/graphql-js-wg that referenced this issue Jun 29, 2022
= add time expectations
= remove references to cross-realm GraphQL (sadly), see graphql/graphql-js#2801 (comment)
@talentlessguy
Copy link

This also happens with Deno, see esm-dev/esm.sh#547 (comment)

@davidroeca
Copy link

davidroeca commented Oct 11, 2023

If experiencing with vitest, the workaround can be found here vitejs/vite#7879. The config needs to be updated to:

// vite.config.ts

export default defineConfig({
  test: {
    server: {
      deps: {
        fallbackCJS: true,
      },
    },
  },
})

@jkepps
Copy link

jkepps commented Dec 7, 2023

i'm experiencing this issue and have posted the issue ardatan/graphql-tools#5732. the workaround suggested by @davidroeca by way of vitejs/vite#7879 doesn't solve the issue for me

@davidroeca
Copy link

@jkepps It's been a while since I've thought about this but here are a few additional bits of info from what I remember.

We also had to add the following workaround to the vite config:

  resolve: {
    alias: {
      graphql: 'graphql/index.js',
    },
  },

See this discussion thread. Could be worth considering alternatives within that thread as well.

Another issue we ran into was name shadowing. We had a directory src/graphql that was also being mapped to the graphql module name and causing issues with vite. Changing the name/mapping fixed the issues there.

@jkepps
Copy link

jkepps commented Dec 8, 2023

@davidroeca looks like the alias by itself did the trick. many thanks! 🙏🏽

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