Why are globalMiddlewares getting called so many times in one request?

[laukaichung]

I've created two global middlewares, ErrorLoggerMiddleware and DataLoaderMiddleware. Both are getting called about hundreds of times in just one request.

I don't think it is okay. It delays the response for 2- 3 seconds since the dataloader middleware will create several dataloader instances every time it gets called. Is that how global middlewares supposed to work?

Update

I just checked createSimpleFieldResolver in resolvers/create.js. I found that the ErrorLoggerMiddleware got called on every single object field in the response data.

That's not ideal in my situation. I use @FieldResolver() to return product's specifications. Most products have around 40-50 spec attributes and every attribute is an objects of its own with around 5 fields. I fetch 10 products' specifications per request. That's why the global middlewares get called hundreds of times. I also use @UseMiddleware() in the resolver. It only gets called once per request. I thought it would be the same for global middlewares.

Is there any workaround for this? I think there should be an option to disable running global middlewares in @FieldResolver().

@Service()
export class ErrorLoggerMiddleware implements MiddlewareInterface<GqlContext> {
    async use({ context, info }: ResolverData<GqlContext>, next: NextFn) {
        console.log('error') // <<== getting many logs!
        try {
            return await next();

        } catch (err) {

            if (!(err instanceof ArgumentValidationError) && !(err instanceof PublicError) ) {
                // hide errors from db like printing sql query
                ServerLogger.error('ErrorLoggerMiddleware',err);
                throw new Error("Unknown error occurred. Try again later!");

            }else if(isDebug()){

                console.log(err);
            }

            throw err;
        }
    }
}

Example Resolver

@Resolver(objectType => ItemInfo)
export class ItemResolver implements ResolverInterface<ItemInfo> {

    @FieldResolver()
    async itemAttrList(@Root() item: ItemInfo,@Ctx(){dataLoader}:GqlContext): Promise<ItemAttr[]> {
        let builder = new itemSpecBuilder();
        return builder.getAttrList(item.specStr)
    }
}

Associated package:

    "graphql": "^14.0.2",
    "graphql-depth-limit": "^1.1.0",
    "graphql-tag": "^2.10.0",
    "graphql-type-json": "^0.2.1",
    "graphql-yoga": "^1.16.7",

[MichalLytek]

since the dataloader middleware will create several dataloader instances every time it gets called

Just create a guard and place it in context object:

if (context.dataLoaderInitialized) {
  return next();
}
context.dataLoaderInitialized = true;

// rest of your dataloader initialization code

I think I could add a @Middleware decorator with option runOnce: true that will do exactly the same check under the hood.

Is that how global middlewares supposed to work?

Yes, global middlewares are run for every single resolver. That's because graphql execute each resolver independently, so if you do try-catch only on the query resolver, it won't catch error from nested object field resolvers.

Is there any workaround for this? I think there should be an option to disable running global middlewares in @FieldResolver().

For now it's not possible to disable global middlewares for field resolvers. I am planing to revise the middlewares feature and allow for placing UseMiddleware on class level + support scoping global middlewares using @Middleware decorator to run them only for base resolvers or advanced field resolvers or simple field resolvers (and its combinations).

[laukaichung]

I have no choice but to disable usage of global middlewares and use this workaround for the error logger since calling it hundreds of times per request is just too much:

    const server = new ApolloServer({
        schema,
        tracing: true,
        cacheControl: true,
        context: ({req:request}) => {
            try {
                let context = {} as GqlContext;
                context.user = request.user;
                context.headers = request.headers as any;
                context = addDataLoaderToContext(context);
                return context;
            }catch(err){
                console.log(err);
            }
        },
        formatError: formatArgumentValidationError,
        formatResponse:(response)=>{
            return GqlErrorLogger(response)
        }
    });

I use the formatResponse function in the apollo server config to log errors if they exist. The GqlErrorLogger looks like this:

export function GqlErrorLogger(response:{errors:GraphQLError[],data:Object,extensions:Object}){
    let {errors} = response;
    if(errors) {
        errors.forEach(error=>{
                ServerLogger.error('GqlErrorLogger', error);
            }
        })
    }
    return response
}

[MichalLytek]

formatError would be a better solution:

const server = new ApolloServer({
  schema,
  formatError: err => {
    ServerLogger.error('GqlErrorLogger', err);
    return formatArgumentValidationError(err);
  },
});

Unfortunately, schema is not aware of the execution, so I'm not able in TypeGraphQL to make documentMiddlewares that will be run once per whole document (HTTP request) and can catch all errors in one try-catch block. Maybe in the future there will be @typegraphql/http helper for better integration with apollo-server.

So closing this issue as scoping middlewares is now tracked by #200 🔒

[MichalLytek]

@abdiu34567 If you need to put a logic one time per whole client request, do it in context initialization phase of your graphql server library.

[abdiu34567]

I am using apollo server 4. Can you provide an example pls?

app.use(
"/",
cors({ credentials: true }),
bodyParser.json(),
.....
expressMiddleware(server, {
context: async ({ req, res }) => ({ req, res }),
})
);

await new Promise((resolve) =>
httpServer.listen({ port: 4000 }, resolve)
);
console.log(🚀 Server ready at ${process.env.SERVER});

[MichalLytek]

Please refer to Apollo Server docs then to find out how to initialize the GraphQL context.