From 0f33b29cf58595f6bf4abf65843ed0abce134f45 Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Mon, 11 Oct 2021 23:00:47 +0300 Subject: [PATCH] execute: Correctly report missing root type error --- src/execution/__tests__/executor-test.ts | 38 +++++++++--- src/execution/execute.ts | 76 +++++++++++++----------- 2 files changed, 71 insertions(+), 43 deletions(-) diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts index 29332bffc2..0439563d20 100644 --- a/src/execution/__tests__/executor-test.ts +++ b/src/execution/__tests__/executor-test.ts @@ -917,18 +917,42 @@ describe('Execute: Handles basic execution tasks', () => { subscription S { __typename } `); - // FIXME: errors should be wrapped into ExecutionResult - expect(() => + expectJSON( executeSync({ schema, document, operationName: 'Q' }), - ).to.throw('Schema is not configured to execute query operation.'); + ).to.deep.equal({ + data: null, + errors: [ + { + message: 'Schema is not configured to execute query operation.', + locations: [{ line: 2, column: 7 }], + }, + ], + }); - expect(() => + expectJSON( executeSync({ schema, document, operationName: 'M' }), - ).to.throw('Schema is not configured to execute mutation operation.'); + ).to.deep.equal({ + data: null, + errors: [ + { + message: 'Schema is not configured to execute mutation operation.', + locations: [{ line: 3, column: 7 }], + }, + ], + }); - expect(() => + expectJSON( executeSync({ schema, document, operationName: 'S' }), - ).to.throw('Schema is not configured to execute subscription operation.'); + ).to.deep.equal({ + data: null, + errors: [ + { + message: + 'Schema is not configured to execute subscription operation.', + locations: [{ line: 4, column: 7 }], + }, + ], + }); }); it('correct field ordering despite execution order', async () => { diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 7fbab33bfc..d25d62facb 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -185,8 +185,27 @@ export function execute(args: ExecutionArgs): PromiseOrValue { // 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. - const data = executeOperation(exeContext, exeContext.operation, rootValue); - return buildResponse(exeContext, data); + // + // 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. + try { + const { operation } = exeContext; + const result = executeOperation(exeContext, operation, rootValue); + if (isPromise(result)) { + return result.then( + (data) => buildResponse(data, exeContext.errors), + (error) => { + exeContext.errors.push(error); + return buildResponse(null, exeContext.errors); + }, + ); + } + return buildResponse(result, exeContext.errors); + } catch (error) { + exeContext.errors.push(error); + return buildResponse(null, exeContext.errors); + } } /** @@ -210,15 +229,10 @@ export function executeSync(args: ExecutionArgs): ExecutionResult { * response defined by the "Response" section of the GraphQL specification. */ function buildResponse( - exeContext: ExecutionContext, - data: PromiseOrValue | null>, -): PromiseOrValue { - if (isPromise(data)) { - return data.then((resolved) => buildResponse(exeContext, resolved)); - } - return exeContext.errors.length === 0 - ? { data } - : { errors: exeContext.errors, data }; + data: ObjMap | null, + errors: ReadonlyArray, +): ExecutionResult { + return errors.length === 0 ? { data } : { errors, data }; } /** @@ -349,33 +363,23 @@ function executeOperation( rootType, operation.selectionSet, ); - const path = undefined; - // 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. - try { - const result = - operation.operation === 'mutation' - ? executeFieldsSerially( - exeContext, - rootType, - rootValue, - path, - rootFields, - ) - : executeFields(exeContext, rootType, rootValue, path, rootFields); - if (isPromise(result)) { - return result.then(undefined, (error) => { - exeContext.errors.push(error); - return null; - }); - } - return result; - } catch (error) { - exeContext.errors.push(error); - return null; + switch (operation.operation) { + case 'query': + return executeFields(exeContext, rootType, rootValue, path, rootFields); + case 'mutation': + return executeFieldsSerially( + exeContext, + rootType, + rootValue, + path, + rootFields, + ); + case 'subscription': + // TODO: deprecate `subscribe` and move all logic here + // Temporary solution until we finish merging execute and subscribe together + return executeFields(exeContext, rootType, rootValue, path, rootFields); } }