Skip to content

Implement onError proposal (but with ErrorBehavior rather than __ErrorBehavior) #4384

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 18 commits into
base: 16.x.x
Choose a base branch
from
Draft
1 change: 1 addition & 0 deletions src/__tests__/starWarsIntrospection-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ describe('Star Wars Introspection Tests', () => {
{ name: 'Droid' },
{ name: 'Query' },
{ name: 'Boolean' },
{ name: 'ErrorBehavior' },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unsure whether we should expose this as a schema property, this is in fact an internal property that tells us just how a GraphQL API will behave. Most GraphQL code I've seen treats it as such where when walking the introspection there is an explicit check for properties starting with __ to be skipped.

{ name: '__Schema' },
{ name: '__Type' },
{ name: '__TypeKind' },
Expand Down
7 changes: 7 additions & 0 deletions src/error/ErrorBehavior.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export type ErrorBehavior = 'NO_PROPAGATE' | 'PROPAGATE' | 'ABORT';

export function isErrorBehavior(onError: unknown): onError is ErrorBehavior {
return (
onError === 'NO_PROPAGATE' || onError === 'PROPAGATE' || onError === 'ABORT'
);
}
1 change: 1 addition & 0 deletions src/error/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ export type {
export { syntaxError } from './syntaxError';

export { locatedError } from './locatedError';
export type { ErrorBehavior } from './ErrorBehavior';
223 changes: 223 additions & 0 deletions src/execution/__tests__/executor-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ describe('Execute: Handles basic execution tasks', () => {
'rootValue',
'operation',
'variableValues',
'errorBehavior',
);

const operation = document.definitions[0];
Expand All @@ -276,6 +277,7 @@ describe('Execute: Handles basic execution tasks', () => {
schema,
rootValue,
operation,
errorBehavior: 'PROPAGATE',
});

const field = operation.selectionSet.selections[0];
Expand All @@ -286,6 +288,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({
Expand Down Expand Up @@ -740,6 +806,163 @@ 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('raises request error with invalid onError', () => {
const schema = new GraphQLSchema({
query: new GraphQLObjectType({
name: 'query',
fields: () => ({
a: {
type: GraphQLInt,
},
}),
}),
});

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 `NO_PROPAGATE`, `PROPAGATE` and `ABORT`.',
},
],
});
});

it('uses the inline operation if no operation name is provided', () => {
const schema = new GraphQLSchema({
query: new GraphQLObjectType({
Expand Down
45 changes: 42 additions & 3 deletions src/execution/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import { promiseForObject } from '../jsutils/promiseForObject';
import type { PromiseOrValue } from '../jsutils/PromiseOrValue';
import { promiseReduce } from '../jsutils/promiseReduce';

import type { ErrorBehavior } from '../error/ErrorBehavior';
import { isErrorBehavior } from '../error/ErrorBehavior';
import type { GraphQLFormattedError } from '../error/GraphQLError';
import { GraphQLError } from '../error/GraphQLError';
import { locatedError } from '../error/locatedError';
Expand Down Expand Up @@ -115,6 +117,7 @@ export interface ExecutionContext {
typeResolver: GraphQLTypeResolver<any, any>;
subscribeFieldResolver: GraphQLFieldResolver<any, any>;
errors: Array<GraphQLError>;
errorBehavior: ErrorBehavior;
}

/**
Expand Down Expand Up @@ -152,6 +155,15 @@ export interface ExecutionArgs {
fieldResolver?: Maybe<GraphQLFieldResolver<any, any>>;
typeResolver?: Maybe<GraphQLTypeResolver<any, any>>;
subscribeFieldResolver?: Maybe<GraphQLFieldResolver<any, any>>;
/**
* Experimental. Set to NO_PROPAGATE to prevent error propagation. Set to ABORT to
* abort a request when any error occurs.
*
* Default: PROPAGATE
*
* @experimental
*/
onError?: ErrorBehavior;
/** Additional execution options. */
options?: {
/** Set the maximum number of errors allowed for coercing (defaults to 50). */
Expand Down Expand Up @@ -291,9 +303,18 @@ export function buildExecutionContext(
fieldResolver,
typeResolver,
subscribeFieldResolver,
onError,
options,
} = args;

if (onError != null && !isErrorBehavior(onError)) {
return [
new GraphQLError(
'Unsupported `onError` value; supported values are `NO_PROPAGATE`, `PROPAGATE` and `ABORT`.',
),
];
}

let operation: OperationDefinitionNode | undefined;
const fragments: ObjMap<FragmentDefinitionNode> = Object.create(null);
for (const definition of document.definitions) {
Expand Down Expand Up @@ -353,6 +374,7 @@ export function buildExecutionContext(
typeResolver: typeResolver ?? defaultTypeResolver,
subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver,
errors: [],
errorBehavior: onError ?? schema.defaultErrorBehavior,
};
}

Expand Down Expand Up @@ -591,6 +613,7 @@ export function buildResolveInfo(
rootValue: exeContext.rootValue,
operation: exeContext.operation,
variableValues: exeContext.variableValues,
errorBehavior: exeContext.errorBehavior,
};
}

Expand All @@ -599,10 +622,26 @@ 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.
/* c8 ignore next 6 */
} else {
invariant(
false,
'Unexpected errorBehavior setting: ' + inspect(exeContext.errorBehavior),
);
}

// Otherwise, error protection is applied, logging the error and resolving
Expand Down
Loading