Repository: mcollina/fastify-gql
Pull request #28 (2019-01-15 15:02:00)

Handle errors according to graphql spec and allow custom error handling
This PR adds the errorHandler option to fastify-gql in order to allow custom error handling.
With test and documentation.


Patch:
diff --git a/README.md b/README.md
index 2a74772..d5570e8 100644
--- a/README.md
+++ b/README.md
@@ -105,6 +105,7 @@ __fastify-gql__ supports the following options:
   exposed at `/graphql`.
 * `prefix`: String. Change the route prefix of the graphql endpoint if enabled.
 * `defineMutation`: Boolean. Add the empty Mutation definition if schema is not defined (Default: `false`).
+* `errorHandler`: `Function`  or `boolean`. Change the default error handler (Default: `true`). _Note: If a custom error handler is defined, it should return the standardized response format according to [GraphQL spec](https://graphql.org/learn/serving-over-http/#response)._
 
 ### HTTP endpoints
 
diff --git a/index.js b/index.js
index d31ed83..0eced26 100644
--- a/index.js
+++ b/index.js
@@ -79,6 +79,7 @@ module.exports = fp(async function (app, opts) {
 
   if (opts.routes !== false) {
     app.register(routes, {
+      errorHandler: opts.errorHandler,
       graphiql: opts.graphiql,
       prefix: opts.prefix
     })
@@ -177,7 +178,7 @@ module.exports = fp(async function (app, opts) {
       const operationAST = getOperationAST(document, operationName)
       if (operationAST.operation !== 'query') {
         const err = new MethodNotAllowed()
-        err.errors = { errors: ['Operation cannot be perfomed via a GET request'] }
+        err.errors = [ new Error('Operation cannot be perfomed via a GET request') ]
         throw err
       }
     }
diff --git a/routes.js b/routes.js
index 66fc609..7fbcda3 100644
--- a/routes.js
+++ b/routes.js
@@ -3,6 +3,7 @@
 const { join } = require('path')
 const Static = require('fastify-static')
 const { BadRequest } = require('http-errors')
+const { formatError, GraphQLError } = require('graphql')
 
 const responseSchema = {
   '2xx': {
@@ -11,27 +12,66 @@ const responseSchema = {
       data: {
         type: 'object',
         additionalProperties: true
+      },
+      errors: {
+        type: 'object',
+        properties: {
+          message: { type: 'string' },
+          locations: {
+            type: 'array',
+            items: {
+              type: 'object',
+              properties: {
+                line: { type: 'integer' },
+                column: { type: 'integer' }
+              }
+            }
+          },
+          path: {
+            type: 'array',
+            items: { type: 'string' }
+          }
+        }
       }
-      // TODO make this include errors
     }
   }
 }
 
-module.exports = async function (app, opts) {
-  app.setErrorHandler(async function (err, request, reply) {
-    if (!err.statusCode) {
-      throw err
-    }
+async function defaultErrorHandler (err, request, reply) {
+  if (!err.statusCode) {
+    throw err
+  }
 
-    reply.code(err.statusCode)
-    if (err.errors) {
-      return { errors: err.errors }
-    } else {
-      return {
-        message: err.message
-      }
+  reply.code(err.statusCode)
+  if (err.errors) {
+    const errors = err.errors.map(error => {
+      return error instanceof GraphQLError ? formatError(error) : { message: error.message }
+    })
+
+    return { errors }
+  } else {
+    return {
+      errors: [
+        { message: err.message }
+      ]
     }
-  })
+  }
+}
+
+function validationHandler (validationError) {
+  if (validationError) {
+    const err = new BadRequest()
+    err.errors = [validationError]
+    throw err
+  }
+}
+
+module.exports = async function (app, opts) {
+  if (typeof opts.errorHandler === 'function') {
+    app.setErrorHandler(opts.errorHandler)
+  } else if (opts.errorHandler === true || opts.errorHandler === undefined) {
+    app.setErrorHandler(defaultErrorHandler)
+  }
 
   app.get('/graphql', {
     schema: {
@@ -50,8 +90,11 @@ module.exports = async function (app, opts) {
         }
       },
       response: responseSchema
-    }
+    },
+    attachValidation: true
   }, function (request, reply) {
+    validationHandler(request.validationError)
+
     let {
       query,
       variables,
@@ -90,8 +133,11 @@ module.exports = async function (app, opts) {
         }
       },
       response: responseSchema
-    }
-  }, function (request, reply) {
+    },
+    attachValidation: true
+  }, async function (request, reply) {
+    validationHandler(request.validationError)
+
     const {
       query,
       variables,


Test patch:
diff --git a/tap-snapshots/test-routes.js-TAP.test.js b/tap-snapshots/test-routes.js-TAP.test.js
index 9b8345e..6acd216 100644
--- a/tap-snapshots/test-routes.js-TAP.test.js
+++ b/tap-snapshots/test-routes.js-TAP.test.js
@@ -61,10 +61,10 @@ exports[`test/routes.js TAP POST return 400 on error > undefined 1`] = `
 
 exports[`test/routes.js TAP mutation with GET errors > undefined 1`] = `
 {
-  "errors": {
-    "errors": [
-      "Operation cannot be perfomed via a GET request"
-    ]
-  }
+  "errors": [
+    {
+      "message": "Operation cannot be perfomed via a GET request"
+    }
+  ]
 }
 `
diff --git a/test/routes.js b/test/routes.js
index 8d49b61..524acb8 100644
--- a/test/routes.js
+++ b/test/routes.js
@@ -655,3 +655,136 @@ test('GET graphql endpoint with prefixed wrapper', async (t) => {
 
   t.strictEqual(res.statusCode, 200)
 })
+
+test('Custom error handler', async (t) => {
+  const app = Fastify()
+  const schema = `
+    type Query {
+      add(x: Int, y: Int): Int
+    }
+  `
+
+  const resolvers = {
+    add: async ({ x, y }) => t.fail('should never get called')
+  }
+
+  function errorHandler (error, req, reply) {
+    app.log.error(error)
+    reply.code(403)
+    reply.send()
+  }
+
+  app.register(GQL, {
+    schema,
+    resolvers,
+    errorHandler
+  })
+
+  // Invalid query, should throw 400 from fastify-gql but catched by user handler and set to 403
+  const res = await app.inject({
+    method: 'GET',
+    url: '/graphql?query={add(x:"2",y:2)}'
+  })
+
+  t.strictEqual(res.statusCode, 403)
+})
+
+test('Error handler set to true should not change default behavior', async (t) => {
+  const app = Fastify()
+  const schema = `
+    type Query {
+      add(x: Int, y: Int): Int
+    }
+  `
+
+  const resolvers = {
+    add: async ({ x, y }) => t.fail('should never get called')
+  }
+
+  app.register(GQL, {
+    schema,
+    resolvers,
+    errorHandler: true
+  })
+
+  // Invalid query
+  const res = await app.inject({
+    method: 'GET',
+    url: '/graphql?query={add(x:"2",y:2)}'
+  })
+
+  const expectedResult = {
+    errors: [{
+      message: 'Expected type Int, found "2".',
+      locations: [{
+        line: 1,
+        column: 8
+      }]
+    }]
+  }
+
+  t.strictEqual(res.statusCode, 400)
+  t.strictDeepEqual(JSON.parse(res.body), expectedResult)
+})
+
+test('Error handler set to false should pass error to higher handler', async (t) => {
+  const app = Fastify()
+  const schema = `
+    type Query {
+      add(x: Int, y: Int): Int
+    }
+  `
+
+  const resolvers = {
+    add: async ({ x, y }) => t.fail('should never get called')
+  }
+
+  app.register(GQL, {
+    schema,
+    resolvers,
+    errorHandler: false
+  })
+
+  app.setErrorHandler((error, req, reply) => {
+    app.log.error(error)
+    reply.code(403)
+    reply.send()
+  })
+
+  // Invalid query
+  const res = await app.inject({
+    method: 'GET',
+    url: '/graphql?query={add(x:"2",y:2)}'
+  })
+
+  t.strictEqual(res.statusCode, 403)
+})
+
+test('route validation is catched and parsed to graphql error', async (t) => {
+  const app = Fastify()
+  const schema = `
+    type Query {
+      add(x: Int, y: Int): Int
+    }
+  `
+
+  const resolvers = {
+    add: async ({ x, y }) => x + y
+  }
+
+  app.register(GQL, {
+    schema,
+    resolvers
+  })
+
+  // Invalid query
+  const res = await app.inject({
+    method: 'POST',
+    url: '/graphql'
+  })
+
+  const expectedResult = { errors: [{ message: 'body should be object' }] }
+
+  t.strictEqual(res.statusCode, 400)
+  t.strictDeepEqual(JSON.parse(res.body), expectedResult)
+})