diff --git a/.changeset/cool-maps-shop.md b/.changeset/cool-maps-shop.md new file mode 100644 index 00000000000..e37ef360223 --- /dev/null +++ b/.changeset/cool-maps-shop.md @@ -0,0 +1,6 @@ +--- +'@graphql-tools/federation': patch +'@graphql-tools/delegate': patch +--- + +Fail on query planning phase if the query plan is not successful before the actual execution diff --git a/.changeset/great-toys-joke.md b/.changeset/great-toys-joke.md new file mode 100644 index 00000000000..293813a7871 --- /dev/null +++ b/.changeset/great-toys-joke.md @@ -0,0 +1,44 @@ +--- +'@graphql-tools/executor': minor +--- + +Ability to create critical errors that prevents to return a partial results + +```ts +import { CRITICAL_ERROR } from '@graphql-tools/executor'; + +const schema = makeExecutableSchema({ + typeDefs: ` + type Query { + hello: String + } + `, + resolvers: { + Query: { + hello: () => new GraphQLError('Critical error', { + extensions: { + [CRITICAL_ERROR]: true + } + }) + } + } +}); +``` + +This will prevent to return a partial results and will return an error instead. + +```ts +const result = await execute({ + schema, + document: parse(`{ hello }`) +}); + +expect(result).toEqual({ + errors: [ + { + message: 'Critical error', + } + ], + data: null, // Instead of { hello: null } +}); +``` diff --git a/packages/delegate/src/finalizeGatewayRequest.ts b/packages/delegate/src/finalizeGatewayRequest.ts index 84bc878faec..5579c7e773d 100644 --- a/packages/delegate/src/finalizeGatewayRequest.ts +++ b/packages/delegate/src/finalizeGatewayRequest.ts @@ -22,8 +22,10 @@ import { visit, visitWithTypeInfo, } from 'graphql'; +import { CRITICAL_ERROR } from '@graphql-tools/executor'; import { ASTVisitorKeyMap, + createGraphQLError, createVariableNameGenerator, ExecutionRequest, getDefinedRootType, @@ -139,6 +141,31 @@ export function finalizeGatewayRequest( operations, ); + // Fail if the query is only the root __typename field + + if ( + newDocument.definitions.length === 1 && + newDocument.definitions[0].kind === Kind.OPERATION_DEFINITION + ) { + const operation = newDocument.definitions[0] as OperationDefinitionNode; + if ( + operation.selectionSet.selections.length === 1 && + operation.selectionSet.selections[0].kind === Kind.FIELD + ) { + const field = operation.selectionSet.selections[0] as any; + if (field.name.value === '__typename' && operation.operation === 'query') { + throw createGraphQLError( + 'Failed to create a gateway request. The query must contain at least one selection.', + { + extensions: { + [CRITICAL_ERROR]: true, + }, + }, + ); + } + } + } + const newVariables: Record = {}; if (variables != null) { for (const variableName of usedVariables) { @@ -405,9 +432,9 @@ function finalizeSelectionSet( leave: node => { const type = typeInfo.getType(); if (type == null) { - console.warn( - `Invalid type for node: ${typeInfo.getParentType()?.name}.${node.name.value}`, - ); + // console.warn( + // `Invalid type for node: ${typeInfo.getParentType()?.name}.${node.name.value}`, + // ); return null; } const namedType = getNamedType(type); diff --git a/packages/executor/src/execution/__tests__/subscribe-test.ts b/packages/executor/src/execution/__tests__/subscribe.test.ts similarity index 100% rename from packages/executor/src/execution/__tests__/subscribe-test.ts rename to packages/executor/src/execution/__tests__/subscribe.test.ts diff --git a/packages/executor/src/execution/execute.ts b/packages/executor/src/execution/execute.ts index d641bf1e23d..36f6ed02f6f 100644 --- a/packages/executor/src/execution/execute.ts +++ b/packages/executor/src/execution/execute.ts @@ -759,6 +759,8 @@ export function buildResolveInfo( }; } +export const CRITICAL_ERROR = 'CRITICAL_ERROR' as const; + function handleFieldError( error: GraphQLError, returnType: GraphQLOutputType, @@ -770,6 +772,10 @@ function handleFieldError( throw error; } + if (error.extensions?.[CRITICAL_ERROR]) { + throw error; + } + // Otherwise, error protection is applied, logging the error and resolving // a null value for this field if one is encountered. errors.push(error); diff --git a/packages/federation/src/supergraph.ts b/packages/federation/src/supergraph.ts index 0913a8077ed..a30aa2ae3e2 100644 --- a/packages/federation/src/supergraph.ts +++ b/packages/federation/src/supergraph.ts @@ -942,6 +942,11 @@ export function getStitchingOptionsFromSupergraphSdl( ) { throw createGraphQLError( `Was not able to find any options for ${node.name.value}: This shouldn't have happened.`, + { + extensions: { + CRITICAL_ERROR: true, + }, + }, ); } } diff --git a/packages/federation/test/federation-compatibility.test.ts b/packages/federation/test/federation-compatibility.test.ts index 7949ca8d924..038db1fcc68 100644 --- a/packages/federation/test/federation-compatibility.test.ts +++ b/packages/federation/test/federation-compatibility.test.ts @@ -163,35 +163,25 @@ describe('Federation Compatibility', () => { } }); it('gives the correct result', () => { - if (test.expected.errors) { - expect(result.errors).toBeInstanceOf(Array); - } else { - result.errors?.forEach(e => console.error(e)); - expect(result.errors).toBeFalsy(); - } - if (test.expected.data) { - expect(result.data).toEqual(test.expected.data); + const received = { + data: result.data ?? null, + errors: !!result.errors?.length, + }; + + const expected = { + data: test.expected.data ?? null, + errors: test.expected.errors ?? false, + }; + + try { + expect(received).toEqual(expected); + } catch (e) { + result.errors?.forEach(err => { + console.error(err); + }); + throw e; } }); - /* - if (!process.env['LEAK_TEST']) { - it('calls the subgraphs at the same number or less than Apollo GW', async () => { - try { - await apolloExecutor({ - document: parse(test.query, { noLocation: true }), - }); - } catch (e) {} - for (const subgraphName in apolloSubgraphCalls) { - if (stitchingSubgraphCalls[subgraphName] != null) { - expect(stitchingSubgraphCalls[subgraphName]).toBeLessThanOrEqual( - apolloSubgraphCalls[subgraphName], - ); - } - // If never called, that's better - } - }); - } - */ }); }); }); diff --git a/packages/federation/test/fixtures/federation-compatibility/non-resolvable-interface-object/tests.json b/packages/federation/test/fixtures/federation-compatibility/non-resolvable-interface-object/tests.json index 5cac91782fd..6c2ace45b2d 100644 --- a/packages/federation/test/fixtures/federation-compatibility/non-resolvable-interface-object/tests.json +++ b/packages/federation/test/fixtures/federation-compatibility/non-resolvable-interface-object/tests.json @@ -14,9 +14,8 @@ { "query": "\n query {\n a {\n field\n }\n }\n ", "expected": { - "data": { - "a": null - } + "data": null, + "errors": true }, "plan": "\n # NOTE\n # Query.a is only available in subgraph A.\n # Query.a resolves Node interface, but none of the object types in subgraph A implement Node.\n QueryPlan {}\n " }, @@ -69,4 +68,4 @@ }, "plan": "\n QueryPlan {\n Sequence {\n Fetch(service: \"a\") {\n {\n product {\n __typename\n id\n }\n }\n },\n Flatten(path: \"product\") {\n Fetch(service: \"b\") {\n {\n ... on Product {\n __typename\n id\n }\n } =>\n {\n ... on Product {\n __typename\n }\n }\n },\n },\n },\n }\n " } -] \ No newline at end of file +] diff --git a/packages/federation/test/managed-federation.test.ts b/packages/federation/test/managed-federation.test.ts index f0d5afccd65..4a1a90ef403 100644 --- a/packages/federation/test/managed-federation.test.ts +++ b/packages/federation/test/managed-federation.test.ts @@ -124,7 +124,7 @@ describe('Managed Federation', () => { }); expect(result).toMatchObject({ - supergraphSdl: supergraphSdl, + supergraphSdl, id: 'test-id-1', minDelaySeconds: 0.1, });