From d5661a07bcaab6c86ae538c8c9c58cd6b64d3e0e Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Thu, 8 Mar 2018 20:33:22 +0200 Subject: [PATCH 1/2] formatError: put all extensions inside 'extensions' property --- src/error/GraphQLError.js | 13 +++++++++++-- src/error/__tests__/GraphQLError-test.js | 3 ++- src/error/formatError.js | 9 ++++----- src/execution/__tests__/executor-test.js | 16 ++++++++++++++++ 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/src/error/GraphQLError.js b/src/error/GraphQLError.js index 926152cf0f..04070e13e1 100644 --- a/src/error/GraphQLError.js +++ b/src/error/GraphQLError.js @@ -86,7 +86,7 @@ declare class GraphQLError extends Error { /** * Extension fields to add to the formatted error. */ - +extensions: ?{ [key: string]: mixed }; + +extensions: { [key: string]: mixed } | void; } export function GraphQLError( // eslint-disable-line no-redeclare @@ -135,6 +135,9 @@ export function GraphQLError( // eslint-disable-line no-redeclare }, []); } + const _extensions = + extensions || (originalError && (originalError: any).extensions); + Object.defineProperties(this, { message: { value: message, @@ -175,7 +178,13 @@ export function GraphQLError( // eslint-disable-line no-redeclare value: originalError, }, extensions: { - value: extensions || (originalError && (originalError: any).extensions), + // Coercing falsey values to undefined ensures they will not be included + // in JSON.stringify() when not provided. + value: _extensions || undefined, + // By being enumerable, JSON.stringify will include `path` in the + // resulting output. This ensures that the simplest possible GraphQL + // service adheres to the spec. + enumerable: Boolean(_extensions), }, }); diff --git a/src/error/__tests__/GraphQLError-test.js b/src/error/__tests__/GraphQLError-test.js index 71563441b1..1d903813f4 100644 --- a/src/error/__tests__/GraphQLError-test.js +++ b/src/error/__tests__/GraphQLError-test.js @@ -136,6 +136,7 @@ describe('GraphQLError', () => { message: 'msg', locations: undefined, path: ['path', 3, 'to', 'field'], + extensions: undefined, }); }); @@ -148,7 +149,7 @@ describe('GraphQLError', () => { message: 'msg', locations: undefined, path: undefined, - foo: 'bar', + extensions: { foo: 'bar' }, }); }); }); diff --git a/src/error/formatError.js b/src/error/formatError.js index e482d0d1ab..811075cdfc 100644 --- a/src/error/formatError.js +++ b/src/error/formatError.js @@ -18,17 +18,16 @@ import type { SourceLocation } from '../language/location'; export function formatError(error: GraphQLError): GraphQLFormattedError { invariant(error, 'Received null or undefined error.'); return { - ...error.extensions, message: error.message || 'An unknown error occurred.', locations: error.locations, path: error.path, + extensions: error.extensions, }; } -export type GraphQLFormattedError = { +export type GraphQLFormattedError = {| +message: string, +locations: $ReadOnlyArray | void, +path: $ReadOnlyArray | void, - // Extensions - +[key: string]: mixed, -}; + +extensions: { [key: string]: mixed } | void, +|}; diff --git a/src/execution/__tests__/executor-test.js b/src/execution/__tests__/executor-test.js index 49f7dcec2d..29c9b3b11e 100644 --- a/src/execution/__tests__/executor-test.js +++ b/src/execution/__tests__/executor-test.js @@ -382,6 +382,7 @@ describe('Execute: Handles basic execution tasks', () => { asyncError asyncRawError asyncReturnError + asyncReturnErrorWithExtensions }`; const data = { @@ -435,6 +436,12 @@ describe('Execute: Handles basic execution tasks', () => { asyncReturnError() { return Promise.resolve(new Error('Error getting asyncReturnError')); }, + asyncReturnErrorWithExtensions() { + const error = new Error('Error getting asyncReturnErrorWithExtensions'); + error.extensions = { foo: 'bar' }; + + return Promise.resolve(error); + }, }; const ast = parse(doc); @@ -449,11 +456,13 @@ describe('Execute: Handles basic execution tasks', () => { syncReturnErrorList: { type: GraphQLList(GraphQLString) }, async: { type: GraphQLString }, asyncReject: { type: GraphQLString }, + asyncRejectWithExtensions: { type: GraphQLString }, asyncRawReject: { type: GraphQLString }, asyncEmptyReject: { type: GraphQLString }, asyncError: { type: GraphQLString }, asyncRawError: { type: GraphQLString }, asyncReturnError: { type: GraphQLString }, + asyncReturnErrorWithExtensions: { type: GraphQLString }, }, }), }); @@ -474,6 +483,7 @@ describe('Execute: Handles basic execution tasks', () => { asyncError: null, asyncRawError: null, asyncReturnError: null, + asyncReturnErrorWithExtensions: null, }, errors: [ { @@ -531,6 +541,12 @@ describe('Execute: Handles basic execution tasks', () => { locations: [{ line: 13, column: 7 }], path: ['asyncReturnError'], }, + { + message: 'Error getting asyncReturnErrorWithExtensions', + locations: [{ line: 14, column: 7 }], + path: ['asyncReturnErrorWithExtensions'], + extensions: { foo: 'bar' }, + }, ], }); }); From 41295a31b10779258575f51a361c85282b9e6ab2 Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Sun, 1 Apr 2018 21:39:12 +0300 Subject: [PATCH 2/2] Address @leebyron review comments --- src/error/__tests__/GraphQLError-test.js | 1 - src/error/formatError.js | 16 +++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/error/__tests__/GraphQLError-test.js b/src/error/__tests__/GraphQLError-test.js index 1d903813f4..9398411548 100644 --- a/src/error/__tests__/GraphQLError-test.js +++ b/src/error/__tests__/GraphQLError-test.js @@ -136,7 +136,6 @@ describe('GraphQLError', () => { message: 'msg', locations: undefined, path: ['path', 3, 'to', 'field'], - extensions: undefined, }); }); diff --git a/src/error/formatError.js b/src/error/formatError.js index 811075cdfc..a671812961 100644 --- a/src/error/formatError.js +++ b/src/error/formatError.js @@ -17,17 +17,19 @@ import type { SourceLocation } from '../language/location'; */ export function formatError(error: GraphQLError): GraphQLFormattedError { invariant(error, 'Received null or undefined error.'); - return { - message: error.message || 'An unknown error occurred.', - locations: error.locations, - path: error.path, - extensions: error.extensions, - }; + const message = error.message || 'An unknown error occurred.'; + const locations = error.locations; + const path = error.path; + const extensions = error.extensions; + + return extensions + ? { message, locations, path, extensions } + : { message, locations, path }; } export type GraphQLFormattedError = {| +message: string, +locations: $ReadOnlyArray | void, +path: $ReadOnlyArray | void, - +extensions: { [key: string]: mixed } | void, + +extensions?: { [key: string]: mixed }, |};