diff --git a/change/@graphitation-supermassive-05461de2-3077-420e-ac4b-37fda806def5.json b/change/@graphitation-supermassive-05461de2-3077-420e-ac4b-37fda806def5.json new file mode 100644 index 000000000..95841262a --- /dev/null +++ b/change/@graphitation-supermassive-05461de2-3077-420e-ac4b-37fda806def5.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "throw on executing undefined root field", + "packageName": "@graphitation/supermassive", + "email": "dsamsonov@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/supermassive/src/__tests__/__snapshots__/execute.test.ts.snap b/packages/supermassive/src/__tests__/__snapshots__/execute.test.ts.snap index d08cdee54..fcae726fc 100644 --- a/packages/supermassive/src/__tests__/__snapshots__/execute.test.ts.snap +++ b/packages/supermassive/src/__tests__/__snapshots__/execute.test.ts.snap @@ -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": { diff --git a/packages/supermassive/src/__tests__/execute.test.ts b/packages/supermassive/src/__tests__/execute.test.ts index dad4ed272..9519d2d55 100644 --- a/packages/supermassive/src/__tests__/execute.test.ts +++ b/packages/supermassive/src/__tests__/execute.test.ts @@ -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(); + }); }); diff --git a/packages/supermassive/src/executeWithoutSchema.ts b/packages/supermassive/src/executeWithoutSchema.ts index 06bb01f25..0286b0232 100644 --- a/packages/supermassive/src/executeWithoutSchema.ts +++ b/packages/supermassive/src/executeWithoutSchema.ts @@ -460,6 +460,10 @@ function executeFieldsSerially( } if (isPromise(result)) { return result.then((resolvedResult: unknown) => { + if (resolvedResult === undefined) { + return results; + } + results[responseName] = resolvedResult; return results; }); @@ -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, @@ -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"; + 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, @@ -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;