Skip to content
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "throw on executing undefined root field",
"packageName": "@graphitation/supermassive",
"email": "dsamsonov@microsoft.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,23 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`executeWithoutSchema - regression tests Executing mutation without resolver should return error 1`] = `
{
"data": null,
"errors": [
[GraphQLError: Resolver for Mutation.downloadFile is missing],
],
}
`;

exports[`executeWithoutSchema - regression tests Executing mutation without type definitions return error 1`] = `
{
"data": null,
"errors": [
[GraphQLError: Type definition for Mutation.downloadFile is missing],
],
}
`;

exports[`graphql-js snapshot check to ensure test stability Advanced Default value 1`] = `
{
"data": {
Expand Down
47 changes: 47 additions & 0 deletions packages/supermassive/src/__tests__/execute.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -490,4 +490,51 @@ describe("executeWithoutSchema - regression tests", () => {
expect(infoAtCallTime?.fieldNodes[0].name.value).toEqual("foo");
expect(infoAtCallTime?.fieldNodes[1].name.value).toEqual("foo");
});

test("Executing mutation without type definitions return error", async () => {
const schemaFragment = {
schemaId: "test",
definitions: { types: {} },
resolvers: {},
};
const document = parse(
`mutation DownloadFile { downloadFile { downloadUrl } }`,
);

const result = await executeWithoutSchema({
document,
schemaFragment,
});

// Note: there used to be a discrepancy in the results based on whether schemaFramentLoader provided or not.
const result1 = await executeWithoutSchema({
document,
schemaFragment,
schemaFragmentLoader: (currentFragment) =>
Promise.resolve({ mergedFragment: currentFragment }),
});

expect(result).toStrictEqual(result1);
expect(result).toMatchSnapshot();
});

test("Executing mutation without resolver should return error", async () => {
const schemaFragment = {
schemaId: "test",
definitions: encodeASTSchema(
parse(`type Mutation { downloadFile: DownloadFileResult }`),
)[0],
resolvers: {},
};
const document = parse(
`mutation DownloadFile { downloadFile { downloadUrl } }`,
);

const result = await executeWithoutSchema({
document,
schemaFragment,
});

expect(result).toMatchSnapshot();
});
});
39 changes: 39 additions & 0 deletions packages/supermassive/src/executeWithoutSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,10 @@ function executeFieldsSerially(
}
if (isPromise(result)) {
return result.then((resolvedResult: unknown) => {
if (resolvedResult === undefined) {
return results;
}

results[responseName] = resolvedResult;
return results;
});
Expand Down Expand Up @@ -566,8 +570,10 @@ function executeField(
fieldName,
});
if (!loading) {
throwIfRootField(parentTypeName, fieldName, fieldGroup, "typeDefinition");
return undefined;
}

return loading.then(() => {
const fieldDef = Definitions.getField(
exeContext.schemaFragment.definitions,
Expand All @@ -585,10 +591,31 @@ function executeField(
incrementalDataRecord,
);
}

throwIfRootField(parentTypeName, fieldName, fieldGroup, "typeDefinition");
return undefined;
});
}

function throwIfRootField(
parentTypeName: string,
fieldName: string,
fieldGroup: FieldGroup,
validationTarget: "typeDefinition" | "resolver",
) {
const isRootField =
parentTypeName === "Mutation" || parentTypeName === "Query";
Comment on lines +606 to +607
Copy link
Contributor

Choose a reason for hiding this comment

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

Subscription? Alternatively we can just check path.length === 0 once

Copy link
Contributor Author

@user1736 user1736 Mar 9, 2026

Choose a reason for hiding this comment

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

Subscriptions have a separate execution and this function is not called there.

As for the path, it's probably not what you think it is. For example, for new test cases I get:

{
  "prev": undefined,
  "key": "downloadFile",
  "typename": "Mutation"
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, right, path is an array is in graphql-js, we changed it to linked list for perf I guess. In our case it's just !path.prev. But whatever, this works too.

if (isRootField) {
const message =
validationTarget === "typeDefinition"
? `Type definition for ${parentTypeName}.${fieldName} is missing`
: validationTarget === "resolver"
? `Resolver for ${parentTypeName}.${fieldName} is missing`
: "Unexpected validation target";
throw locatedError(new Error(message), fieldGroup);
}
}

function requestSchemaFragment(
exeContext: ExecutionContext,
request: SchemaFragmentRequest,
Expand Down Expand Up @@ -999,6 +1026,18 @@ function resolveAndCompleteField(

const isDefaultResolverUsed =
resolveFn === exeContext.fieldResolver || fieldName === "__typename";

if (resolveFn === exeContext.fieldResolver && typeof source === "undefined") {
/**
* Resolving a root field with default resolver makes operation look succsessful, even though nothing was actually done.
* This leads to problems that are hard to debug, especially for mutations.
*
* NOTE1: executing Mutation.__typename or Query.__typename with default resolver is fine
* NOTE2: source check is required to account for systems like Grats that work exclusively on default resolvers
*/
throwIfRootField(parentTypeName, fieldName, fieldGroup, "resolver");
}

const hooks = exeContext.fieldExecutionHooks;
let hookContext: unknown = undefined;

Expand Down