Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle Errors with circular references gracefully #2490

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 8 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions packages/apollo-server-core/CHANGELOG.md
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions packages/apollo-server-core/package.json
Expand Up @@ -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"
Expand Down
43 changes: 42 additions & 1 deletion packages/apollo-server-core/src/__tests__/runHttpQuery.test.ts
Expand Up @@ -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',
Expand All @@ -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;
},
},
},
});

Expand Down Expand Up @@ -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']);
},
);
});
});
});
12 changes: 11 additions & 1 deletion packages/apollo-server-core/src/runHttpQuery.ts
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down