Skip to content

fix: handle more unhandled promise rejections #3761

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

Open
wants to merge 1 commit into
base: next
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 98 additions & 9 deletions src/execution/__tests__/lists-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,17 @@ describe('Execute: Accepts async iterables as list value', () => {

function completeObjectList(
resolve: GraphQLFieldResolver<{ index: number }, unknown>,
nonNullable = false,
): PromiseOrValue<ExecutionResult> {
const ObjectWrapperType = new GraphQLObjectType({
name: 'ObjectWrapper',
fields: {
index: {
type: new GraphQLNonNull(GraphQLString),
resolve,
},
},
});
const schema = new GraphQLSchema({
query: new GraphQLObjectType({
name: 'Query',
Expand All @@ -101,15 +111,9 @@ describe('Execute: Accepts async iterables as list value', () => {
yield await Promise.resolve({ index: 2 });
},
type: new GraphQLList(
new GraphQLObjectType({
name: 'ObjectWrapper',
fields: {
index: {
type: new GraphQLNonNull(GraphQLString),
resolve,
},
},
}),
nonNullable
? new GraphQLNonNull(ObjectWrapperType)
: ObjectWrapperType,
),
},
},
Expand Down Expand Up @@ -216,6 +220,27 @@ describe('Execute: Accepts async iterables as list value', () => {
],
});
});

it('Handles mixture of synchronous and asynchronous errors from `completeValue` in AsyncIterables', async () => {
expectJSON(
await completeObjectList(({ index }) => {
if (index === 0) {
return Promise.reject(new Error('bad'));
}
throw new Error('also bad');
}, true),
).toDeepEqual({
data: { listField: null },
errors: [
{
message: 'also bad',
locations: [{ line: 1, column: 15 }],
path: ['listField', 1, 'index'],
},
],
});
});

it('Handles nulls yielded by async generator', async () => {
async function* listField() {
yield await Promise.resolve(1);
Expand Down Expand Up @@ -265,6 +290,11 @@ describe('Execute: Handles list nullability', () => {
expectJSON(await executeQuery(promisify(listOfPromises))).toDeepEqual(
result,
);

// Test mix of synchronous and non-synchronous values
const [first, ...rest] = listField;
const listOfSomePromises = [first, ...rest.map(promisify)];
expectJSON(await executeQuery(listOfSomePromises)).toDeepEqual(result);
}
return result;

Expand Down Expand Up @@ -322,6 +352,32 @@ describe('Execute: Handles list nullability', () => {
});
});

it('Contains multiple nulls', async () => {
const listField = [null, null, 2];
const errors = [
{
message: 'Cannot return null for non-nullable field Query.listField.',
locations: [{ line: 1, column: 3 }],
path: ['listField', 0],
},
];

expect(await complete({ listField, as: '[Int]' })).to.deep.equal({
data: { listField: [null, null, 2] },
});
expect(await complete({ listField, as: '[Int]!' })).to.deep.equal({
data: { listField: [null, null, 2] },
});
expectJSON(await complete({ listField, as: '[Int!]' })).toDeepEqual({
data: { listField: null },
errors,
});
expectJSON(await complete({ listField, as: '[Int!]!' })).toDeepEqual({
data: null,
errors,
});
});

it('Returns null', async () => {
const listField = null;
const errors = [
Expand Down Expand Up @@ -376,6 +432,39 @@ describe('Execute: Handles list nullability', () => {
});
});

it('Contains multiple errors', async () => {
const listField = [new Error('bad'), new Error('also bad'), 2];

const firstError = {
message: 'bad',
locations: [{ line: 1, column: 3 }],
path: ['listField', 0],
};

const secondError = {
message: 'also bad',
locations: [{ line: 1, column: 3 }],
path: ['listField', 1],
};

expectJSON(await complete({ listField, as: '[Int]' })).toDeepEqual({
data: { listField: [null, null, 2] },
errors: [firstError, secondError],
});
expectJSON(await complete({ listField, as: '[Int]!' })).toDeepEqual({
data: { listField: [null, null, 2] },
errors: [firstError, secondError],
});
expectJSON(await complete({ listField, as: '[Int!]' })).toDeepEqual({
data: { listField: null },
errors: [firstError],
});
expectJSON(await complete({ listField, as: '[Int!]!' })).toDeepEqual({
data: null,
errors: [firstError],
});
});

it('Results in error', async () => {
const listField = new Error('bad');
const errors = [
Expand Down
203 changes: 118 additions & 85 deletions src/execution/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1029,59 +1029,69 @@ async function completeAsyncIteratorValue(
let containsPromise = false;
const completedResults: Array<unknown> = [];
let index = 0;
// eslint-disable-next-line no-constant-condition
while (true) {
if (
stream &&
typeof stream.initialCount === 'number' &&
index >= stream.initialCount
) {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
executeStreamIterator(
index,
iterator,
exeContext,
fieldNodes,
info,
itemType,
path,
stream.label,
asyncPayloadRecord,
);
break;
}
try {
// eslint-disable-next-line no-constant-condition
while (true) {
if (
stream &&
typeof stream.initialCount === 'number' &&
index >= stream.initialCount
) {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
executeStreamIterator(
index,
iterator,
exeContext,
fieldNodes,
info,
itemType,
path,
stream.label,
asyncPayloadRecord,
);
break;
}

const itemPath = addPath(path, index, undefined);
let iteration;
try {
// eslint-disable-next-line no-await-in-loop
iteration = await iterator.next();
if (iteration.done) {
const itemPath = addPath(path, index, undefined);
let iteration;
try {
// eslint-disable-next-line no-await-in-loop
iteration = await iterator.next();
if (iteration.done) {
break;
}
} catch (rawError) {
const error = locatedError(rawError, fieldNodes, pathToArray(itemPath));
completedResults.push(handleFieldError(error, itemType, errors));
break;
}
} catch (rawError) {
const error = locatedError(rawError, fieldNodes, pathToArray(itemPath));
completedResults.push(handleFieldError(error, itemType, errors));
break;
}

if (
completeListItemValue(
iteration.value,
completedResults,
errors,
exeContext,
itemType,
fieldNodes,
info,
itemPath,
asyncPayloadRecord,
)
) {
containsPromise = true;
if (
completeListItemValue(
iteration.value,
completedResults,
errors,
exeContext,
itemType,
fieldNodes,
info,
itemPath,
asyncPayloadRecord,
)
) {
containsPromise = true;
}
index += 1;
}
index += 1;
} catch (error) {
if (containsPromise) {
return Promise.all(completedResults).finally(() => {
throw error;
});
}
throw error;
}

return containsPromise ? Promise.all(completedResults) : completedResults;
}

Expand Down Expand Up @@ -1129,48 +1139,71 @@ function completeListValue(
let previousAsyncPayloadRecord = asyncPayloadRecord;
const completedResults: Array<unknown> = [];
let index = 0;
for (const item of result) {
// No need to modify the info object containing the path,
// since from here on it is not ever accessed by resolver functions.
const itemPath = addPath(path, index, undefined);
const iterator = result[Symbol.iterator]();
try {
let iteration = iterator.next();
while (!iteration.done) {
const item = iteration.value;
// No need to modify the info object containing the path,
// since from here on it is not ever accessed by resolver functions.
const itemPath = addPath(path, index, undefined);

if (
stream &&
typeof stream.initialCount === 'number' &&
index >= stream.initialCount
) {
previousAsyncPayloadRecord = executeStreamField(
path,
itemPath,
item,
exeContext,
fieldNodes,
info,
itemType,
stream.label,
previousAsyncPayloadRecord,
);
index++;
iteration = iterator.next();
continue;
}

if (
completeListItemValue(
item,
completedResults,
errors,
exeContext,
itemType,
fieldNodes,
info,
itemPath,
asyncPayloadRecord,
)
) {
containsPromise = true;
}

if (
stream &&
typeof stream.initialCount === 'number' &&
index >= stream.initialCount
) {
previousAsyncPayloadRecord = executeStreamField(
path,
itemPath,
item,
exeContext,
fieldNodes,
info,
itemType,
stream.label,
previousAsyncPayloadRecord,
);
index++;
continue;
iteration = iterator.next();
}

if (
completeListItemValue(
item,
completedResults,
errors,
exeContext,
itemType,
fieldNodes,
info,
itemPath,
asyncPayloadRecord,
)
) {
containsPromise = true;
} catch (error) {
let iteration = iterator.next();
while (!iteration.done) {
const item = iteration.value;
if (isPromise(item)) {
containsPromise = true;
completedResults.push(item);
}
iteration = iterator.next();
}

index++;
if (containsPromise) {
return Promise.all(completedResults).finally(() => {
throw error;
});
}
throw error;
}

return containsPromise ? Promise.all(completedResults) : completedResults;
Expand Down