nestjs / docs.nestjs.com

The official documentation https://docs.nestjs.com 📕
MIT License
1.2k stars 1.73k forks source link

[Documentation] GraphQL rate limit guard source code gives an error #2528

Closed Rafat97 closed 1 year ago

Rafat97 commented 2 years ago

I'm submitting a...

Current behavior

Docs link

https://docs.nestjs.com/security/rate-limiting#graphql

Current Code

@Injectable()
export class GqlThrottlerGuard extends ThrottlerGuard {
  getRequestResponse(context: ExecutionContext) {
    const gqlCtx = GqlExecutionContext.create(context);
    const ctx = gqlCtx.getContext();
    return { req: ctx.req, res: ctx.res };
  }
}

Error Message:

TypeError: Cannot read properties of undefined (reading 'header') at GqlThrottlerGuard.handleRequest (C:\Users\User\Documents\GitHub\erp\node_modules\@nestjs\throttler\src\throttler.guard.ts:89:9) at processTicksAndRejections (node:internal/process/task_queues:96:5)

Minimal reproduction of the problem with instructions

https://stackblitz.com/edit/nestjs-typescript-starter-evkpln?file=src/config/config.module.ts

What is the motivation / use case for changing the behavior?

Code will be

@Injectable()
export class GqlThrottlerGuard extends ThrottlerGuard {
  getRequestResponse(context: ExecutionContext) {
    const gqlCtx = GqlExecutionContext.create(context);
    const ctx = gqlCtx.getContext();
    return { req: ctx.req, res: ctx.req.res };
  }
}

Environment

For Tooling issues:

Others:

jmcdo29 commented 2 years ago

Please provide a minimum reproduction repository (Git repository/StackBlitz/CodeSandbox project).

why reproductions are required

Rafat97 commented 1 year ago

Please provide a minimum reproduction repository (Git repository/StackBlitz/CodeSandbox project).

why reproductions are required

https://stackblitz.com/edit/nestjs-typescript-starter-evkpln?file=src/config/config.module.ts

Error In the console

image

Solution

image

Rafat97 commented 1 year ago

Can I update documentation? @jmcdo29

jmcdo29 commented 1 year ago

Rather than accessing ctx.req.res you should be able to set the gql context option to be context: ({ req, res }) => ({ req, res }) and get the current solution that the docs show

Rafat97 commented 1 year ago

Rather than accessing ctx.req.res you should be able to set the gql context option to be context: ({ req, res }) => ({ req, res }) and get the current solution that the docs show

May be we should mention this here. Or we can create an example.

What do you think @jmcdo29?