Skip to content

Commit ce5fb23

Browse files
committed
execute: Correctly report missing root type error
1 parent 71c7a14 commit ce5fb23

File tree

2 files changed

+70
-43
lines changed

2 files changed

+70
-43
lines changed

src/execution/__tests__/executor-test.ts

+30-7
Original file line numberDiff line numberDiff line change
@@ -917,18 +917,41 @@ describe('Execute: Handles basic execution tasks', () => {
917917
subscription S { __typename }
918918
`);
919919

920-
// FIXME: errors should be wrapped into ExecutionResult
921-
expect(() =>
920+
expectJSON(
922921
executeSync({ schema, document, operationName: 'Q' }),
923-
).to.throw('Schema is not configured to execute query operation.');
922+
).to.deep.equal({
923+
data: null,
924+
errors: [
925+
{
926+
message: 'Schema is not configured to execute query operation.',
927+
locations: [{ line: 2, column: 7 }],
928+
},
929+
],
930+
});
924931

925-
expect(() =>
932+
expectJSON(
926933
executeSync({ schema, document, operationName: 'M' }),
927-
).to.throw('Schema is not configured to execute mutation operation.');
934+
).to.deep.equal({
935+
data: null,
936+
errors: [
937+
{
938+
message: 'Schema is not configured to execute mutation operation.',
939+
locations: [{ line: 3, column: 7 }],
940+
},
941+
],
942+
});
928943

929-
expect(() =>
944+
expectJSON(
930945
executeSync({ schema, document, operationName: 'S' }),
931-
).to.throw('Schema is not configured to execute subscription operation.');
946+
).to.deep.equal({
947+
data: null,
948+
errors: [
949+
{
950+
message: 'Schema is not configured to execute subscription operation.',
951+
locations: [{ line: 4, column: 7 }],
952+
},
953+
],
954+
});
932955
});
933956

934957
it('correct field ordering despite execution order', async () => {

src/execution/execute.ts

+40-36
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,27 @@ export function execute(args: ExecutionArgs): PromiseOrValue<ExecutionResult> {
185185
// field and its descendants will be omitted, and sibling fields will still
186186
// be executed. An execution which encounters errors will still result in a
187187
// resolved Promise.
188-
const data = executeOperation(exeContext, exeContext.operation, rootValue);
189-
return buildResponse(exeContext, data);
188+
//
189+
// Errors from sub-fields of a NonNull type may propagate to the top level,
190+
// at which point we still log the error and null the parent field, which
191+
// in this case is the entire response.
192+
try {
193+
const { operation } = exeContext;
194+
const result = executeOperation(exeContext, operation, rootValue);
195+
if (isPromise(result)) {
196+
return result.then(
197+
(data) => buildResponse(data, exeContext.errors),
198+
(error) => {
199+
exeContext.errors.push(error);
200+
return buildResponse(null, exeContext.errors);
201+
},
202+
);
203+
}
204+
return buildResponse(result, exeContext.errors);
205+
} catch (error) {
206+
exeContext.errors.push(error);
207+
return buildResponse(null, exeContext.errors);
208+
}
190209
}
191210

192211
/**
@@ -210,15 +229,10 @@ export function executeSync(args: ExecutionArgs): ExecutionResult {
210229
* response defined by the "Response" section of the GraphQL specification.
211230
*/
212231
function buildResponse(
213-
exeContext: ExecutionContext,
214-
data: PromiseOrValue<ObjMap<unknown> | null>,
215-
): PromiseOrValue<ExecutionResult> {
216-
if (isPromise(data)) {
217-
return data.then((resolved) => buildResponse(exeContext, resolved));
218-
}
219-
return exeContext.errors.length === 0
220-
? { data }
221-
: { errors: exeContext.errors, data };
232+
data: ObjMap<unknown> | null,
233+
errors: ReadonlyArray<GraphQLError>,
234+
): ExecutionResult {
235+
return errors.length === 0 ? { data } : { errors, data };
222236
}
223237

224238
/**
@@ -349,33 +363,23 @@ function executeOperation(
349363
rootType,
350364
operation.selectionSet,
351365
);
352-
353366
const path = undefined;
354367

355-
// Errors from sub-fields of a NonNull type may propagate to the top level,
356-
// at which point we still log the error and null the parent field, which
357-
// in this case is the entire response.
358-
try {
359-
const result =
360-
operation.operation === 'mutation'
361-
? executeFieldsSerially(
362-
exeContext,
363-
rootType,
364-
rootValue,
365-
path,
366-
rootFields,
367-
)
368-
: executeFields(exeContext, rootType, rootValue, path, rootFields);
369-
if (isPromise(result)) {
370-
return result.then(undefined, (error) => {
371-
exeContext.errors.push(error);
372-
return null;
373-
});
374-
}
375-
return result;
376-
} catch (error) {
377-
exeContext.errors.push(error);
378-
return null;
368+
switch (operation.operation) {
369+
case 'query':
370+
return executeFields(exeContext, rootType, rootValue, path, rootFields);
371+
case 'mutation':
372+
return executeFieldsSerially(
373+
exeContext,
374+
rootType,
375+
rootValue,
376+
path,
377+
rootFields,
378+
);
379+
case 'subscription':
380+
// TODO: deprecate `subscribe` and move all logic here
381+
// Temporary solution until we finish merging execute and subscribe together
382+
return executeFields(exeContext, rootType, rootValue, path, rootFields);
379383
}
380384
}
381385

0 commit comments

Comments
 (0)