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

Cannot read properties of undefined (reading 'getModuleContext') when using @ExecutionContext() #2227

Closed
aslansky opened this issue Jul 31, 2022 · 20 comments · Fixed by #2461
Assignees

Comments

@aslansky
Copy link

When using @ExecutionContext() in a provider I get the error:

TypeError: Cannot read properties of undefined (reading 'getModuleContext')
      at Object.getModuleContext (/Users/alexander/graphql-modules/node_modules/graphql-modules/index.js:221:23)
      at ReflectiveInjector.moduleExecutionContextGetter [as _executionContextGetter] (/Users/alexander/graphql-modules/node_modules/graphql-modules/index.js:945:41)
      at MyProvider.get (/Users/alexander/graphql-modules/node_modules/graphql-modules/index.js:660:41)
      at MyProvider.<anonymous> (/Users/alexander/graphql-modules/src/module.ts:9:22)
      at Generator.next (<anonymous>)
      at /Users/alexander/graphql-modules/src/module.ts:17:71
      at new Promise (<anonymous>)
      at __awaiter (/Users/alexander/graphql-modules/src/module.ts:13:12)
      at MyProvider.create (/Users/alexander/graphql-modules/src/module.ts:25:16)
      at hello (/Users/alexander/graphql-modules/src/module.ts:27:30),
  [Symbol(async_id_symbol)]: 75,
  [Symbol(trigger_async_id_symbol)]: 74

To Reproduce
Steps to reproduce the behavior:

Check out the codesandbox to see the error: https://codesandbox.io/s/graphql-modules-jxwdrv?file=/src/module.ts

  • open a terminal
  • run npm start
  • open https://....io/graphql
  • run query { hello }

Expected behavior

Context should be available in provider.

Environment:

  • graphql-modules: 2.1.0
  • NodeJS: 18.7.0

Additional context

To me the problem seems to be that async_hooks.executionAsyncId() returns a different id in executionContext.getModuleContext than in executionContext.create.

@kamilkisiela kamilkisiela self-assigned this Aug 1, 2022
@justindoherty
Copy link

We're seeing this too in our project that relies on ExecutionContext. It's intermittent but we don't know what's causing it or are able to reliably reproduce it.

@daytona63
Copy link

Same here, it works in some cases and doesn't in other, for some reason executionContextStore is empty which is causing that error

 getModuleContext(moduleId) {
        const picker = executionContextStore.get(executionAsyncId());
       // picker is undefined  because executionContextStore is empty
        return picker.getModuleContext(moduleId);
    },

Has anyone figured it out yet ?

@darkbasic
Copy link
Contributor

darkbasic commented Sep 16, 2023

What graphql server are you using? I'm encountering this only if I use yoga while apollo-server works fine.
With yoga the first time a client hits the server with a request it works as expected, the second time you get TypeError: Cannot read properties of undefined (reading 'getModuleContext').
A workaround is to ensure you run the following in the context builder:

const controller = createOperationController({
  context: {},
  autoDestroy: false,
});
controller.destroy();

Which is weird because:
A) We destroy the controller immediately after creating it.
B) The controller gets created anyway in the onContextBuilding lifecycle via the useGraphQLModules plugin anyway.

To reproduce, run this yoga server example (remember to yarn && yarn compile at the top level of the monorepo before).
While the server is running run the client example and try to login using any wrong credential twice.
You can run docker compose up at the root of the monorepo if you need a mongo server.

This will be the output:

niko@talos2 ~/devel/accounts/examples/graphql-server-typescript $ yarn start
[nodemon] 3.0.1
[nodemon] to restart at any time, enter `rs`
[nodemon] watching path(s): src/**/*
[nodemon] watching extensions: ts,json
[nodemon] starting `ts-node src/index.ts`

You are using the default secret "secret" which is not secure.
Please change it with a strong random token.
Server is running on http://localhost:4000/graphql
context builder
onContextBuilding
loginWithService
ERR Error: Invalid credentials
    at AccountsPassword.passwordAuthenticator (/home/niko/devel/accounts/packages/password/src/accounts-password.ts:708:15)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async AccountsPassword.authenticate (/home/niko/devel/accounts/packages/password/src/accounts-password.ts:231:23)
    at async AccountsServer.loginWithService (/home/niko/devel/accounts/packages/server/src/accounts-server.ts:222:39)
    at async /home/niko/devel/accounts/modules/module-core/src/resolvers/mutation.ts:11:29
    at async /home/niko/devel/accounts/node_modules/@envelop/core/cjs/orchestrator.js:380:27
    at async YogaServer.getResultForParams (/home/niko/devel/accounts/node_modules/graphql-yoga/cjs/server.js:304:26)
    at async YogaServer.handle (/home/niko/devel/accounts/node_modules/graphql-yoga/cjs/server.js:77:29)
    at async handlerWithErrorHandling (/home/niko/devel/accounts/node_modules/@whatwg-node/server/cjs/plugins/useErrorHandling.js:33:38) {
  path: [ 'authenticate' ],
  locations: [ { line: 2, column: 3 } ],
  extensions: [Object: null prototype] {}
}
context builder
onContextBuilding
loginWithService
ERR TypeError: Cannot read properties of undefined (reading 'getModuleContext')
    at Object.getModuleContext (/home/niko/devel/accounts/node_modules/graphql-modules/index.js:243:50)
    at ReflectiveInjector.moduleExecutionContextGetter [as _executionContextGetter] (/home/niko/devel/accounts/node_modules/graphql-modules/index.js:968:43)
    at AccountsServer.get (/home/niko/devel/accounts/node_modules/graphql-modules/index.js:683:41)
    at AccountsServer.getService (/home/niko/devel/accounts/packages/server/src/accounts-server.ts:125:16)
    at AccountsServer.loginWithService (/home/niko/devel/accounts/packages/server/src/accounts-server.ts:222:50)
    at /home/niko/devel/accounts/modules/module-core/src/resolvers/mutation.ts:13:10
    at AsyncLocalStorage.run (node:async_hooks:338:14)
    at authenticate (/home/niko/devel/accounts/modules/module-core/src/resolvers/mutation.ts:10:33)
    at /home/niko/devel/accounts/node_modules/graphql-modules/index.js:1679:38
    at dispatch (/home/niko/devel/accounts/node_modules/graphql-modules/index.js:1502:40) {
  path: [ 'authenticate' ],
  locations: [ { line: 2, column: 3 } ],
  extensions: [Object: null prototype] {}
}

The first time you get Invalid credentials which is correct, while the second time Cannot read properties of undefined (reading 'getModuleContext') which is not.

If you try the apollo-server example you will notice that it works fine even for subsequent requests.
EDIT: the apollo-server examples uses createSchemaForApollo because of #2270. I guess that's the real difference at play here.

To "fix" the yoga server example just change the context builder function and comment out line 47 to 50 and line 87 (also comment 73-76 and 85). What that does is basically ensuring that createOperationController gets called every time and not just when necessary (usually we don't need to access the injector in the context builder unless we have to resume an existing session).

@darkbasic
Copy link
Contributor

darkbasic commented Sep 18, 2023

I have seen similar issues for AsyncLocalStorage.run where middlewares like body-parser/raw-body were not preserving the context: nodejs/help#2546

We don't use these middlewares in yoga, also graphql-modules uses AsyncLocalStorage.enterWith and not run so that's probably an entirely different issue. I've looked at the graphql-modules source and I couldn't figure out anything wrong. I've tried to enforce the usage of Hooks.executionContext instead of Async.executionContext but the outcome didn't change.
I'm pretty sure something weird is happening in envelop/yoga causing the async context from onContextBuilding to not be preserved.
The usage of promise chains instead of async/await might be one such issue but I haven't checked because I'm not familiar with envelop: https://stackoverflow.com/questions/64546094/asynclocalstorage-doesnt-propagate-across-promise-chains

For sure running app.createOperationController inside YogaServerOptions.context instead of onContextBuilding (as done by the useGraphQLModules envelop plugin) makes the async context persist correctly.

@darkbasic
Copy link
Contributor

darkbasic commented Oct 16, 2023

I've upgraded accounts-js to graphql-modules-3.0.0-alpha-20231010152921-a1eddea3 but unfortunately this issue is still not fixed.

To reproduce checkout the current master of accounts-js (accounts-js/accounts@f13e9cf) following these steps:

  • git clone https://github.com/accounts-js/accounts.git
  • cd accounts-js
  • git checkout f13e9cf184e958943bd3408a11a4e67e9986167b
  • Apply the following patch:
diff --git a/modules/module-core/src/utils/context-builder.ts b/modules/module-core/src/utils/context-builder.ts
index 01574a3d..ed51e5fe 100644
--- a/modules/module-core/src/utils/context-builder.ts
+++ b/modules/module-core/src/utils/context-builder.ts
@@ -66,10 +66,10 @@ export const context = async <IUser extends User = User, Ctx extends object = ob
     };
   }
 
-  const controller = createOperationController({
+  /*const controller = createOperationController({
     context: { ...ctx },
     autoDestroy: false,
-  });
+  });*/
 
   const headerName = options.headerName || 'Authorization';
   let authToken =
@@ -80,10 +80,10 @@ export const context = async <IUser extends User = User, Ctx extends object = ob
   let user;
 
   if (authToken && !options.excludeAddUserInContext) {
-    /*const controller = createOperationController({
+    const controller = createOperationController({
       context: { ...ctx },
       autoDestroy: false,
-    });*/
+    });
     try {
       user = await controller.injector
         .get<AccountsServer<IUser>>(AccountsServer)
@@ -92,7 +92,7 @@ export const context = async <IUser extends User = User, Ctx extends object = ob
       // Empty catch
       console.error(error);
     }
-    //controller.destroy();
+    controller.destroy();
   }
 
   let ip: string | null = null;
@@ -106,7 +106,7 @@ export const context = async <IUser extends User = User, Ctx extends object = ob
     getHeader(reqOrRequest, 'user-agent') ??
     '';
 
-  controller.destroy();
+  //controller.destroy();
 
   return {
     authToken,
  • corepack enable
  • yarn
  • yarn compile
  • docker compose up
  • cd examples/graphql-server-typescript
  • yarn start
  • cd examples/react-graphql-typescript
  • yarn start
  • Try to login twice using any wrong credentials.
  • Look for Cannot read properties of undefined (reading 'getModuleContext') in the graphql-server-typescript console output.

@kamilkisiela
Copy link
Collaborator

@ardatan's fix (#2444) works for me.
This is a reproduction of the issue: https://github.com/kamilkisiela/yoga-modules-execution-context-error

@darkbasic
Copy link
Contributor

@kamilkisiela with your repro bumping to 3.0 does indeed fix the issue, but I can still reproduce it with #2227 (comment)

Have you been able to reproduce it that way?

@fridaystreet
Copy link

just hit this issue, have tried the contextBuilder fix above which does work but then I can't put the injector in the context for some reason.

We were originally running providers in operational scope, we've since changed everythig over to singleton scope as we hit another issue where by we have a schema directive (@auth) that was injectiing the user into the context. This was all working fine with apollo server and modules. We moved to yoga and it broke. Investigation found that it was in fact injecting it, but it was only injecting it into the context of the provider that was immediately called from the query with @auth on it.

A bit more inviestigation and we came to the conclusion, that appears to be in some way working as graphql-modules designed. Operational providers reloading from their cached context. All good so we just moved to Singletons as I think our old design using Operational was wrong anyway.

As part of apollo server v3 we were running a context function being passed to apollo server that called createOperationController to put injector into the grapghql-modules context.

Cut a long story short, then we hit this error once we moved to yoga. again originally still running the same context function to put the injector into the context (although I wasn't sure why useGraphqlModules doesn't do this?)

So we tried the above mentioned fix, which fixed this error at first, but then we were missing the injector. Tried putting the injector in at the same time as per below and this error comes back. Any assistance would be greatly appreciated.

Or if anyone knows how to inject into the context context of all operational providers from a directive, that would be great.

{
      onExecute({ args }) {
        return {
          onExecuteDone() {
            destroy(args);
          },
        };
      },
      async onContextBuilding({ extendContext, context }) {
        console.log('onContextBuilding')

      
        const controller = App.createOperationController({
          context,
          autoDestroy: false,
        })

        console.log('context', controller)
        extendContext({
          ...controller.context,
          injector: controller.injector,
          [graphqlModulesControllerSymbol]: controller
        })
      }
    }

@fridaystreet
Copy link

Just to add some more findings in case it helps. I've run up our graphql-modules app using envelop as per the example in the useGraphqlModules readme, instead of running it through yoga. And aside from still needing to manually put the injector into the context, it works fine with no errors.

Haven't quite worked out what that means yet, but without jumping to conclusions, feels like yoga is breaking something somewhere. below is how I instaniated things and ran it

*note on the injector. The doco for useGraphqlModules seems to imply that the injector is handled by the plugin. Maybe something has changed since the readme was last updated, but unless I manually put the injector in as per below, it's never available in the context.

I've lifted the plugin from the useGraphqlModules code and used it directly so can put the injector in


const graphqlModulesControllerSymbol = Symbol('GRAPHQL_MODULES');

function destroy(context) {
  if (context.contextValue?.[graphqlModulesControllerSymbol]) {
    context.contextValue[graphqlModulesControllerSymbol].destroy();
    context.contextValue[graphqlModulesControllerSymbol] = null;
  }
}



const getEnveloped = envelop({
      plugins: [
        useEngine({ parse, validate, specifiedRules, execute, subscribe }),
        // ... other plugins ...
        {
          onPluginInit({ setSchema }) {
            setSchema(App.schema);
          },
          onContextBuilding({ extendContext, context }) {
            const controller = App.createOperationController({
              context,
              autoDestroy: false,
            });

            extendContext({
              ...controller.context,
              injector: controller.injector,
              [graphqlModulesControllerSymbol]: controller,
            });
          },
          onExecute({ args }) {
            return {
              onExecuteDone() {
                destroy(args);
              },
            };
          },
          onSubscribe({ args }) {
            return {
              onSubscribeResult({ args }) {
                return {
                  onEnd() {
                    destroy(args);
                  },
                };
              },
              onSubscribeError() {
                destroy(args);
              },
            };
          }
        }
      ]
    })

    const mongo = await connectToMongo()

    const appEnvelop = getEnveloped()

      // Parse the initial request and validate it
      const { query, variables } = JSON.parse(event.body)
      const document = appEnvelop.parse(query)
      const validationErrors = appEnvelop.validate(appEnvelop.schema, document)

      console.log('document', document)
      if (validationErrors.length > 0) {
        return {
          statusCode: 400,
//          headers: responseHeaders,
          body: JSON.stringify({ errors: validationErrors }),
          isBase64Encoded: false
        }
      }

      // Build the context and execute
      const context = await appEnvelop.contextFactory()

      console.log('contextFactory', context)
      const result = await appEnvelop.execute({
        document,
        schema: appEnvelop.schema,
        variableValues: variables,
        contextValue:context
      })

      console.log('result', result)

      return {
        statusCode: 200,
       // headers: responseHeaders,
        body: JSON.stringify(result),
        isBase64Encoded: false
      }

@fridaystreet
Copy link

fridaystreet commented Oct 27, 2023

sorry for the multiple posts.

so it works if we use the yoga envelop like below without passing any context initially. We are running this in aws lambda

const { schema, execute, contextFactory, parse, validate } = YogaApp.getEnveloped()

but it doesn't work when we use yoga.fetch or above and pass an initial context as per below. I'm wondering if this has something to do with the context being passed in fetch below? Not sure if anyone can shed any light on it. Or is anyone able to explain what's the difference between using execute in enveloped yoga vs using yago.fetch?

const response = await YogaApp.fetch(
      event.resource +
        '?' +
        new URLSearchParams((event.queryStringParameters) || {}).toString(),
      {
        method: event.httpMethod,
        headers: event.headers,
        body: event.body
          ? Buffer.from(event.body, event.isBase64Encoded ? 'base64' : 'utf8')
          : undefined
      },
      {
        event,
        lambdaContext,
        pubSub
      }
    )

@enisdenjo
Copy link
Collaborator

enisdenjo commented Oct 30, 2023

hey @darkbasic, I am currently debugging this. I did what you said here, but I also updated the graphql-modules in graphql-server-typescript to 2.2.0 (latest stable release). Doing so, I got a different runtime error (in addition to a bunch of type errors I ignored) when trying to run the server:

$ node -r ts-node/register/transpile-only src/index.ts

/Users/enisdenjo/Develop/src/github.com/accounts-js/accounts/node_modules/graphql-modules/index.js:79
    const error = (originalError ? wrappedError('', originalError) : Error());
                                                                     ^
Error: Cannot instantiate cyclic dependency! (AccountsServer -> AccountsServer) - in Module "accounts-core" (Singleton Scope)
    at injectionError (/Users/enisdenjo/Develop/src/github.com/accounts-js/accounts/node_modules/graphql-modules/index.js:79:70)
    at cyclicDependencyError (/Users/enisdenjo/Develop/src/github.com/accounts-js/accounts/node_modules/graphql-modules/index.js:62:12)
    at ReflectiveInjector._new (/Users/enisdenjo/Develop/src/github.com/accounts-js/accounts/node_modules/graphql-modules/index.js:702:19)
    at ReflectiveInjector._getObjByKeyId (/Users/enisdenjo/Develop/src/github.com/accounts-js/accounts/node_modules/graphql-modules/index.js:645:42)
    at ReflectiveInjector._getObjByKeyId (/Users/enisdenjo/Develop/src/github.com/accounts-js/accounts/examples/graphql-server-typescript/node_modules/graphql-modules/index.js:637:103)
    at getObj (/Users/enisdenjo/Develop/src/github.com/accounts-js/accounts/examples/graphql-server-typescript/node_modules/graphql-modules/index.js:601:34)
    at ReflectiveInjector._getByKey (/Users/enisdenjo/Develop/src/github.com/accounts-js/accounts/examples/graphql-server-typescript/node_modules/graphql-modules/index.js:609:31)
    at ReflectiveInjector.get (/Users/enisdenjo/Develop/src/github.com/accounts-js/accounts/examples/graphql-server-typescript/node_modules/graphql-modules/index.js:591:21)
    at ReflectiveInjector._getByKey (/Users/enisdenjo/Develop/src/github.com/accounts-js/accounts/node_modules/graphql-modules/index.js:625:24)
    at ReflectiveInjector._getByDependency (/Users/enisdenjo/Develop/src/github.com/accounts-js/accounts/node_modules/graphql-modules/index.js:697:21) {
  addKey: [Function: addKey],
  keys: [
    Key { token: [Function], id: 1 },
    Key { token: [Function], id: 1 },
    Key { token: [Function], id: 1 },
    Key { token: [Function], id: 1 }
  ],
  constructResolvingMessage: [Function: wrappedConstructResolvingMessage],
  diOriginalError: undefined
}

Any chance you can try the latest stable 2.2.0 of graphql-modules yourself? The latest works in the repro here. Looking forward to your reply, thanks!

@darkbasic
Copy link
Contributor

hi @enisdenjo thanks for looking into this. I'm not sure I understand why you downgraded to 2.2.0, currently accounts.js' master branch already uses 3.0 alpha from ardatan's pr which is the one supposed to fix this. Am I missing something?

I'm also not sure why it doesn't work with 2.2.0 for you: anything prior to this commit already used 2.2.0 and I've never got cyclic dependencies before.

Any chance you can try the latest stable 2.2.0 of graphql-modules yourself?

As I said that's the version I was already using before trying ardatan's pr. I bumped account.js to 3.0 because I needed apollo 4 support anyway so I've been able to get ahead and update the examples.

The latest works in the #2227 (comment).

Yeah I've seen kamil's repro and I promised myself to spend some time trying to reproduce the issue in his minimal example. Still didn't manage to do so unfortunately.

@enisdenjo
Copy link
Collaborator

The latest release of ardatan's PR already includes stuff from the main branch. Also, I wanted to start with a fresh set of eyes and get to "why?" does the PR fix stuff (if it even does).

Nevertheless, it might be stale node_modules. I'll do a fresh install and see whether I still get the cyclic error.

@enisdenjo
Copy link
Collaborator

Nevertheless, it might be stale node_modules. I'll do a fresh install and see whether I still get the cyclic error.

It's not. Even after a fresh install I get the cyclic dep error.

@enisdenjo
Copy link
Collaborator

enisdenjo commented Oct 31, 2023

Oh, my bad. The error was because of multiple graphql-modules versions. I forgot to update a few workspaces. 😓

All versions, including the latest alpha, have the issue as described here.

@enisdenjo
Copy link
Collaborator

Happy to report that we've figured it out (see #2459) and we're working on a solution! 🎉

@fridaystreet
Copy link

Apologies for poluting your issue previously. We were in fact having the problem we described here #2460. We had tried to get around that issue by using singletons instead and hit this issue. Hopefully OK for us to reference the issue here in case anyone else falls into the same hole.

@enisdenjo
Copy link
Collaborator

Fixed in graphql-modules@2.2.1 with #2461. Please try it out!

@darkbasic
Copy link
Contributor

I confirm 2.3.0 fixes the issue, thanks!
I've rebased the 3.0 PR on top of your work, can you please force push from darkbasic:apollo-v4 into apollo-v4 so that new 3.0 alpha packages get created?

@enisdenjo
Copy link
Collaborator

@darkbasic rebased. New alpha versions will be available soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment