Graphql request hangs with subfields and per request dependency injection

wanghq commented 2 years ago

Is there an existing issue for this?

[X] I have searched the existing issues

Current behavior

For this code: https://github.com/wanghq/nestjs-graphql-hanging-req/blob/master/src/app.resolver.ts#L41-L69

@Resolver((of) => Recipe)
export class AppResolver {
  constructor(
    @Inject('CONNECTION')
    private readonly connection,
  ) {}

  @Query(() => Boolean)
  public test(): boolean {
    return true;
  }

  @ResolveField()
  public creator(@Parent() recipe): Creator {
    console.log('in creator');
    const { id } = recipe;
    const creator: Creator = {
      id: '1',
    };
    return creator;
  }

  @ResolveField()
  public creator2(@Parent() recipe): Creator {
    console.log('in creator');
    const { id } = recipe;
    const creator: Creator = {
      id: '2',
    };
    return creator;
  }
}

Req hangs if more than 1 subfields depend on connection which throws error.

curl --location --request POST 'http://localhost:3001/graphql' \
--header 'Content-Type: application/json' \
--data-raw '{"query":"{\n  recipe(id: \"string\") {\n    id\n    description\n    creator {\n      id\n    }\n    creator2 {\n      id\n    }\n  }\n}\n","variables":{}}'

Req doesn't hang if 1 subfield depends on connection which throws error.

curl --location --request POST 'http://localhost:3001/graphql' \
--header 'Content-Type: application/json' \
--data-raw '{"query":"{\n  recipe(id: \"string\") {\n    id\n    description\n    creator {\n      id\n    }\n  }\n}\n","variables":{}}'

Minimum reproduction code

https://github.com/wanghq/nestjs-graphql-hanging-req

Steps to reproduce

No response

Expected behavior

nestjs should return response with extra error info.

Package

[ ] I don't know. Or some 3rd-party package
[ ] code>@nestjs/common</code
[ ] code>@nestjs/core</code
[ ] code>@nestjs/microservices</code
[ ] code>@nestjs/platform-express</code
[ ] code>@nestjs/platform-fastify</code
[ ] code>@nestjs/platform-socket.io</code
[ ] code>@nestjs/platform-ws</code
[ ] code>@nestjs/testing</code
[ ] code>@nestjs/websockets</code
[X] Other (see below)

Other package

https://github.com/graphql/graphql-js

NestJS version

8.1.4

Packages versions

{
  "name": "nest-typescript-starter",
  "version": "1.0.0",
  "description": "Nest TypeScript starter repository",
  "license": "MIT",
  "scripts": {
    "prebuild": "rimraf dist",
    "build": "nest build",
    "format": "prettier --write \"src/**/*.ts\" \"test/**/*.ts\"",
    "start": "nest start",
    "start:dev": "nest start --watch",
    "start:debug": "nest start --debug --watch",
    "start:prod": "node dist/main",
    "lint": "eslint '{src,apps,libs,test}/**/*.ts' --fix",
    "test": "jest",
    "test:watch": "jest --watch",
    "test:cov": "jest --coverage",
    "test:debug": "node --inspect-brk -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --runInBand",
    "test:e2e": "echo 'No e2e tests implemented yet.'"
  },
  "dependencies": {
    "@nestjs/common": "^7.6.15",
    "@nestjs/core": "^7.5.5",
    "@nestjs/graphql": "^7.10.6",
    "@nestjs/platform-express": "^7.5.5",
    "apollo-server-core": "^2.19.0",
    "apollo-server-express": "^2.19.0",
    "cache-manager": "^3.4.0",
    "chokidar": "^3.4.3",
    "class-transformer": "^0.3.1",
    "class-validator": "^0.12.2",
    "colors": "^1.4.0",
    "commander": "^7.2.0",
    "cookie-parser": "^1.4.5",
    "cpx": "^1.5.0",
    "dotenv": "^8.2.0",
    "eslint-plugin-custom-eslint-rules": "file:eslint",
    "express-session": "^1.17.1",
    "fast-safe-stringify": "^2.0.7",
    "google-auth-library": "^6.1.3",
    "googleapis": "^52.1.0",
    "graphql": "^15.4.0",
    "module-alias": "^2.2.2",
    "ms": "^2.1.2",
    "mustache": "^4.0.1",
    "nanoid": "^3.1.23",
    "nest-winston": "^1.4.0",
    "nestjs-console": "^5.0.1",
    "ora": "^4.1.1",
    "reflect-metadata": "^0.1.13",
    "rimraf": "^3.0.2",
    "rxjs": "^6.6.3"
  },
  "devDependencies": {
    "@types/express": "^4.17.9",
    "@types/node": "14.18.3",
    "@typescript-eslint/eslint-plugin": "^4.8.2",
    "@typescript-eslint/parser": "^4.8.2",
    "apollo-server-testing": "^2.19.0",
    "eslint": "^7.14.0",
    "eslint-config-prettier": "^6.15.0",
    "jest": "^25.5.4",
    "nodemon": "^1.19.4",
    "prettier": "^2.2.1",
    "supertest": "^6.0.1",
    "ts-jest": "^25.5.1",
    "ts-node": "^8.10.2",
    "typescript": "^3.9.7"
  },
  "jest": {
    "moduleFileExtensions": [
      "js",
      "json",
      "ts"
    ],
    "rootDir": "src",
    "testRegex": ".spec.ts$",
    "transform": {
      "^.+\\.(t|j)s$": "ts-jest"
    },
    "coverageDirectory": "../coverage",
    "testEnvironment": "node"
  },
  "engines": {
    "node": "14.17.0"
  },
  "_moduleAliases": {
    "@src": "./dist"
  }
}

Node.js version

v18.0.0

In which operating systems have you tested?

[X] macOS
[ ] Windows
[ ] Linux

Other

No response

wanghq commented 2 years ago

Can anyone take a look? The problem is obvious with re-pro. I tried to debug the problem and the request got lost when this handleFieldError return null.

https://github.com/graphql/graphql-js/blob/main/src/execution/execute.ts#L532

kamilmysliwiec commented 2 years ago

This will be fixed in the next release of @nestjs/core. Here's the corresponding commit https://github.com/nestjs/nest/pull/9588/commits/8ca45b31639a42262875280b4c61fe76bc9b8688

wanghq commented 2 years ago

Hi @kamilmysliwiec , thanks for the update! Will the fix be released to v8.x.x? When will the next release happen?

update: seems the fix is merged to the v9 branch. Does this mean we have to upgrade nest to v9 to have this fix?

nestjs / graphql