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..1458edd510 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -114,7 +114,46 @@ export interface ExecutionContext { fieldResolver: GraphQLFieldResolver; typeResolver: GraphQLTypeResolver; subscribeFieldResolver: GraphQLFieldResolver; - errors: Array; + collectedErrors: CollectedErrors; +} + +/** + * @internal + */ +class CollectedErrors { + private _errorPositions: Set; + private _errors: Array; + constructor() { + this._errorPositions = new Set(); + this._errors = []; + } + + get errors(): ReadonlyArray { + return this._errors; + } + + add(error: GraphQLError, path: Path | undefined) { + // Do not modify errors list if the execution position for this error or + // any of its ancestors has already been nulled via error propagation. + // This check should be unnecessary for implementations able to implement + // actual cancellation. + if (this._hasNulledPosition(path)) { + return; + } + this._errorPositions.add(path); + this._errors.push(error); + } + + private _hasNulledPosition(startPath: Path | undefined): boolean { + let path = startPath; + while (path !== undefined) { + if (this._errorPositions.has(path)) { + return true; + } + path = path.prev; + } + return this._errorPositions.has(undefined); + } } /** @@ -206,17 +245,17 @@ export function execute(args: ExecutionArgs): PromiseOrValue { const result = executeOperation(exeContext, operation, rootValue); if (isPromise(result)) { return result.then( - (data) => buildResponse(data, exeContext.errors), + (data) => buildResponse(data, exeContext.collectedErrors.errors), (error) => { - exeContext.errors.push(error); - return buildResponse(null, exeContext.errors); + exeContext.collectedErrors.add(error, undefined); + return buildResponse(null, exeContext.collectedErrors.errors); }, ); } - return buildResponse(result, exeContext.errors); + return buildResponse(result, exeContext.collectedErrors.errors); } catch (error) { - exeContext.errors.push(error); - return buildResponse(null, exeContext.errors); + exeContext.collectedErrors.add(error, undefined); + return buildResponse(null, exeContext.collectedErrors.errors); } } @@ -352,7 +391,7 @@ export function buildExecutionContext( fieldResolver: fieldResolver ?? defaultFieldResolver, typeResolver: typeResolver ?? defaultTypeResolver, subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver, - errors: [], + collectedErrors: new CollectedErrors(), }; } @@ -558,13 +597,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 +636,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 @@ -607,7 +647,7 @@ function handleFieldError( // Otherwise, error protection is applied, logging the error and resolving // a null value for this field if one is encountered. - exeContext.errors.push(error); + exeContext.collectedErrors.add(error, path); return null; } @@ -779,13 +819,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); } });