MichalLytek / type-graphql

Create GraphQL schema and resolvers with TypeScript, using classes and decorators!
https://typegraphql.com
MIT License
8.02k stars 675 forks source link

Server crashes when multiple resolvers fail #1525

Closed Zikoat closed 11 months ago

Zikoat commented 11 months ago

Describe the Bug When using an async field resolver and a required field, when both the required field is missing and the resolver is throwing an error, the error which is thrown in the field resolver is not caught and returned in the graphql response, but instead bubbles up and crashes the process. This can happen while an express server is running and will crash the server.

To Reproduce

git clone https://github.com/Zikoat/reproduce-apollo-server-crash
cd reproduce-apollo-server-crash
npm i
npm run reproduce

At this point, the script will throw an error in the resolver, and this error is not caught by the graphql engine, but instead bubbles up and crashes the process.

Expected Behavior The script executes and returns the error as a graphql object.

npm run reproduce

> reproduce-apollo-server-crash@1.0.0 reproduce
> npx ts-node-esm ./index.ts
{
  "http": {
    "headers": {}
  },
  "errors": [
    {
      "message": "This is a test error!",
      "locations": [
        {
          "line": 4,
          "column": 7
        }
      ],
      "path": [
        "library",
        "book",
        "author"
      ],
      "extensions": {
        "code": "INTERNAL_SERVER_ERROR"
      }
    }
  ],
  "data": null
}

Logs If applicable, add some console logs to help explain your problem. You can paste the errors with stack trace that were printed when the error occurred.

file:///home/zikoat/dev/reproduce-apollo-server-crash/index.ts:48
    throw Error("This is a test error!");
          ^
Error: This is a test error!
    at BooksResolver.author (file:///home/zikoat/dev/reproduce-apollo-server-crash/index.ts:48:11)
    at /home/zikoat/dev/reproduce-apollo-server-crash/node_modules/type-graphql/dist/resolvers/create.js:34:68
    at Object.applyMiddlewares (/home/zikoat/dev/reproduce-apollo-server-crash/node_modules/type-graphql/dist/resolvers/helpers.js:58:16)
    at /home/zikoat/dev/reproduce-apollo-server-crash/node_modules/type-graphql/dist/resolvers/create.js:27:26
    at field.resolve (/home/zikoat/dev/reproduce-apollo-server-crash/node_modules/apollo-server-core/src/utils/schemaInstrumentation.ts:106:18)
    at resolveField (/home/zikoat/dev/reproduce-apollo-server-crash/node_modules/graphql/execution/execute.js:464:18)
    at executeFields (/home/zikoat/dev/reproduce-apollo-server-crash/node_modules/graphql/execution/execute.js:292:18)
    at collectAndExecuteSubfields (/home/zikoat/dev/reproduce-apollo-server-crash/node_modules/graphql/execution/execute.js:748:10)
    at completeObjectValue (/home/zikoat/dev/reproduce-apollo-server-crash/node_modules/graphql/execution/execute.js:738:10)
    at completeValue (/home/zikoat/dev/reproduce-apollo-server-crash/node_modules/graphql/execution/execute.js:590:12) {
  locations: [ { line: 4, column: 7 } ],
  path: [ 'library', 'book', 'author' ]
}

Environment (please complete the following information):

Additional Context Add any other context about the problem here. This happens on type-graphql@1.2.0-rc.1 This happens on all versions of apollo-server, from 2 to 4.

This does not happen on type-graphql@2.0.0-beta.3, so it seems like it has been fixed there.

MichalLytek commented 11 months ago

This does not happen on type-graphql@2.0.0-beta.3, so it seems like it has been fixed there.

If so, you can close the ticket, as I don't have plans for fixing older releases.

Zikoat commented 11 months ago

type-graphql@2.0.0 isn't released yet, and type-graphql@1.1.1 isn't marked as deprecated (or planned deprecation in the future, afaik), so I'd argue that type-graphql@1.1.1 should still be maintained, but if you don't have the resources for that, then I understand.

Would you allow a pull request with fixes, to create a new patch release 1.1.2?

MichalLytek commented 11 months ago

I don't think it's an issue with type-graphql itself, rather old graphql version or express-graphql, which are used by old type-graphql version.

Zikoat commented 11 months ago

Ok, we'll wait for type-graphql 2 then. Good luck on the next major version 😊