From 1b7fbec985a957a1e07aaa561518dd0d15acd0f4 Mon Sep 17 00:00:00 2001 From: Matt Lavin Date: Mon, 25 Mar 2019 10:03:26 -0400 Subject: [PATCH] Handle Errors with circular references gracefully --- package-lock.json | 10 ++++- package.json | 1 + packages/apollo-server-core/CHANGELOG.md | 1 + packages/apollo-server-core/package.json | 1 + .../src/__tests__/runHttpQuery.test.ts | 43 ++++++++++++++++++- .../apollo-server-core/src/runHttpQuery.ts | 12 +++++- 6 files changed, 64 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index ac6231e8482..395a570232c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1629,6 +1629,12 @@ "integrity": "sha512-riJTpNwDOoRgRrrOjfcjwMBYJLVg8rzwMEAAcZ2JBvdmYWCLIsbzTFXHV517terbYzGmdzkwFYeXUem7eXVNoQ==", "dev": true }, + "@types/json-stringify-safe": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/@types/json-stringify-safe/-/json-stringify-safe-5.0.0.tgz", + "integrity": "sha512-UUA1sH0RSRROdInuDOA1yoRzbi5xVFD1RHCoOvNRPTNwR8zBkJ/84PZ6NhKVDtKp0FTeIccJCdQz1X2aJPr4uw==", + "dev": true + }, "@types/keygrip": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/@types/keygrip/-/keygrip-1.0.1.tgz", @@ -2243,6 +2249,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" @@ -7516,8 +7523,7 @@ "json-stringify-safe": { "version": "5.0.1", "resolved": "https://registry.npmjs.org/json-stringify-safe/-/json-stringify-safe-5.0.1.tgz", - "integrity": "sha1-Epai1Y/UXxmg9s4B1lcB4sc1tus=", - "dev": true + "integrity": "sha1-Epai1Y/UXxmg9s4B1lcB4sc1tus=" }, "json5": { "version": "2.1.0", diff --git a/package.json b/package.json index 6822c59651e..3970598a091 100644 --- 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 index a188c3d0e47..2ef16da7451 100644 --- 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 index 47f068a9989..ba06e2a7707 100644 --- 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 index 2f0a1565d0b..139b502bd47 100644 --- 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 index 5159dc88154..f0926acff48 100644 --- 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(object: T): T {