diff --git a/src/execution/__tests__/nonnull-test.ts b/src/execution/__tests__/nonnull-test.ts index 427f2a64d6..bb0de995a6 100644 --- a/src/execution/__tests__/nonnull-test.ts +++ b/src/execution/__tests__/nonnull-test.ts @@ -2,6 +2,9 @@ import { expect } from 'chai'; import { describe, it } from 'mocha'; import { expectJSON } from '../../__testUtils__/expectJSON'; +import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick'; + +import { invariant } from '../../jsutils/invariant'; import { parse } from '../../language/parser'; @@ -524,6 +527,143 @@ describe('Execute: handles non-nullable types', () => { }); }); + describe('Handles multiple errors for a single response position', () => { + it('nullable and non-nullable root fields throw nested errors', async () => { + const query = ` + { + promiseNonNullNest { + syncNonNull + } + promiseNest { + syncNonNull + } + } + `; + const result = await executeQuery(query, throwingData); + + expectJSON(result).toDeepEqual({ + data: null, + errors: [ + { + message: syncNonNullError.message, + path: ['promiseNest', 'syncNonNull'], + locations: [{ line: 7, column: 13 }], + }, + { + message: syncNonNullError.message, + path: ['promiseNonNullNest', 'syncNonNull'], + locations: [{ line: 4, column: 13 }], + }, + ], + }); + }); + + it('a nullable root field throws a slower nested error after a non-nullable root field throws a nested error', async () => { + const query = ` + { + promiseNonNullNest { + syncNonNull + } + promiseNest { + promiseNonNull + } + } + `; + const result = await executeQuery(query, throwingData); + + expectJSON(result).toDeepEqual({ + data: null, + errors: [ + { + message: syncNonNullError.message, + path: ['promiseNonNullNest', 'syncNonNull'], + locations: [{ line: 4, column: 13 }], + }, + ], + }); + + // allow time for slower error to reject + invariant(result.errors !== undefined); + const initialErrors = [...result.errors]; + for (let i = 0; i < 5; i++) { + // eslint-disable-next-line no-await-in-loop + await resolveOnNextTick(); + } + expectJSON(initialErrors).toDeepEqual(result.errors); + }); + + it('nullable and non-nullable nested fields throw nested errors', async () => { + const query = ` + { + syncNest { + promiseNonNullNest { + syncNonNull + } + promiseNest { + syncNonNull + } + } + } + `; + const result = await executeQuery(query, throwingData); + + expectJSON(result).toDeepEqual({ + data: { syncNest: null }, + errors: [ + { + message: syncNonNullError.message, + path: ['syncNest', 'promiseNest', 'syncNonNull'], + locations: [{ line: 8, column: 15 }], + }, + { + message: syncNonNullError.message, + path: ['syncNest', 'promiseNonNullNest', 'syncNonNull'], + locations: [{ line: 5, column: 15 }], + }, + ], + }); + }); + + it('a nullable nested field throws a slower nested error after a non-nullable nested field throws a nested error', async () => { + const query = ` + { + syncNest { + promiseNonNullNest { + syncNonNull + } + promiseNest { + promiseNest { + promiseNest { + promiseNonNull + } + } + } + } + } + `; + const result = await executeQuery(query, throwingData); + + expectJSON(result).toDeepEqual({ + data: { syncNest: null }, + errors: [ + { + message: syncNonNullError.message, + path: ['syncNest', 'promiseNonNullNest', 'syncNonNull'], + locations: [{ line: 5, column: 15 }], + }, + ], + }); + + invariant(result.errors !== undefined); + const initialErrors = [...result.errors]; + for (let i = 0; i < 20; i++) { + // eslint-disable-next-line no-await-in-loop + await resolveOnNextTick(); + } + expectJSON(initialErrors).toDeepEqual(result.errors); + }); + }); + describe('Handles non-null argument', () => { const schemaWithNonNullArg = new GraphQLSchema({ query: new GraphQLObjectType({ diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 3021354e28..bed2114c63 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -114,6 +114,7 @@ export interface ExecutionContext { fieldResolver: GraphQLFieldResolver; typeResolver: GraphQLTypeResolver; subscribeFieldResolver: GraphQLFieldResolver; + errorPositions: Set; errors: Array; } @@ -208,15 +209,19 @@ export function execute(args: ExecutionArgs): PromiseOrValue { return result.then( (data) => buildResponse(data, exeContext.errors), (error) => { - exeContext.errors.push(error); - return buildResponse(null, exeContext.errors); + const { errorPositions, errors } = exeContext; + errorPositions.add(undefined); + errors.push(error); + return buildResponse(null, errors); }, ); } return buildResponse(result, exeContext.errors); } catch (error) { - exeContext.errors.push(error); - return buildResponse(null, exeContext.errors); + const { errorPositions, errors } = exeContext; + errorPositions.add(undefined); + errors.push(error); + return buildResponse(null, errors); } } @@ -352,6 +357,7 @@ export function buildExecutionContext( fieldResolver: fieldResolver ?? defaultFieldResolver, typeResolver: typeResolver ?? defaultTypeResolver, subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver, + errorPositions: new Set(), errors: [], }; } @@ -558,13 +564,13 @@ function executeField( // to take a second callback for the error case. return completed.then(undefined, (rawError) => { const error = locatedError(rawError, fieldNodes, pathToArray(path)); - return handleFieldError(error, returnType, exeContext); + return handleFieldError(error, returnType, path, exeContext); }); } return completed; } catch (rawError) { const error = locatedError(rawError, fieldNodes, pathToArray(path)); - return handleFieldError(error, returnType, exeContext); + return handleFieldError(error, returnType, path, exeContext); } } @@ -597,6 +603,7 @@ export function buildResolveInfo( function handleFieldError( error: GraphQLError, returnType: GraphQLOutputType, + path: Path, exeContext: ExecutionContext, ): null { // If the field type is non-nullable, then it is resolved without any @@ -605,12 +612,32 @@ function handleFieldError( throw error; } + // Do not modify errors list if a response position for this error has already been nulled. + const errorPositions = exeContext.errorPositions; + if (hasNulledPosition(errorPositions, path)) { + return null; + } + errorPositions.add(path); + // Otherwise, error protection is applied, logging the error and resolving // a null value for this field if one is encountered. exeContext.errors.push(error); return null; } +function hasNulledPosition( + errorPositions: Set, + path: Path | undefined, +): boolean { + if (errorPositions.has(path)) { + return true; + } + if (path === undefined) { + return false; + } + return hasNulledPosition(errorPositions, path.prev); +} + /** * Implements the instructions for completeValue as defined in the * "Value Completion" section of the spec. @@ -779,13 +806,13 @@ function completeListValue( fieldNodes, pathToArray(itemPath), ); - return handleFieldError(error, itemType, exeContext); + return handleFieldError(error, itemType, itemPath, exeContext); }); } return completedItem; } catch (rawError) { const error = locatedError(rawError, fieldNodes, pathToArray(itemPath)); - return handleFieldError(error, itemType, exeContext); + return handleFieldError(error, itemType, itemPath, exeContext); } });