From f3109c3952828cb9e8c2be9cfea93f000069cbd6 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 27 Mar 2025 17:33:45 +0000 Subject: [PATCH 1/7] Implement onError proposal --- src/error/ErrorBehavior.ts | 9 +++++ src/error/index.ts | 1 + src/execution/__tests__/executor-test.ts | 2 ++ src/execution/execute.ts | 45 ++++++++++++++++++++++-- src/graphql.ts | 13 +++++++ src/index.ts | 1 + src/type/definition.ts | 3 ++ 7 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 src/error/ErrorBehavior.ts diff --git a/src/error/ErrorBehavior.ts b/src/error/ErrorBehavior.ts new file mode 100644 index 0000000000..665f241905 --- /dev/null +++ b/src/error/ErrorBehavior.ts @@ -0,0 +1,9 @@ +export type GraphQLErrorBehavior = 'PROPAGATE' | 'NO_PROPAGATE' | 'ABORT'; + +export function isErrorBehavior( + onError: unknown, +): onError is GraphQLErrorBehavior { + return ( + onError === 'PROPAGATE' || onError === 'NO_PROPAGATE' || onError === 'ABORT' + ); +} diff --git a/src/error/index.ts b/src/error/index.ts index 7e5d267f50..b9da3e897e 100644 --- a/src/error/index.ts +++ b/src/error/index.ts @@ -9,3 +9,4 @@ export type { export { syntaxError } from './syntaxError'; export { locatedError } from './locatedError'; +export type { GraphQLErrorBehavior } from './ErrorBehavior'; diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts index c758d3e426..80be5a9ff0 100644 --- a/src/execution/__tests__/executor-test.ts +++ b/src/execution/__tests__/executor-test.ts @@ -263,6 +263,7 @@ describe('Execute: Handles basic execution tasks', () => { 'rootValue', 'operation', 'variableValues', + 'errorBehavior', ); const operation = document.definitions[0]; @@ -275,6 +276,7 @@ describe('Execute: Handles basic execution tasks', () => { schema, rootValue, operation, + errorBehavior: 'PROPAGATE', }); const field = operation.selectionSet.selections[0]; diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 55c22ea9de..30cdbc1612 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -13,6 +13,8 @@ import { promiseForObject } from '../jsutils/promiseForObject'; import type { PromiseOrValue } from '../jsutils/PromiseOrValue'; import { promiseReduce } from '../jsutils/promiseReduce'; +import type { GraphQLErrorBehavior } from '../error/ErrorBehavior'; +import { isErrorBehavior } from '../error/ErrorBehavior'; import type { GraphQLFormattedError } from '../error/GraphQLError'; import { GraphQLError } from '../error/GraphQLError'; import { locatedError } from '../error/locatedError'; @@ -115,6 +117,7 @@ export interface ExecutionContext { typeResolver: GraphQLTypeResolver; subscribeFieldResolver: GraphQLFieldResolver; errors: Array; + errorBehavior: GraphQLErrorBehavior; } /** @@ -130,6 +133,7 @@ export interface ExecutionResult< > { errors?: ReadonlyArray; data?: TData | null; + onError?: GraphQLErrorBehavior; extensions?: TExtensions; } @@ -152,6 +156,15 @@ export interface ExecutionArgs { fieldResolver?: Maybe>; typeResolver?: Maybe>; subscribeFieldResolver?: Maybe>; + /** + * Experimental. Set to NO_PROPAGATE to prevent error propagation. Set to ABORT to + * abort a request when any error occurs. + * + * Default: PROPAGATE + * + * @experimental + */ + onError?: GraphQLErrorBehavior; } /** @@ -286,8 +299,17 @@ export function buildExecutionContext( fieldResolver, typeResolver, subscribeFieldResolver, + onError, } = args; + if (onError != null && !isErrorBehavior(onError)) { + return [ + new GraphQLError( + 'Unsupported `onError` value; supported values are `PROPAGATE`, `NO_PROPAGATE` and `ABORT`.', + ), + ]; + } + let operation: OperationDefinitionNode | undefined; const fragments: ObjMap = Object.create(null); for (const definition of document.definitions) { @@ -347,6 +369,7 @@ export function buildExecutionContext( typeResolver: typeResolver ?? defaultTypeResolver, subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver, errors: [], + errorBehavior: onError ?? 'PROPAGATE', }; } @@ -585,6 +608,7 @@ export function buildResolveInfo( rootValue: exeContext.rootValue, operation: exeContext.operation, variableValues: exeContext.variableValues, + errorBehavior: exeContext.errorBehavior, }; } @@ -593,10 +617,25 @@ function handleFieldError( returnType: GraphQLOutputType, exeContext: ExecutionContext, ): null { - // If the field type is non-nullable, then it is resolved without any - // protection from errors, however it still properly locates the error. - if (isNonNullType(returnType)) { + if (exeContext.errorBehavior === 'PROPAGATE') { + // If the field type is non-nullable, then it is resolved without any + // protection from errors, however it still properly locates the error. + // Note: semantic non-null types are treated as nullable for the purposes + // of error handling. + if (isNonNullType(returnType)) { + throw error; + } + } else if (exeContext.errorBehavior === 'ABORT') { + // In this mode, any error aborts the request throw error; + } else if (exeContext.errorBehavior === 'NO_PROPAGATE') { + // In this mode, the client takes responsibility for error handling, so we + // treat the field as if it were nullable. + } else { + invariant( + false, + 'Unexpected errorBehavior setting: ' + inspect(exeContext.errorBehavior), + ); } // Otherwise, error protection is applied, logging the error and resolving diff --git a/src/graphql.ts b/src/graphql.ts index bc6fb9bb72..7edd260b83 100644 --- a/src/graphql.ts +++ b/src/graphql.ts @@ -3,6 +3,8 @@ import { isPromise } from './jsutils/isPromise'; import type { Maybe } from './jsutils/Maybe'; import type { PromiseOrValue } from './jsutils/PromiseOrValue'; +import type { GraphQLErrorBehavior } from './error/ErrorBehavior'; + import { parse } from './language/parser'; import type { Source } from './language/source'; @@ -66,6 +68,15 @@ export interface GraphQLArgs { operationName?: Maybe; fieldResolver?: Maybe>; typeResolver?: Maybe>; + /** + * Experimental. Set to NO_PROPAGATE to prevent error propagation. Set to ABORT to + * abort a request when any error occurs. + * + * Default: PROPAGATE + * + * @experimental + */ + onError?: GraphQLErrorBehavior; } export function graphql(args: GraphQLArgs): Promise { @@ -106,6 +117,7 @@ function graphqlImpl(args: GraphQLArgs): PromiseOrValue { operationName, fieldResolver, typeResolver, + onError, } = args; // Validate Schema @@ -138,5 +150,6 @@ function graphqlImpl(args: GraphQLArgs): PromiseOrValue { operationName, fieldResolver, typeResolver, + onError, }); } diff --git a/src/index.ts b/src/index.ts index 73c713a203..4df70d7d74 100644 --- a/src/index.ts +++ b/src/index.ts @@ -395,6 +395,7 @@ export { } from './error/index'; export type { + GraphQLErrorBehavior, GraphQLErrorOptions, GraphQLFormattedError, GraphQLErrorExtensions, diff --git a/src/type/definition.ts b/src/type/definition.ts index 7eaac560dc..61c57c4f38 100644 --- a/src/type/definition.ts +++ b/src/type/definition.ts @@ -14,6 +14,7 @@ import type { PromiseOrValue } from '../jsutils/PromiseOrValue'; import { suggestionList } from '../jsutils/suggestionList'; import { toObjMap } from '../jsutils/toObjMap'; +import type { GraphQLErrorBehavior } from '../error/ErrorBehavior'; import { GraphQLError } from '../error/GraphQLError'; import type { @@ -988,6 +989,8 @@ export interface GraphQLResolveInfo { readonly rootValue: unknown; readonly operation: OperationDefinitionNode; readonly variableValues: { [variable: string]: unknown }; + /** @experimental */ + readonly errorBehavior: GraphQLErrorBehavior; } /** From a4cec5c45524992aa20b59b715f0f920d736de38 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 27 Mar 2025 17:52:34 +0000 Subject: [PATCH 2/7] Add tests --- src/execution/__tests__/executor-test.ts | 192 +++++++++++++++++++++++ 1 file changed, 192 insertions(+) diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts index 80be5a9ff0..27a628a195 100644 --- a/src/execution/__tests__/executor-test.ts +++ b/src/execution/__tests__/executor-test.ts @@ -287,6 +287,70 @@ describe('Execute: Handles basic execution tasks', () => { }); }); + it('reflects onError:NO_PROPAGATE via errorBehavior', () => { + let resolvedInfo; + const testType = new GraphQLObjectType({ + name: 'Test', + fields: { + test: { + type: GraphQLString, + resolve(_val, _args, _ctx, info) { + resolvedInfo = info; + }, + }, + }, + }); + const schema = new GraphQLSchema({ query: testType }); + + const document = parse('query ($var: String) { result: test }'); + const rootValue = { root: 'val' }; + const variableValues = { var: 'abc' }; + + executeSync({ + schema, + document, + rootValue, + variableValues, + onError: 'NO_PROPAGATE', + }); + + expect(resolvedInfo).to.include({ + errorBehavior: 'NO_PROPAGATE', + }); + }); + + it('reflects onError:ABORT via errorBehavior', () => { + let resolvedInfo; + const testType = new GraphQLObjectType({ + name: 'Test', + fields: { + test: { + type: GraphQLString, + resolve(_val, _args, _ctx, info) { + resolvedInfo = info; + }, + }, + }, + }); + const schema = new GraphQLSchema({ query: testType }); + + const document = parse('query ($var: String) { result: test }'); + const rootValue = { root: 'val' }; + const variableValues = { var: 'abc' }; + + executeSync({ + schema, + document, + rootValue, + variableValues, + onError: 'ABORT', + }); + + expect(resolvedInfo).to.include({ + errorBehavior: 'ABORT', + }); + }); + it('populates path correctly with complex types', () => { let path; const someObject = new GraphQLObjectType({ @@ -741,6 +805,134 @@ describe('Execute: Handles basic execution tasks', () => { }); }); + it('Full response path is included for non-nullable fields with onError:NO_PROPAGATE', () => { + const A: GraphQLObjectType = new GraphQLObjectType({ + name: 'A', + fields: () => ({ + nullableA: { + type: A, + resolve: () => ({}), + }, + nonNullA: { + type: new GraphQLNonNull(A), + resolve: () => ({}), + }, + throws: { + type: new GraphQLNonNull(GraphQLString), + resolve: () => { + throw new Error('Catch me if you can'); + }, + }, + }), + }); + const schema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'query', + fields: () => ({ + nullableA: { + type: A, + resolve: () => ({}), + }, + }), + }), + }); + + const document = parse(` + query { + nullableA { + aliasedA: nullableA { + nonNullA { + anotherA: nonNullA { + throws + } + } + } + } + } + `); + + const result = executeSync({ schema, document, onError: 'NO_PROPAGATE' }); + expectJSON(result).toDeepEqual({ + data: { + nullableA: { + aliasedA: { + nonNullA: { + anotherA: { + throws: null, + }, + }, + }, + }, + }, + errors: [ + { + message: 'Catch me if you can', + locations: [{ line: 7, column: 17 }], + path: ['nullableA', 'aliasedA', 'nonNullA', 'anotherA', 'throws'], + }, + ], + }); + }); + + it('Full response path is included for non-nullable fields with onError:ABORT', () => { + const A: GraphQLObjectType = new GraphQLObjectType({ + name: 'A', + fields: () => ({ + nullableA: { + type: A, + resolve: () => ({}), + }, + nonNullA: { + type: new GraphQLNonNull(A), + resolve: () => ({}), + }, + throws: { + type: new GraphQLNonNull(GraphQLString), + resolve: () => { + throw new Error('Catch me if you can'); + }, + }, + }), + }); + const schema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'query', + fields: () => ({ + nullableA: { + type: A, + resolve: () => ({}), + }, + }), + }), + }); + + const document = parse(` + query { + nullableA { + aliasedA: nullableA { + nonNullA { + anotherA: nonNullA { + throws + } + } + } + } + } + `); + + const result = executeSync({ schema, document, onError: 'ABORT' }); + expectJSON(result).toDeepEqual({ + data: null, + errors: [ + { + message: 'Catch me if you can', + locations: [{ line: 7, column: 17 }], + path: ['nullableA', 'aliasedA', 'nonNullA', 'anotherA', 'throws'], + }, + ], + }); + }); + it('uses the inline operation if no operation name is provided', () => { const schema = new GraphQLSchema({ query: new GraphQLObjectType({ From 947b0408558d87b0abcf7c7ef14d946b082c767c Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 27 Mar 2025 17:58:07 +0000 Subject: [PATCH 3/7] Test invalid onError is handled --- src/execution/__tests__/executor-test.ts | 30 ++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts index 27a628a195..8826060e8c 100644 --- a/src/execution/__tests__/executor-test.ts +++ b/src/execution/__tests__/executor-test.ts @@ -933,6 +933,36 @@ describe('Execute: Handles basic execution tasks', () => { }); }); + it('raises request error with invalid onError', () => { + const schema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'query', + fields: () => ({ + a: { + type: GraphQLInt, + resolve: () => ({}), + }, + }), + }), + }); + + const document = parse('{ a }'); + const result = executeSync({ + schema, + document, + // @ts-expect-error + onError: 'DANCE', + }); + expectJSON(result).toDeepEqual({ + errors: [ + { + message: + 'Unsupported `onError` value; supported values are `PROPAGATE`, `NO_PROPAGATE` and `ABORT`.', + }, + ], + }); + }); + it('uses the inline operation if no operation name is provided', () => { const schema = new GraphQLSchema({ query: new GraphQLObjectType({ From 1bcc31d243398fe19f6fdc5addefdf9bbeb94cae Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 27 Mar 2025 18:02:32 +0000 Subject: [PATCH 4/7] Ignore invariant from code coverage --- src/execution/execute.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 30cdbc1612..277656f98a 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -632,6 +632,7 @@ function handleFieldError( // In this mode, the client takes responsibility for error handling, so we // treat the field as if it were nullable. } else { + /* c8 ignore next 4 */ invariant( false, 'Unexpected errorBehavior setting: ' + inspect(exeContext.errorBehavior), From 83386564267f94082aba81c68827510eca70f853 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 27 Mar 2025 18:03:44 +0000 Subject: [PATCH 5/7] Finickity --- src/execution/execute.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 277656f98a..30e3ea9448 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -632,7 +632,7 @@ function handleFieldError( // In this mode, the client takes responsibility for error handling, so we // treat the field as if it were nullable. } else { - /* c8 ignore next 4 */ + /* c8 ignore next 5 */ invariant( false, 'Unexpected errorBehavior setting: ' + inspect(exeContext.errorBehavior), From c8fdfbadc7475c52b70e2c4ae44d23d4f89c4984 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 27 Mar 2025 18:06:20 +0000 Subject: [PATCH 6/7] Urghhhhhh --- src/execution/execute.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 30e3ea9448..65953098c1 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -631,8 +631,8 @@ function handleFieldError( } else if (exeContext.errorBehavior === 'NO_PROPAGATE') { // In this mode, the client takes responsibility for error handling, so we // treat the field as if it were nullable. + /* c8 ignore next 6 */ } else { - /* c8 ignore next 5 */ invariant( false, 'Unexpected errorBehavior setting: ' + inspect(exeContext.errorBehavior), From 1cff42169aaa149c22408c03e099443b887bd229 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 27 Mar 2025 18:09:25 +0000 Subject: [PATCH 7/7] Remove unnecessary resolver causing coverage issue --- src/execution/__tests__/executor-test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts index 8826060e8c..e8cdc0b39a 100644 --- a/src/execution/__tests__/executor-test.ts +++ b/src/execution/__tests__/executor-test.ts @@ -940,7 +940,6 @@ describe('Execute: Handles basic execution tasks', () => { fields: () => ({ a: { type: GraphQLInt, - resolve: () => ({}), }, }), }),