Repository: mercurius-js/mercurius
Pull request #604 (2021-10-14 15:18:06)

Refine the default behaviour of Mercurius errors and associated HTTP Status Codes
Implements: https://github.com/mercurius-js/mercurius/issues/599


Patch:
diff --git a/README.md b/README.md
index 514e5a4..1bcad8e 100644
--- a/README.md
+++ b/README.md
@@ -34,6 +34,7 @@ Features:
 - [Integrations](docs/integrations/)
 - [Related Plugins](docs/plugins.md)
 - [Protocol Extensions](/docs/protocol-extension.md)
+- [HTTP](/docs/http.md)
 - [Acknowledgements](#acknowledgements)
 - [License](#license)
 
diff --git a/docs/api/options.md b/docs/api/options.md
index b6a76d9..da06da5 100644
--- a/docs/api/options.md
+++ b/docs/api/options.md
@@ -533,24 +533,21 @@ app.listen(3000)
 To control the status code for the response, the third optional parameter can be used.
 
 ```js
+throw new mercurius.ErrorWithProps('Invalid User ID', {moreErrorInfo})
+// using the defaultErrorFormatter, the response statusCode will be 500 when there is a single error
 
-    throw new mercurius.ErrorWithProps('Invalid User ID', {moreErrorInfo})
-    // using de defaultErrorFormatter the response statusCode will be 500
-
-    throw new mercurius.ErrorWithProps('Invalid User ID', {moreErrorInfo}, 200)
-    // using de defaultErrorFormatter the response statusCode will be 200
-
-    const error = new mercurius.ErrorWithProps('Invalid User ID', {moreErrorInfo}, 500)
-    error.data = {foo: 'bar'} 
-    throw error
-    // using de defaultErrorFormatter the response status code will be always 200 because error.data is defined
-
+throw new mercurius.ErrorWithProps('Invalid User ID', {moreErrorInfo}, 200)
+// using the defaultErrorFormatter, the response statusCode will be 200 when there is a single error
 
+const error = new mercurius.ErrorWithProps('Invalid User ID', {moreErrorInfo}, 500)
+error.data = {foo: 'bar'} 
+throw error
+// using the defaultErrorFormatter, the response status code will be always 200 because error.data is defined
 ```
 
 ### Error formatter
 
-Allows the status code of the response to be set, and a GraphQL response for the error to be defined. 
+Allows the status code of the response to be set, and a GraphQL response for the error to be defined. You find out how to do this [here](../http.md#custom-behaviour).
 
 By default uses the defaultErrorFormatter, but it can be overridden in the [mercurius options](/docs/api/options.md#plugin-options) changing the errorFormatter parameter.
 
diff --git a/docs/http.md b/docs/http.md
new file mode 100644
index 0000000..1129412
--- /dev/null
+++ b/docs/http.md
@@ -0,0 +1,137 @@
+# mercurius
+
+- [HTTP Status Codes](#http-status-codes)
+  - [Default behaviour](#default-behaviour)
+    - [Response with data](#response-with-data)
+    - [Invalid input document](#invalid-input-document)
+    - [Multiple errors](#multiple-errors)
+    - [Single error with `statusCode` property](#single-error-with-statuscode-property)
+    - [Single error with no `statusCode` property](#single-error-with-no-statuscode-property)
+  - [Custom behaviour](#custom-behaviour)
+    - [`200 OK` on all requests](#200-ok-on-all-requests)
+
+Mercurius exhibits the following behaviour when serving GraphQL over HTTP.
+
+## HTTP Status Codes
+
+### Default behaviour
+
+Mercurius has the following default behaviour for HTTP Status Codes.
+
+#### Response with data
+
+When a GraphQL response contains `data` that is defined, the HTTP Status Code is `200 OK`.
+
+- **HTTP Status Code**: `200 OK`
+- **Data**: `!== null`
+- **Errors**: `N/A`
+
+#### Invalid input document
+
+When a GraphQL input document is invalid and fails GraphQL validation, the HTTP Status Code is `400 Bad Request`.
+
+- **HTTP Status Code**: `400 Bad Request`
+- **Data**: `null`
+- **Errors**: `MER_ERR_GQL_VALIDATION`
+
+#### Multiple errors
+
+When a GraphQL response contains multiple errors and no data, the HTTP Status Code is `400 Bad Request`.
+
+- **HTTP Status Code**: `400 Bad Request`
+- **Data**: `null`
+- **Errors**: `Multiple`
+
+#### Single error with `statusCode` property
+
+When a GraphQL response contains a single error with the `statusCode` property set and no data, the HTTP Status Code is set to this value. See [ErrorWithProps](./api/options.md#errorwithprops) for more details.
+
+- **HTTP Status Code**: `Error statusCode`
+- **Data**: `null`
+- **Errors**: `Single`
+
+#### Single error with no `statusCode` property
+
+When a GraphQL response contains a single error with no `statusCode` property set and no data, the HTTP Status Code is `500 Internal Server Error`.
+
+- **HTTP Status Code**: `500 Internal Server Error`
+- **Data**: `null`
+- **Errors**: `Single with no .statusCode property`
+
+### Custom behaviour
+
+If you wish to customise the default HTTP Status Code behaviour, one can do this using the [`errorFormatter`](./api/options.md#plugin-options) option.
+
+#### `200 OK` on all requests
+
+Enable `200 OK` on all requests as follows:
+
+```js
+'use strict'
+
+const Fastify = require('fastify')
+const mercurius = require('..')
+
+const app = Fastify()
+
+const schema = `
+  type Query {
+    add(x: Int, y: Int): Int
+  }
+`
+
+const resolvers = {
+  Query: {
+    add: async (_, obj) => {
+      const { x, y } = obj
+      return x + y
+    }
+  }
+}
+
+/**
+ * Define error formatter so we always return 200 OK
+ */
+function errorFormatter (err, ctx) {
+  const response = mercurius.defaultErrorFormatter(err, ctx)
+  response.statusCode = 200
+  return response
+}
+
+app.register(mercurius, {
+  schema,
+  resolvers,
+  errorFormatter
+})
+
+app.listen(3000)
+```
+
+With an invalid request:
+
+```graphql
+{
+  add(wrong: 1 x: 2)
+}
+```
+
+The response is:
+
+HTTP Status Code: `200 OK`
+
+```json
+{
+  "data": null,
+  "errors": [
+    {
+      "message": "Unknown argument \"wrong\" on field \"Query.add\".",
+      "locations": [
+        {
+          "line": 2,
+          "column": 7
+        }
+      ]
+    }
+  ]
+}
+```
\ No newline at end of file
diff --git a/docsify/sidebar.md b/docsify/sidebar.md
index 0df840a..75306f6 100644
--- a/docsify/sidebar.md
+++ b/docsify/sidebar.md
@@ -19,6 +19,7 @@
 * [Batched Queries](/docs/batched-queries)
 * [Persisted Queries](/docs/persisted-queries)
 * [TypeScript Usage](/docs/typescript)
+* [HTTP](/docs/http)
 * [Integrations](/docs/integrations/)
   * [nexus](/docs/integrations/nexus)
   * [TypeGraphQL](/docs/integrations/type-graphql)
@@ -27,6 +28,10 @@
   * [Tracing - OpenTelemetry](/docs/integrations/open-telemetry)
 * [Related Plugins](/docs/plugins)
   * [mercurius-auth](/docs/plugins#mercurius-auth)
+  * [mercurius-cache](/docs/plugins#mercurius-cache)
+  * [mercurius-validation](/docs/plugins#mercurius-validation)
   * [mercurius-upload](/docs/plugins#mercurius-upload)
   * [altair-fastify-plugin](/docs/plugins#altair-fastify-plugin)
+  * [mercurius-apollo-registry](/docs/plugins#mercurius-apollo-registry)
+  * [mercurius-apollo-tracing](/docs/plugins#mercurius-apollo-tracing)
 * [Protocol Extensions](/docs/protocol-extension)
diff --git a/examples/custom-http-behaviour.js b/examples/custom-http-behaviour.js
new file mode 100644
index 0000000..9490eea
--- /dev/null
+++ b/examples/custom-http-behaviour.js
@@ -0,0 +1,38 @@
+'use strict'
+
+const Fastify = require('fastify')
+const mercurius = require('..')
+
+const app = Fastify()
+
+const schema = `
+  type Query {
+    add(x: Int, y: Int): Int
+  }
+`
+
+const resolvers = {
+  Query: {
+    add: async (_, obj) => {
+      const { x, y } = obj
+      return x + y
+    }
+  }
+}
+
+/**
+ * Define error formatter so we always return 200 OK
+ */
+function errorFormatter (err, ctx) {
+  const response = mercurius.defaultErrorFormatter(err, ctx)
+  response.statusCode = 200
+  return response
+}
+
+app.register(mercurius, {
+  schema,
+  resolvers,
+  errorFormatter
+})
+
+app.listen(3000)
diff --git a/lib/errors.js b/lib/errors.js
index 09abe47..8be99ce 100644
--- a/lib/errors.js
+++ b/lib/errors.js
@@ -70,6 +70,7 @@ function defaultErrorFormatter (err, ctx) {
     log = ctx.reply ? ctx.reply.log : ctx.app.log
   }
 
+  let statusCode = err.data ? 200 : (err.statusCode || 500)
   if (err.errors) {
     errors = err.errors.map((error, idx) => {
       if (log) {
@@ -83,10 +84,26 @@ function defaultErrorFormatter (err, ctx) {
       // as the result of the outer map could potentially contain arrays with federated errors
       // the result needs to be flattened
     }).reduce((acc, val) => acc.concat(val), [])
+
+    // Override status code when there is no data or statusCode present
+    if (!err.data && typeof err.statusCode === 'undefined' && err.errors.length > 0) {
+      if (errors.length === 1) {
+        // If single error defined, use status code if present
+        if (typeof err.errors[0].originalError !== 'undefined' && typeof err.errors[0].originalError.statusCode === 'number') {
+          statusCode = err.errors[0].originalError.statusCode
+        // Otherwise, use 500
+        } else {
+          statusCode = 500
+        }
+      } else {
+        // Otherwise, if multiple errors are defined, set status code to 400
+        statusCode = 400
+      }
+    }
   }
 
   return {
-    statusCode: err.data ? 200 : /* istanbul ignore next */ (err.statusCode || 500),
+    statusCode,
     response: {
       data: err.data || null,
       errors


Test patch:
diff --git a/test/errors.js b/test/errors.js
index d01010a..9dee9c4 100644
--- a/test/errors.js
+++ b/test/errors.js
@@ -732,7 +732,9 @@ test('app.graphql which throws, with JIT enabled, twice', async (t) => {
   t.equal(errors.length, 0)
 })
 
-test('errors - should override statusCode to 200 if the data is present', async (t) => {
+test('errors - should default to HTTP Status Code `200 OK` if the data is present', async (t) => {
+  t.plan(2)
+
   const schema = `
     type Query {
       error: String
@@ -752,6 +754,7 @@ test('errors - should override statusCode to 200 if the data is present', async
   }
 
   const app = Fastify()
+  t.teardown(app.close.bind(app))
 
   app.register(GQL, {
     schema,
@@ -765,5 +768,213 @@ test('errors - should override statusCode to 200 if the data is present', async
     url: '/graphql?query={error,successful}'
   })
 
+  t.same(JSON.parse(res.body), {
+    data: {
+      error: null,
+      successful: 'Runs OK'
+    },
+    errors: [
+      {
+        message: 'Error',
+        locations: [{ column: 2, line: 1 }],
+        path: ['error']
+      }
+    ]
+  })
   t.equal(res.statusCode, 200)
 })
+
+test('errors - should default to `statusCode` from error if present, when there is a single error and no data in the response', async (t) => {
+  t.plan(2)
+
+  const schema = `
+    type Query {
+      error: String!
+    }
+  `
+
+  const resolvers = {
+    Query: {
+      error () {
+        throw new ErrorWithProps('Conflict Error', undefined, 409)
+      }
+    }
+  }
+
+  const app = Fastify()
+  t.teardown(app.close.bind(app))
+
+  app.register(GQL, {
+    schema,
+    resolvers
+  })
+
+  await app.ready()
+
+  const res = await app.inject({
+    method: 'GET',
+    url: '/graphql?query={error}'
+  })
+
+  t.same(JSON.parse(res.body), {
+    data: null,
+    errors: [
+      {
+        message: 'Conflict Error',
+        locations: [{ line: 1, column: 2 }],
+        path: ['error']
+      }
+    ]
+  })
+  t.equal(res.statusCode, 409)
+})
+
+test('errors - should default to HTTP Status Code `400 Bad Request` if multiple errors are present and no data in the response', async (t) => {
+  t.plan(2)
+
+  const schema = `
+    type Query {
+      errorOne: String
+      errorTwo: String!
+    }
+  `
+
+  const resolvers = {
+    Query: {
+      errorOne () {
+        throw new ErrorWithProps('Error One', undefined, 500)
+      },
+      errorTwo () {
+        throw new ErrorWithProps('Error Two', undefined, 500)
+      }
+    }
+  }
+
+  const app = Fastify()
+  t.teardown(app.close.bind(app))
+
+  app.register(GQL, {
+    schema,
+    resolvers
+  })
+
+  await app.ready()
+
+  const res = await app.inject({
+    method: 'GET',
+    url: '/graphql?query={errorOne errorTwo}'
+  })
+
+  t.same(JSON.parse(res.body), {
+    data: null,
+    errors: [
+      {
+        message: 'Error One',
+        locations: [{ line: 1, column: 2 }],
+        path: ['errorOne']
+      },
+      {
+        message: 'Error Two',
+        locations: [{ line: 1, column: 11 }],
+        path: ['errorTwo']
+      }
+    ]
+  })
+  t.equal(res.statusCode, 400)
+})
+
+test('errors - should default to HTTP Status Code `400 Bad Request` if GraphQL validation fails', async (t) => {
+  t.plan(2)
+
+  const schema = `
+    type Query {
+      error: String
+    }
+  `
+
+  const resolvers = {
+    Query: {
+      error () {
+        throw new ErrorWithProps('Error', undefined, 500)
+      }
+    }
+  }
+
+  const app = Fastify()
+  t.teardown(app.close.bind(app))
+
+  app.register(GQL, {
+    schema,
+    resolvers
+  })
+
+  await app.ready()
+
+  const res = await app.inject({
+    method: 'GET',
+    url: '/graphql?query={wrong}'
+  })
+
+  t.same(JSON.parse(res.body), {
+    data: null,
+    errors: [
+      {
+        message: 'Cannot query field "wrong" on type "Query".',
+        locations: [{ line: 1, column: 2 }]
+      }
+    ]
+  })
+  t.equal(res.statusCode, 400)
+})
+
+test('errors - should default to HTTP Status Code `500 Internal Server Error` if single error present but no status code defined', async (t) => {
+  t.plan(2)
+
+  const schema = `
+    type Query {
+      error: String!
+    }
+  `
+
+  const resolvers = {
+    Query: {
+      error () {
+        throw new Error('Error')
+      }
+    }
+  }
+
+  const app = Fastify()
+  t.teardown(app.close.bind(app))
+
+  app.register(GQL, {
+    schema,
+    resolvers
+  })
+
+  await app.ready()
+
+  const res = await app.inject({
+    method: 'GET',
+    url: '/graphql?query={error}'
+  })
+
+  t.same(JSON.parse(res.body), {
+    data: null,
+    errors: [
+      {
+        message: 'Error',
+        locations: [
+          {
+            line: 1,
+            column: 2
+          }
+        ],
+        path: [
+          'error'
+        ]
+      }
+    ]
+  })
+  t.equal(res.statusCode, 500)
+})