From 508aa3a2b6a1d38edc1044763d2901feb12b2ea0 Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Thu, 25 May 2017 22:12:02 -0700 Subject: [PATCH] Refactor executeOperation (#883) This ensures failures related to buildExecutionContext() yield a GraphQL Result with errors rather than a thrown error from execute(), and handles top level error catching and nulling within executeOperation() instead of execute() for better alignment to spec text. This helps align to the idea that internal errors throw and GraphQL user errors are returned within the GraphQL Result. --- src/execution/__tests__/executor-test.js | 74 +++-- src/execution/__tests__/variables-test.js | 365 +++++++++++----------- src/execution/execute.js | 96 ++++-- src/graphql.js | 39 ++- src/language/__tests__/parser-test.js | 10 + src/language/parser.js | 3 + 6 files changed, 332 insertions(+), 255 deletions(-) diff --git a/src/execution/__tests__/executor-test.js b/src/execution/__tests__/executor-test.js index a502c0a8a1..08929e1d8a 100644 --- a/src/execution/__tests__/executor-test.js +++ b/src/execution/__tests__/executor-test.js @@ -612,7 +612,7 @@ describe('Execute: Handles basic execution tasks', () => { expect(result).to.deep.equal({ data: { second: 'b' } }); }); - it('throws if no operation is provided', () => { + it('provides error if no operation is provided', async () => { const doc = 'fragment Example on Type { a }'; const data = { a: 'b' }; const ast = parse(doc); @@ -625,12 +625,19 @@ describe('Execute: Handles basic execution tasks', () => { }) }); - expect(() => execute(schema, ast, data)).to.throw( - 'Must provide an operation.' - ); + const result = await execute(schema, ast, data); + expect(result).to.deep.equal({ + errors: [ + { + message: 'Must provide an operation.', + locations: undefined, + path: undefined, + } + ] + }); }); - it('throws if no operation name is provided with multiple operations', () => { + it('throws if no op name is provided with multiple operations', async () => { const doc = 'query Example { a } query OtherExample { a }'; const data = { a: 'b' }; const ast = parse(doc); @@ -643,14 +650,21 @@ describe('Execute: Handles basic execution tasks', () => { }) }); - expect(() => execute(schema, ast, data)).to.throw( - 'Must provide operation name if query contains multiple operations.' - ); + const result = await execute(schema, ast, data); + expect(result).to.deep.equal({ + errors: [ + { + message: 'Must provide operation name if query contains ' + + 'multiple operations.', + locations: undefined, + path: undefined, + } + ] + }); }); - it('throws if unknown operation name is provided', () => { + it('throws if unknown operation name is provided', async () => { const doc = 'query Example { a } query OtherExample { a }'; - const data = { a: 'b' }; const ast = parse(doc); const schema = new GraphQLSchema({ query: new GraphQLObjectType({ @@ -661,11 +675,20 @@ describe('Execute: Handles basic execution tasks', () => { }) }); - expect(() => - execute(schema, ast, data, null, null, 'UnknownExample') - ).to.throw( - 'Unknown operation named "UnknownExample".' - ); + const result = await execute({ + schema, + document: ast, + operationName: 'UnknownExample' + }); + expect(result).to.deep.equal({ + errors: [ + { + message: 'Unknown operation named "UnknownExample".', + locations: undefined, + path: undefined, + } + ] + }); }); it('uses the query schema for queries', async () => { @@ -960,17 +983,16 @@ describe('Execute: Handles basic execution tasks', () => { }) }); - let caughtError; - try { - await execute(schema, query); - } catch (error) { - caughtError = error; - } - - expect(caughtError).to.jsonEqual({ - message: - 'GraphQL cannot execute a request containing a ObjectTypeDefinition.', - locations: [ { line: 4, column: 7 } ] + const result = await execute(schema, query); + expect(result).to.deep.equal({ + errors: [ + { + message: 'GraphQL cannot execute a request containing a ' + + 'ObjectTypeDefinition.', + locations: [ { line: 4, column: 7 } ], + path: undefined, + } + ] }); }); diff --git a/src/execution/__tests__/variables-test.js b/src/execution/__tests__/variables-test.js index 078c2134bd..625ee26820 100644 --- a/src/execution/__tests__/variables-test.js +++ b/src/execution/__tests__/variables-test.js @@ -137,7 +137,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect(await execute(schema, ast)).to.deep.equal({ + expect(await execute(schema, ast)).to.deep.equal({ data: { fieldWithObjectInput: '{"a":"foo","b":["bar"],"c":"baz"}' } @@ -152,7 +152,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect(await execute(schema, ast)).to.deep.equal({ + expect(await execute(schema, ast)).to.deep.equal({ data: { fieldWithObjectInput: '{"a":"foo","b":["bar"],"c":"baz"}' } @@ -167,7 +167,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect(await execute(schema, ast)).to.deep.equal({ + expect(await execute(schema, ast)).to.deep.equal({ data: { fieldWithObjectInput: '{"a":null,"b":null,"c":"C","d":null}' } @@ -182,7 +182,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect(await execute(schema, ast)).to.deep.equal({ + expect(await execute(schema, ast)).to.deep.equal({ data: { fieldWithObjectInput: '{"b":["A",null,"C"],"c":"C"}' } @@ -220,7 +220,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect(await execute(schema, ast)).to.deep.equal({ + expect(await execute(schema, ast)).to.deep.equal({ data: { fieldWithObjectInput: '{"c":"foo","d":"DeserializedValue"}', } @@ -242,7 +242,7 @@ describe('Execute: Handles inputs', () => { const params = { input: { a: 'foo', b: [ 'bar' ], c: 'baz' } }; const result = await execute(schema, ast, null, null, params); - return expect(result).to.deep.equal({ + expect(result).to.deep.equal({ data: { fieldWithObjectInput: '{"a":"foo","b":["bar"],"c":"baz"}' } @@ -258,7 +258,7 @@ describe('Execute: Handles inputs', () => { const result = await execute(schema, withDefaultsAST); - return expect(result).to.deep.equal({ + expect(result).to.deep.equal({ data: { fieldWithObjectInput: '{"a":"foo","b":["bar"],"c":"baz"}' } @@ -269,7 +269,7 @@ describe('Execute: Handles inputs', () => { const params = { input: { a: 'foo', b: 'bar', c: 'baz' } }; const result = await execute(schema, ast, null, null, params); - return expect(result).to.deep.equal({ + expect(result).to.deep.equal({ data: { fieldWithObjectInput: '{"a":"foo","b":["bar"],"c":"baz"}' } @@ -280,7 +280,7 @@ describe('Execute: Handles inputs', () => { const params = { input: { c: 'foo', d: 'SerializedValue' } }; const result = await execute(schema, ast, null, null, params); - return expect(result).to.deep.equal({ + expect(result).to.deep.equal({ data: { fieldWithObjectInput: '{"c":"foo","d":"DeserializedValue"}' } @@ -290,55 +290,52 @@ describe('Execute: Handles inputs', () => { it('errors on null for nested non-null', async () => { const params = { input: { a: 'foo', b: 'bar', c: null } }; - let caughtError; - try { - execute(schema, ast, null, null, params); - } catch (error) { - caughtError = error; - } - - expect(caughtError).to.containSubset({ - locations: [ { line: 2, column: 17 } ], - message: - 'Variable "$input" got invalid value ' + - '{"a":"foo","b":"bar","c":null}.' + - '\nIn field "c": Expected "String!", found null.' + const result = await execute(schema, ast, null, null, params); + expect(result).to.deep.equal({ + errors: [ + { + message: + 'Variable "$input" got invalid value ' + + '{"a":"foo","b":"bar","c":null}.' + + '\nIn field "c": Expected "String!", found null.', + locations: [ { line: 2, column: 17 } ], + path: undefined, + } + ] }); }); it('errors on incorrect type', async () => { const params = { input: 'foo bar' }; - let caughtError; - try { - execute(schema, ast, null, null, params); - } catch (error) { - caughtError = error; - } - - expect(caughtError).to.containSubset({ - locations: [ { line: 2, column: 17 } ], - message: - 'Variable "$input" got invalid value "foo bar".' + - '\nExpected "TestInputObject", found not an object.' + const result = await execute(schema, ast, null, null, params); + expect(result).to.deep.equal({ + errors: [ + { + message: + 'Variable "$input" got invalid value "foo bar".' + + '\nExpected "TestInputObject", found not an object.', + locations: [ { line: 2, column: 17 } ], + path: undefined, + } + ] }); }); it('errors on omission of nested non-null', async () => { const params = { input: { a: 'foo', b: 'bar' } }; - let caughtError; - try { - execute(schema, ast, null, null, params); - } catch (error) { - caughtError = error; - } - - expect(caughtError).to.containSubset({ - locations: [ { line: 2, column: 17 } ], - message: - 'Variable "$input" got invalid value {"a":"foo","b":"bar"}.' + - '\nIn field "c": Expected "String!", found null.' + const result = await execute(schema, ast, null, null, params); + expect(result).to.deep.equal({ + errors: [ + { + message: + 'Variable "$input" got invalid value {"a":"foo","b":"bar"}.' + + '\nIn field "c": Expected "String!", found null.', + locations: [ { line: 2, column: 17 } ], + path: undefined, + } + ] }); }); @@ -351,21 +348,20 @@ describe('Execute: Handles inputs', () => { const nestedAst = parse(nestedDoc); const params = { input: { na: { a: 'foo' } } }; - let caughtError; - try { - execute(schema, nestedAst, null, null, params); - } catch (error) { - caughtError = error; - } - - expect(caughtError).to.containSubset({ - locations: [ { line: 2, column: 19 } ], - message: - 'Variable "$input" got invalid value {"na":{"a":"foo"}}.' + - '\nIn field "na": In field "c": Expected "String!", found null.' + - '\nIn field "nb": Expected "String!", found null.' + const result = await execute(schema, nestedAst, null, null, params); + expect(result).to.deep.equal({ + errors: [ + { + message: + 'Variable "$input" got invalid value {"na":{"a":"foo"}}.' + + '\nIn field "na": In field "c": Expected "String!", ' + + 'found null.' + + '\nIn field "nb": Expected "String!", found null.', + locations: [ { line: 2, column: 19 } ], + path: undefined, + } + ] }); - }); it('errors on addition of unknown input field', async () => { @@ -373,19 +369,18 @@ describe('Execute: Handles inputs', () => { input: { a: 'foo', b: 'bar', c: 'baz', extra: 'dog' } }; - let caughtError; - try { - execute(schema, ast, null, null, params); - } catch (error) { - caughtError = error; - } - - expect(caughtError).to.containSubset({ - locations: [ { line: 2, column: 17 } ], - message: - 'Variable "$input" got invalid value ' + - '{"a":"foo","b":"bar","c":"baz","extra":"dog"}.' + - '\nIn field "extra": Unknown field.' + const result = await execute(schema, ast, null, null, params); + expect(result).to.deep.equal({ + errors: [ + { + message: + 'Variable "$input" got invalid value ' + + '{"a":"foo","b":"bar","c":"baz","extra":"dog"}.' + + '\nIn field "extra": Unknown field.', + locations: [ { line: 2, column: 17 } ], + path: undefined, + } + ] }); }); @@ -401,7 +396,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect(await execute(schema, ast)).to.deep.equal({ + expect(await execute(schema, ast)).to.deep.equal({ data: { fieldWithNullableStringInput: null } @@ -416,7 +411,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect(await execute(schema, ast)).to.deep.equal({ + expect(await execute(schema, ast)).to.deep.equal({ data: { fieldWithNullableStringInput: null } @@ -431,7 +426,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect(await execute(schema, ast)).to.deep.equal({ + expect(await execute(schema, ast)).to.deep.equal({ data: { fieldWithNullableStringInput: null } @@ -446,7 +441,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect( + expect( await execute(schema, ast, null, null, { value: null }) ).to.deep.equal({ data: { @@ -463,7 +458,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect( + expect( await execute(schema, ast, null, null, { value: 'a' }) ).to.deep.equal({ data: { @@ -480,7 +475,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect(await execute(schema, ast)).to.deep.equal({ + expect(await execute(schema, ast)).to.deep.equal({ data: { fieldWithNullableStringInput: '"a"' } @@ -510,17 +505,16 @@ describe('Execute: Handles inputs', () => { } `; - let caughtError; - try { - execute(schema, parse(doc)); - } catch (error) { - caughtError = error; - } - - expect(caughtError).to.containSubset({ - locations: [ { line: 2, column: 31 } ], - message: - 'Variable "$value" of required type "String!" was not provided.' + const result = await execute(schema, parse(doc)); + expect(result).to.deep.equal({ + errors: [ + { + message: + 'Variable "$value" of required type "String!" was not provided.', + locations: [ { line: 2, column: 31 } ], + path: undefined, + } + ] }); }); @@ -532,18 +526,17 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - let caughtError; - try { - execute(schema, ast, null, null, { value: null }); - } catch (error) { - caughtError = error; - } - - expect(caughtError).to.containSubset({ - locations: [ { line: 2, column: 31 } ], - message: - 'Variable "$value" got invalid value null.\n' + - 'Expected "String!", found null.' + const result = await execute(schema, ast, null, null, { value: null }); + expect(result).to.deep.equal({ + errors: [ + { + message: + 'Variable "$value" got invalid value null.\n' + + 'Expected "String!", found null.', + locations: [ { line: 2, column: 31 } ], + path: undefined, + } + ] }); }); @@ -555,7 +548,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect( + expect( await execute(schema, ast, null, null, { value: 'a' }) ).to.deep.equal({ data: { @@ -572,7 +565,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect(await execute(schema, ast)).to.deep.equal({ + expect(await execute(schema, ast)).to.deep.equal({ data: { fieldWithNonNullableStringInput: '"a"' } @@ -587,7 +580,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect(await execute(schema, ast)).to.deep.equal({ + expect(await execute(schema, ast)).to.deep.equal({ data: { fieldWithNonNullableStringInput: null }, @@ -612,7 +605,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect(await execute(schema, ast)).to.deep.equal({ + expect(await execute(schema, ast)).to.deep.equal({ data: { fieldWithNonNullableStringInput: null }, @@ -636,7 +629,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect( + expect( await execute(schema, ast, null, null, { input: null }) ).to.deep.equal({ data: { @@ -653,7 +646,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect( + expect( await execute(schema, ast, null, null, { input: [ 'A' ] }) ).to.deep.equal({ data: { @@ -670,7 +663,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect( + expect( await execute(schema, ast, null, null, { input: [ 'A', null, 'B' ] }) ).to.deep.equal({ data: { @@ -687,18 +680,17 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - let caughtError; - try { - execute(schema, ast, null, null, { input: null }); - } catch (error) { - caughtError = error; - } - - expect(caughtError).to.containSubset({ - locations: [ { line: 2, column: 17 } ], - message: - 'Variable "$input" got invalid value null.\n' + - 'Expected "[String]!", found null.' + const result = await execute(schema, ast, null, null, { input: null }); + expect(result).to.deep.equal({ + errors: [ + { + message: + 'Variable "$input" got invalid value null.\n' + + 'Expected "[String]!", found null.', + locations: [ { line: 2, column: 17 } ], + path: undefined, + } + ] }); }); @@ -710,7 +702,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect( + expect( await execute(schema, ast, null, null, { input: [ 'A' ] }) ).to.deep.equal({ data: { @@ -727,7 +719,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect( + expect( await execute(schema, ast, null, null, { input: [ 'A', null, 'B' ] }) ).to.deep.equal({ data: { @@ -744,7 +736,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect( + expect( await execute(schema, ast, null, null, { input: null }) ).to.deep.equal({ data: { @@ -761,7 +753,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect( + expect( await execute(schema, ast, null, null, { input: [ 'A' ] }) ).to.deep.equal({ data: { @@ -779,18 +771,17 @@ describe('Execute: Handles inputs', () => { const ast = parse(doc); const vars = { input: [ 'A', null, 'B' ] }; - let caughtError; - try { - execute(schema, ast, null, null, vars); - } catch (error) { - caughtError = error; - } - - expect(caughtError).to.containSubset({ - locations: [ { line: 2, column: 17 } ], - message: - 'Variable "$input" got invalid value ["A",null,"B"].' + - '\nIn element #1: Expected "String!", found null.' + const result = await execute(schema, ast, null, null, vars); + expect(result).to.deep.equal({ + errors: [ + { + message: + 'Variable "$input" got invalid value ["A",null,"B"].' + + '\nIn element #1: Expected "String!", found null.', + locations: [ { line: 2, column: 17 } ], + path: undefined, + } + ] }); }); @@ -802,18 +793,17 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - let caughtError; - try { - execute(schema, ast, null, null, { input: null }); - } catch (error) { - caughtError = error; - } - - expect(caughtError).to.containSubset({ - locations: [ { line: 2, column: 17 } ], - message: - 'Variable "$input" got invalid value null.\n' + - 'Expected "[String!]!", found null.' + const result = await execute(schema, ast, null, null, { input: null }); + expect(result).to.deep.equal({ + errors: [ + { + message: + 'Variable "$input" got invalid value null.\n' + + 'Expected "[String!]!", found null.', + locations: [ { line: 2, column: 17 } ], + path: undefined, + } + ] }); }); @@ -825,7 +815,7 @@ describe('Execute: Handles inputs', () => { `; const ast = parse(doc); - return expect( + expect( await execute(schema, ast, null, null, { input: [ 'A' ] }) ).to.deep.equal({ data: { @@ -843,18 +833,17 @@ describe('Execute: Handles inputs', () => { const ast = parse(doc); const vars = { input: [ 'A', null, 'B' ] }; - let caughtError; - try { - execute(schema, ast, null, null, vars); - } catch (error) { - caughtError = error; - } - - expect(caughtError).to.containSubset({ - locations: [ { line: 2, column: 17 } ], - message: - 'Variable "$input" got invalid value ["A",null,"B"].' + - '\nIn element #1: Expected "String!", found null.' + const result = await execute(schema, ast, null, null, vars); + expect(result).to.deep.equal({ + errors: [ + { + message: + 'Variable "$input" got invalid value ["A",null,"B"].' + + '\nIn element #1: Expected "String!", found null.', + locations: [ { line: 2, column: 17 } ], + path: undefined, + } + ] }); }); @@ -867,18 +856,17 @@ describe('Execute: Handles inputs', () => { const ast = parse(doc); const vars = { input: { list: [ 'A', 'B' ] } }; - let caughtError; - try { - execute(schema, ast, null, null, vars); - } catch (error) { - caughtError = error; - } - - expect(caughtError).to.containSubset({ - locations: [ { line: 2, column: 25 } ], - message: - 'Variable "$input" expected value of type "TestType!" which cannot ' + - 'be used as an input type.' + const result = await execute(schema, ast, null, null, vars); + expect(result).to.deep.equal({ + errors: [ + { + message: + 'Variable "$input" expected value of type "TestType!" which ' + + 'cannot be used as an input type.', + locations: [ { line: 2, column: 25 } ], + path: undefined, + } + ] }); }); @@ -891,18 +879,17 @@ describe('Execute: Handles inputs', () => { const ast = parse(doc); const vars = { input: 'whoknows' }; - let caughtError; - try { - execute(schema, ast, null, null, vars); - } catch (error) { - caughtError = error; - } - - expect(caughtError).to.containSubset({ - locations: [ { line: 2, column: 25 } ], - message: - 'Variable "$input" expected value of type "UnknownType!" which ' + - 'cannot be used as an input type.' + const result = await execute(schema, ast, null, null, vars); + expect(result).to.deep.equal({ + errors: [ + { + message: + 'Variable "$input" expected value of type "UnknownType!" which ' + + 'cannot be used as an input type.', + locations: [ { line: 2, column: 25 } ], + path: undefined, + } + ] }); }); @@ -915,7 +902,7 @@ describe('Execute: Handles inputs', () => { fieldWithDefaultArgumentValue }`); - return expect(await execute(schema, ast)).to.deep.equal({ + expect(await execute(schema, ast)).to.deep.equal({ data: { fieldWithDefaultArgumentValue: '"Hello World"' } @@ -927,7 +914,7 @@ describe('Execute: Handles inputs', () => { fieldWithDefaultArgumentValue(input: $optional) }`); - return expect(await execute(schema, ast)).to.deep.equal({ + expect(await execute(schema, ast)).to.deep.equal({ data: { fieldWithDefaultArgumentValue: '"Hello World"' } @@ -939,7 +926,7 @@ describe('Execute: Handles inputs', () => { fieldWithDefaultArgumentValue(input: WRONG_TYPE) }`); - return expect(await execute(schema, ast)).to.deep.equal({ + expect(await execute(schema, ast)).to.deep.equal({ data: { fieldWithDefaultArgumentValue: null }, diff --git a/src/execution/execute.js b/src/execution/execute.js index 91e81d87eb..78a170de28 100644 --- a/src/execution/execute.js +++ b/src/execution/execute.js @@ -156,16 +156,29 @@ export function execute( // If a valid context cannot be created due to incorrect arguments, // this will throw an error. - const context = buildExecutionContext( + assertValidExecutionArguments( schema, document, - rootValue, - contextValue, - variableValues, - operationName, - fieldResolver + variableValues ); + // If a valid context cannot be created due to incorrect arguments, + // a "Response" with only errors is returned. + let context; + try { + context = buildExecutionContext( + schema, + document, + rootValue, + contextValue, + variableValues, + operationName, + fieldResolver + ); + } catch (error) { + return Promise.resolve({ errors: [ error ] }); + } + // Return a Promise that will eventually resolve to the data described by // The "Response" section of the GraphQL specification. // @@ -173,20 +186,12 @@ export function execute( // field and its descendants will be omitted, and sibling fields will still // be executed. An execution which encounters errors will still result in a // resolved Promise. - return new Promise(resolve => { - resolve(executeOperation(context, context.operation, rootValue)); - }).then(undefined, error => { - // Errors from sub-fields of a NonNull type may propagate to the top level, - // at which point we still log the error and null the parent field, which - // in this case is the entire response. - context.errors.push(error); - return null; - }).then(data => { - if (!context.errors.length) { - return { data }; - } - return { errors: context.errors, data }; - }); + return Promise.resolve( + executeOperation(context, context.operation, rootValue) + ).then(data => context.errors.length === 0 ? + { data } : + { errors: context.errors, data } + ); } /** @@ -213,6 +218,32 @@ export function addPath(prev: ResponsePath, key: string | number) { return { prev, key }; } +/** + * Essential assertions before executing to provide developer feedback for + * improper use of the GraphQL library. + */ +export function assertValidExecutionArguments( + schema: GraphQLSchema, + document: DocumentNode, + rawVariableValues: ?{[key: string]: mixed} +): void { + invariant(schema, 'Must provide schema'); + invariant(document, 'Must provide document'); + invariant( + schema instanceof GraphQLSchema, + 'Schema must be an instance of GraphQLSchema. Also ensure that there are ' + + 'not multiple versions of GraphQL installed in your node_modules directory.' + ); + + // Variables, if provided, must be an object. + invariant( + !rawVariableValues || typeof rawVariableValues === 'object', + 'Variables must be provided as an Object where each property is a ' + + 'variable value. Perhaps look to see if an unparsed JSON string ' + + 'was provided.' + ); +} + /** * Constructs a ExecutionContext object from the arguments passed to * execute, which we will pass throughout the other execution methods. @@ -302,7 +333,7 @@ function executeOperation( exeContext: ExecutionContext, operation: OperationDefinitionNode, rootValue: mixed -): {[key: string]: mixed} { +): ?{[key: string]: mixed} { const type = getOperationRootType(exeContext.schema, operation); const fields = collectFields( exeContext, @@ -314,10 +345,27 @@ function executeOperation( const path = undefined; - if (operation.operation === 'mutation') { - return executeFieldsSerially(exeContext, type, rootValue, path, fields); + // Errors from sub-fields of a NonNull type may propagate to the top level, + // at which point we still log the error and null the parent field, which + // in this case is the entire response. + // + // Similar to completeValueCatchingError. + try { + const result = operation.operation === 'mutation' ? + executeFieldsSerially(exeContext, type, rootValue, path, fields) : + executeFields(exeContext, type, rootValue, path, fields); + const promise = getPromise(result); + if (promise) { + return promise.then(undefined, error => { + exeContext.errors.push(error); + return Promise.resolve(null); + }); + } + return result; + } catch (error) { + exeContext.errors.push(error); + return null; } - return executeFields(exeContext, type, rootValue, path, fields); } /** diff --git a/src/graphql.js b/src/graphql.js index a138dbe74e..40d6844a6e 100644 --- a/src/graphql.js +++ b/src/graphql.js @@ -89,24 +89,31 @@ export function graphql( } return new Promise(resolve => { - const document = parse(source); + // Parse + let document; + try { + document = parse(source); + } catch (syntaxError) { + return resolve({ errors: [ syntaxError ]}); + } + + // Validate const validationErrors = validate(schema, document); if (validationErrors.length > 0) { - resolve({ errors: validationErrors }); - } else { - resolve( - execute( - schema, - document, - rootValue, - contextValue, - variableValues, - operationName, - fieldResolver - ) - ); + return resolve({ errors: validationErrors }); } - }).then(undefined, error => { - return { errors: [ error ] }; + + // Execute + resolve( + execute( + schema, + document, + rootValue, + contextValue, + variableValues, + operationName, + fieldResolver + ) + ); }); } diff --git a/src/language/__tests__/parser-test.js b/src/language/__tests__/parser-test.js index 55ea6ac903..29768e0231 100644 --- a/src/language/__tests__/parser-test.js +++ b/src/language/__tests__/parser-test.js @@ -17,6 +17,16 @@ import { join } from 'path'; describe('Parser', () => { + it('asserts that a source to parse was provided', () => { + expect(() => parse()).to.throw('Must provide Source. Received: undefined'); + }); + + it('asserts that a source to parse was provided', () => { + expect( + () => parse({}) + ).to.throw('Must provide Source. Received: [object Object]'); + }); + it('parse provides useful errors', () => { let caughtError; diff --git a/src/language/parser.js b/src/language/parser.js index b649eb50da..ea4ac0f68c 100644 --- a/src/language/parser.js +++ b/src/language/parser.js @@ -141,6 +141,9 @@ export function parse( options?: ParseOptions ): DocumentNode { const sourceObj = typeof source === 'string' ? new Source(source) : source; + if (!(sourceObj instanceof Source)) { + throw new TypeError('Must provide Source. Received: ' + String(sourceObj)); + } const lexer = createLexer(sourceObj, options || {}); return parseDocument(lexer); }