-
Notifications
You must be signed in to change notification settings - Fork 113
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
Comments
We're seeing this too in our project that relies on |
Same here, it works in some cases and doesn't in other, for some reason getModuleContext(moduleId) {
const picker = executionContextStore.get(executionAsyncId());
// picker is undefined because executionContextStore is empty
return picker.getModuleContext(moduleId);
}, Has anyone figured it out yet ? |
What graphql server are you using? I'm encountering this only if I use yoga while apollo-server works fine. const controller = createOperationController({
context: {},
autoDestroy: false,
});
controller.destroy(); Which is weird because: To reproduce, run this yoga server example (remember to This will be the output:
The first time you get If you try the apollo-server example you will notice that it works fine even for subsequent requests. 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 |
I have seen similar issues for We don't use these middlewares in yoga, also graphql-modules uses For sure running |
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:
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,
|
@ardatan's fix (#2444) works for me. |
@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? |
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.
|
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
|
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
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?
|
hey @darkbasic, I am currently debugging this. I did what you said here, but I also updated the
Any chance you can try the latest stable 2.2.0 of |
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.
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.
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. |
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. |
It's not. Even after a fresh install I get the cyclic dep error. |
Oh, my bad. The error was because of multiple All versions, including the latest alpha, have the issue as described here. |
Happy to report that we've figured it out (see #2459) and we're working on a solution! 🎉 |
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. |
Fixed in |
I confirm 2.3.0 fixes the issue, thanks! |
@darkbasic rebased. New alpha versions will be available soon. |
When using @ExecutionContext() in a provider I get the error:
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
npm start
{ hello }
Expected behavior
Context should be available in provider.
Environment:
graphql-modules
: 2.1.0Additional context
To me the problem seems to be that
async_hooks.executionAsyncId()
returns a different id in executionContext.getModuleContext than in executionContext.create.The text was updated successfully, but these errors were encountered: