Handle Errors with circular references gracefully

apollographql · Mar 25, 2019 · 9666875 · 9666875
1 parent cb50f21
commit 9666875
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 6 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -72,6 +72,7 @@
     "@types/graphql": "14.0.7",
     "@types/hapi": "17.8.5",
     "@types/jest": "24.0.11",
+    "@types/json-stringify-safe": "^5.0.0",
     "@types/koa-multer": "1.0.0",
     "@types/koa-router": "7.0.40",
     "@types/lodash": "4.14.123",

diff --git a/packages/apollo-server-core/CHANGELOG.md b/packages/apollo-server-core/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ### vNEXT
 
+* `apollo-server-core`: Handle Errors with circular references gracefully [PR#2490](https://github.com/apollographql/apollo-server/pull/2490)
 * `apollo-server-core`: Add persisted queries [PR#1149](https://github.com/apollographql/apollo-server/pull/1149)
 * `apollo-server-core`: added `BadUserInputError`
 * `apollo-server-core`: **breaking** gql is exported from gql-tag and ApolloServer requires a DocumentNode [PR#1146](https://github.com/apollographql/apollo-server/pull/1146)

diff --git a/packages/apollo-server-core/package.json b/packages/apollo-server-core/package.json
@@ -41,6 +41,7 @@
     "graphql-tag": "^2.9.2",
     "graphql-tools": "^4.0.0",
     "graphql-upload": "^8.0.2",
+    "json-stringify-safe": "^5.0.1",
     "sha.js": "^2.4.11",
     "subscriptions-transport-ws": "^0.9.11",
     "ws": "^6.0.0"

diff --git a/packages/apollo-server-core/src/__tests__/runHttpQuery.test.ts b/packages/apollo-server-core/src/__tests__/runHttpQuery.test.ts
@@ -2,8 +2,14 @@
 import MockReq = require('mock-req');
 
 import { GraphQLSchema, GraphQLObjectType, GraphQLString } from 'graphql';
+import { FormatErrorExtension } from '../formatters';
 
-import { runHttpQuery, HttpQueryError } from '../runHttpQuery';
+import {
+  runHttpQuery,
+  HttpQueryError,
+  HttpQueryResponse,
+  HttpQueryRequest,
+} from '../runHttpQuery';
 
 const queryType = new GraphQLObjectType({
   name: 'QueryType',
@@ -14,6 +20,16 @@ const queryType = new GraphQLObjectType({
         return 'it works';
       },
     },
+    circularFailure: {
+      type: GraphQLString,
+      resolve() {
+        // Errors that have circular references are surprisingly common. Many
+        // HTTP client libraries throw errors that include circular references
+        const error = new Error('Self referencing failure');
+        (error as any).circle = error;
+        throw error;
+      },
+    },
   },
 });
 
@@ -45,5 +61,30 @@ describe('runHttpQuery', () => {
         expect(err.message).toEqual('Must provide query string.');
       });
     });
+
+    it('handles errors with circular references gracefully', () => {
+      const failingRequest: HttpQueryRequest = Object.assign(
+        {},
+        mockQueryRequest,
+        {
+          options: {
+            schema,
+            extensions: [() => new FormatErrorExtension(undefined, true)],
+          },
+          query: {
+            query: '{circularFailure}',
+          },
+        },
+      );
+
+      return runHttpQuery([], failingRequest).then(
+        (response: HttpQueryResponse) => {
+          const parsed = JSON.parse(response.graphqlResponse);
+          expect(parsed.errors).toHaveLength(1);
+          expect(parsed.errors[0].message).toBe('Self referencing failure');
+          expect(parsed.errors[0].path).toEqual(['circularFailure']);
+        },
+      );
+    });
   });
 });
diff --git a/packages/apollo-server-core/src/runHttpQuery.ts b/packages/apollo-server-core/src/runHttpQuery.ts
@@ -18,6 +18,8 @@ import {
 import { CacheControlExtensionOptions } from 'apollo-cache-control';
 import { ApolloServerPlugin } from 'apollo-server-plugin-base';
 import { WithRequired } from 'apollo-server-env';
+import stableStringify from 'fast-json-stable-stringify';
+import safeStringify from 'json-stringify-safe';
 
 export interface HttpQueryRequest {
   method: string;
@@ -422,7 +424,15 @@ function serializeGraphQLResponse(
 
 // The result of a curl does not appear well in the terminal, so we add an extra new line
 function prettyJSONStringify(value: any) {
-  return JSON.stringify(value) + '\n';
+  try {
+    return stableStringify(value) + '\n';
+  } catch (e) {
+    // Some Errors thrown from some libraries include circular references and it
+    // causes "Converting circular structure to JSON" errors. As a fallback,
+    // retry with a safer version of stringify that is designed to handle
+    // circular references gracefully
+    return safeStringify(value) + '\n';
+  }
 }
 
 function cloneObject<T extends Object>(object: T): T {