nestjs / nest

A progressive Node.js framework for building efficient, scalable, and enterprise-grade server-side applications with TypeScript/JavaScript 🚀
https://nestjs.com
MIT License
66.89k stars 7.56k forks source link

catchError with graphql is never executed with guards #1552

Closed cojack closed 5 years ago

cojack commented 5 years ago

I'm submitting a...


[ ] Regression 
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

I'm going to split it to two parts:

1) Whenever I add Guard to the Resolver, Interceptor is not executed, stacktrace looks like that (from graphql):

"stacktrace": [
            "Error: [object Object]",
            "    at GraphqlGuard.handleRequest (/home/cojack/Projects/awesome/awesome-backend/node_modules/@nestjs/passport/dist/auth.guard.js:63:30)",
            "    at passportFn (/home/cojack/Projects/awesome/awesome-backend/node_modules/@nestjs/passport/dist/auth.guard.js:47:120)",
            "    at passport.authenticate (/home/cojack/Projects/awesome/awesome-backend/node_modules/@nestjs/passport/dist/auth.guard.js:78:24)",
            "    at allFailed (/home/cojack/Projects/awesome/awesome-backend/node_modules/passport/lib/middleware/authenticate.js:107:18)",
            "    at attempt (/home/cojack/Projects/awesome/awesome-backend/node_modules/passport/lib/middleware/authenticate.js:180:28)",
            "    at JwtStrategy.strategy.fail (/home/cojack/Projects/awesome/awesome-backend/node_modules/passport/lib/middleware/authenticate.js:297:9)",
            "    at /home/cojack/Projects/awesome/awesome-backend/node_modules/passport-jwt/lib/strategy.js:106:33",
            "    at /home/cojack/Projects/awesome/awesome-backend/node_modules/jsonwebtoken/verify.js:126:14",
            "    at getSecret (/home/cojack/Projects/awesome/awesome-backend/node_modules/jsonwebtoken/verify.js:80:14)",
            "    at Object.module.exports [as verify] (/home/cojack/Projects/awesome/awesome-backend/node_modules/jsonwebtoken/verify.js:84:10)"

2) When I throwError from interceptor when I catch an exception thrown from resolver method, exception filter is not called. Why?

Expected behavior

Interceptors should be always called.

Minimal reproduction of the problem with instructions

import {Injectable, NestInterceptor, ExecutionContext, HttpStatus, HttpException} from '@nestjs/common';
import {Observable, throwError} from 'rxjs';
import {catchError, map, tap} from 'rxjs/operators';

@Injectable()
export class GraphqlInterceptor implements NestInterceptor {
    intercept(context: ExecutionContext, call$: Observable<any>): Observable<any> {
        console.log('Before...');

        const now = Date.now();
        return call$.pipe(
            tap(() => console.log(`After... ${Date.now() - now}ms`)),
            map(response => {
                if (response.errors) {
                    console.log('Do I have graphql errors? I do!');
                }
                console.log(response);
                return response;
            }),
            catchError(err => {
                console.log('Does it works?');
                console.error(err);
                return throwError(new HttpException('Message', HttpStatus.BAD_GATEWAY));
            })
        );
    }
}

Then I tried to put Interceptor almost everywhere, fe:

this.app.useGlobalInterceptors(new GraphqlInterceptor());

or for @Module of AppModule

    providers: [
        {
            provide: APP_INTERCEPTOR,
            useClass: GraphqlInterceptor
        }
    ]

even for:

@Resolver('Home')
@UseInterceptors(GraphqlInterceptor)
export class HomeResolver {
// logic...
}

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

I would like to manage errors for user output.

Environment


Nest version: 5.7.2

@aws/dynamodb-data-mapper   0.7.3
@aws/dynamodb-data-mapper-annotations   0.7.3
@nestjs/common  5.7.2
@nestjs/core    5.7.2
@nestjs/graphql 5.5.2
@nestjs/microservices   5.7.2
@nestjs/passport    5.1.0
@nestjs/swagger 2.5.1
@nestjs/testing 5.7.2
@nestjs/websockets  5.7.2
@types/express  4.16.1
@types/faker    4.1.5   
@types/graphql  14.0.5
@types/jsonwebtoken 8.3.0   
@types/node 10.12.21    
@types/nodemailer   4.6.5   
@types/redis    2.8.10  
@types/socket.io    2.1.2   
apollo-server-express   2.4.0   
async-exit-hook 2.0.1   
aws-sdk 2.401.0 
class-transformer   0.2.0   
class-validator 0.9.1   
cls-hooked  4.2.2   
cors    2.8.5   
dotenv-safe 5.0.1   
faker   4.1.0   
graphql 14.1.1  
graphql-subscriptions   1.0.0   
graphql-tools   4.0.4   
helmet  3.15.1  
jsonwebtoken    8.4.0   
luxon   1.11.2  
multer-s3   2.9.0   
nestjs-command  0.0.4   
nodemailer  5.1.1   
nodemon 1.18.9  
passport    0.4.0   
passport-facebook-token 3.3.0   
passport-http-bearer    1.0.1   
passport-jwt    4.0.0   
pre-commit  1.2.2   
qs-middleware   1.0.3   
reflect-metadata    0.1.13  
rxjs    6.4.0   
rxjs-compat 6.4.0   
ts-node 8.0.2   
tsconfig-paths  3.7.0   
tslib   1.9.3   
tslint  5.12.1  
twing   2.2.1   
typescript  3.2.2   
winston 2.4.4   
npm 6.7.0   



For Tooling issues:
- Node version: v10.15.1  
- Platform: Linux  

Others:

kamilmysliwiec commented 5 years ago

Could you provide a small repo which reproduces your issue? PS. Exception filters aren't supported in the current version of @nestjs/graphql.

lock[bot] commented 4 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.