From 3686fcaafcf26ec7c8020c2214c94aaf243aafc7 Mon Sep 17 00:00:00 2001 From: Sylvain Lebresne Date: Tue, 28 Jun 2022 11:00:03 +0200 Subject: [PATCH] Cleanup error related code (#1914) We had a `GraphQLErrorArgs` type that was almost equivalent to the graphql-js `GraphQLErrorOptions`, and the reason being that the later was added recently-ish, but keeping that slight inconsistency doesn't make sense and this commit fixes. It does imply mimicking the new `GraphQLError` constructor signature which have the message first and the options, while we had put the message inside the options on our side, so this imply changing all the call sites, but it's one-off easy-if-annoying fix, so just went with it. This commit also: * removes the `error()` method from `definition.ts` and replaces it by either `assert` or a proper error. * fix a bit of code duplication. --- composition-js/src/compose.ts | 2 +- composition-js/src/merging/merge.ts | 122 +++--- docs/source/errors.md | 6 + gateway-js/CHANGELOG.md | 3 +- .../integration/configuration.test.ts | 4 +- gateway-js/src/executeQueryPlan.ts | 42 +- gateway-js/src/operationContext.ts | 12 +- internals-js/src/buildSchema.ts | 29 +- internals-js/src/coreSpec.ts | 90 ++-- internals-js/src/definitions.ts | 160 +++---- .../src/directiveAndTypeSpecification.ts | 98 ++--- internals-js/src/error.ts | 106 +++-- internals-js/src/federation.ts | 225 +++++----- internals-js/src/inaccessibleSpec.ts | 398 +++++++++--------- internals-js/src/operations.ts | 10 +- internals-js/src/schemaUpgrader.ts | 16 +- internals-js/src/supergraphs.ts | 9 +- internals-js/src/tagSpec.ts | 5 +- internals-js/src/validate.ts | 112 ++--- .../KnownTypeNamesInFederationRule.ts | 2 +- internals-js/src/values.ts | 37 +- query-planner-js/src/buildPlan.ts | 9 +- 22 files changed, 767 insertions(+), 730 deletions(-) diff --git a/composition-js/src/compose.ts b/composition-js/src/compose.ts index d94160e71..c091d9594 100644 --- a/composition-js/src/compose.ts +++ b/composition-js/src/compose.ts @@ -53,7 +53,7 @@ export function compose(subgraphs: Subgraphs): CompositionResult { const federatedQueryGraph = buildFederatedQueryGraph(supergraphSchema, false); const validationResult = validateGraphComposition(supergraphQueryGraph, federatedQueryGraph); if (validationResult.errors) { - return { errors: validationResult.errors.map(e => ERRORS.SATISFIABILITY_ERROR.err({ message: e.message })) }; + return { errors: validationResult.errors.map(e => ERRORS.SATISFIABILITY_ERROR.err(e.message)) }; } // printSchema calls validateOptions, which can throw diff --git a/composition-js/src/merging/merge.ts b/composition-js/src/merging/merge.ts index 31bf15a6a..d374687b6 100644 --- a/composition-js/src/merging/merge.ts +++ b/composition-js/src/merging/merge.ts @@ -400,7 +400,7 @@ class Merger { } if (!this.merged.schemaDefinition.rootType('query')) { - this.errors.push(ERRORS.NO_QUERIES.err({ message: "No queries found in any subgraph: a supergraph must have a query root type." })); + this.errors.push(ERRORS.NO_QUERIES.err("No queries found in any subgraph: a supergraph must have a query root type.")); } // If we already encountered errors, `this.merged` is probably incomplete. Let's not risk adding errors that @@ -508,10 +508,10 @@ class Merger { (elt, names) => `${elt} in ${names}`, (elt, names) => `${elt} in ${names}`, (distribution, nodes) => { - this.errors.push(code.err({ - message: message + joinStrings(distribution, ' and ', ' but '), - nodes - })); + this.errors.push(code.err( + message + joinStrings(distribution, ' and ', ' but '), + { nodes } + )); }, elt => !elt ); @@ -547,10 +547,10 @@ class Merger { supergraphElementPrinter, otherElementsPrinter, (distribution, nodes) => { - this.errors.push(code.err({ - message: message + distribution[0] + joinStrings(distribution.slice(1), ' and '), - nodes: nodes.concat(extraNodes ?? []) - })); + this.errors.push(code.err( + message + distribution[0] + joinStrings(distribution.slice(1), ' and '), + { nodes: nodes.concat(extraNodes ?? []) } + )); }, ignorePredicate, includeMissingSources @@ -763,10 +763,10 @@ class Merger { } if (extensionSubgraphs.length > 0 && defSubgraphs.length === 0) { for (const [i, subgraph] of extensionSubgraphs.entries()) { - this.errors.push(ERRORS.EXTENSION_WITH_NO_BASE.err({ - message: `[${subgraph}] Type "${dest}" is an extension type, but there is no type definition for "${dest}" in any subgraph.`, - nodes: extensionASTs[i], - })); + this.errors.push(ERRORS.EXTENSION_WITH_NO_BASE.err( + `[${subgraph}] Type "${dest}" is an extension type, but there is no type definition for "${dest}" in any subgraph.`, + { nodes: extensionASTs[i] }, + )); } } } @@ -929,10 +929,10 @@ class Merger { // unlikely to span multiple subgraphs. In fact, we could almost have thrown this error during subgraph validation // if this wasn't for the fact that it is only thrown for directives being merged and so is more logical to // be thrown only when merging. - this.errors.push(ERRORS.MERGED_DIRECTIVE_APPLICATION_ON_EXTERNAL.err({ - message: `[${this.names[i]}] Cannot apply merged directive ${directive} to external field "${source.coordinate}"`, - nodes: directive.sourceAST - })); + this.errors.push(ERRORS.MERGED_DIRECTIVE_APPLICATION_ON_EXTERNAL.err( + `[${this.names[i]}] Cannot apply merged directive ${directive} to external field "${source.coordinate}"`, + { nodes: directive.sourceAST }, + )); } } } @@ -1042,15 +1042,15 @@ class Merger { overridingSubgraphASTNode, )); } else if (sourceSubgraphName === subgraphName) { - this.errors.push(ERRORS.OVERRIDE_FROM_SELF_ERROR.err({ - message: `Source and destination subgraphs "${sourceSubgraphName}" are the same for overridden field "${coordinate}"`, - nodes: overrideDirective?.sourceAST, - })); + this.errors.push(ERRORS.OVERRIDE_FROM_SELF_ERROR.err( + `Source and destination subgraphs "${sourceSubgraphName}" are the same for overridden field "${coordinate}"`, + { nodes: overrideDirective?.sourceAST }, + )); } else if (subgraphsWithOverride.includes(sourceSubgraphName)) { - this.errors.push(ERRORS.OVERRIDE_SOURCE_HAS_OVERRIDE.err({ - message: `Field "${coordinate}" on subgraph "${subgraphName}" is also marked with directive @override in subgraph "${sourceSubgraphName}". Only one @override directive is allowed per field.`, - nodes: sourceASTs(overrideDirective, subgraphMap[sourceSubgraphName].overrideDirective) - })); + this.errors.push(ERRORS.OVERRIDE_SOURCE_HAS_OVERRIDE.err( + `Field "${coordinate}" on subgraph "${subgraphName}" is also marked with directive @override in subgraph "${sourceSubgraphName}". Only one @override directive is allowed per field.`, + { nodes: sourceASTs(overrideDirective, subgraphMap[sourceSubgraphName].overrideDirective) } + )); } else if (subgraphMap[sourceSubgraphName] === undefined) { this.hints.push(new CompositionHint( HINTS.OVERRIDE_DIRECTIVE_CAN_BE_REMOVED, @@ -1070,10 +1070,10 @@ class Merger { }); if (hasIncompatible) { assert(conflictingDirective !== undefined, 'conflictingDirective should not be undefined'); - this.errors.push(ERRORS.OVERRIDE_COLLISION_WITH_ANOTHER_DIRECTIVE.err({ - message: `@override cannot be used on field "${fromField?.coordinate}" on subgraph "${subgraphName}" since "${fromField?.coordinate}" on "${subgraph}" is marked with directive "@${conflictingDirective.name}"`, - nodes: sourceASTs(overrideDirective, conflictingDirective) - })); + this.errors.push(ERRORS.OVERRIDE_COLLISION_WITH_ANOTHER_DIRECTIVE.err( + `@override cannot be used on field "${fromField?.coordinate}" on subgraph "${subgraphName}" since "${fromField?.coordinate}" on "${subgraph}" is marked with directive "@${conflictingDirective.name}"`, + { nodes: sourceASTs(overrideDirective, conflictingDirective) } + )); } else { // if we get here, then the @override directive is valid // if the field being overridden is used, then we need to add an @external directive @@ -1113,10 +1113,10 @@ class Merger { if (sources.every((s, i) => s === undefined || this.isExternal(i, s))) { const definingSubgraphs = sources.map((source, i) => source ? this.names[i] : undefined).filter(s => s !== undefined) as string[]; const nodes = sources.map(source => source?.sourceAST).filter(s => s !== undefined) as ASTNode[]; - this.errors.push(ERRORS.EXTERNAL_MISSING_ON_BASE.err({ - message: `Field "${dest.coordinate}" is marked @external on all the subgraphs in which it is listed (${printSubgraphNames(definingSubgraphs)}).`, - nodes - })); + this.errors.push(ERRORS.EXTERNAL_MISSING_ON_BASE.err( + `Field "${dest.coordinate}" is marked @external on all the subgraphs in which it is listed (${printSubgraphNames(definingSubgraphs)}).`, + { nodes } + )); return; } @@ -1161,10 +1161,10 @@ class Merger { const nonShareables = shareableSources.length > 0 ? printSubgraphNames(nonShareableSources.map((s) => this.names[s])) : 'all of them'; - this.errors.push(ERRORS.INVALID_FIELD_SHARING.err({ - message: `Non-shareable field "${dest.coordinate}" is resolved from multiple subgraphs: it is resolved from ${printSubgraphNames(resolvingSubgraphs)} and defined as non-shareable in ${nonShareables}`, - nodes: sourceASTs(...allResolving), - })); + this.errors.push(ERRORS.INVALID_FIELD_SHARING.err( + `Non-shareable field "${dest.coordinate}" is resolved from multiple subgraphs: it is resolved from ${printSubgraphNames(resolvingSubgraphs)} and defined as non-shareable in ${nonShareables}`, + { nodes: sourceASTs(...allResolving) }, + )); } } @@ -1466,10 +1466,10 @@ class Merger { if (nonOptionalSources.length > 0) { const nonOptionalSubgraphs = printSubgraphNames(nonOptionalSources); const missingSources = printSubgraphNames(sources.map((s, i) => s && !s.argument(argName) ? this.names[i] : undefined).filter((s) => !!s) as string[]); - this.errors.push(ERRORS.REQUIRED_ARGUMENT_MISSING_IN_SOME_SUBGRAPH.err({ - message: `Argument "${arg.coordinate}" is required in some subgraphs but does not appear in all subgraphs: it is required in ${nonOptionalSubgraphs} but does not appear in ${missingSources}`, - nodes: sourceASTs(...sources.map((s) => s?.argument(argName))), - })); + this.errors.push(ERRORS.REQUIRED_ARGUMENT_MISSING_IN_SOME_SUBGRAPH.err( + `Argument "${arg.coordinate}" is required in some subgraphs but does not appear in all subgraphs: it is required in ${nonOptionalSubgraphs} but does not appear in ${missingSources}`, + { nodes: sourceASTs(...sources.map((s) => s?.argument(argName))) }, + )); } else { this.reportMismatchHint( HINTS.INCONSISTENT_ARGUMENT_PRESENCE, @@ -1641,10 +1641,10 @@ class Merger { // We could be left with an enum type with no values, and that's invalid in graphQL if (dest.values.length === 0) { - this.errors.push(ERRORS.EMPTY_MERGED_ENUM_TYPE.err({ - message: `None of the values of enum type "${dest}" are defined consistently in all the subgraphs defining that type. As only values common to all subgraphs are merged, this would result in an empty type.`, - nodes: sourceASTs(...sources), - })); + this.errors.push(ERRORS.EMPTY_MERGED_ENUM_TYPE.err( + `None of the values of enum type "${dest}" are defined consistently in all the subgraphs defining that type. As only values common to all subgraphs are merged, this would result in an empty type.`, + { nodes: sourceASTs(...sources) }, + )); } } @@ -1754,10 +1754,10 @@ class Merger { if (nonOptionalSources.length > 0) { const nonOptionalSubgraphs = printSubgraphNames(nonOptionalSources); const missingSources = printSubgraphNames(sources.map((s, i) => s && !s.field(name) ? this.names[i] : undefined).filter((s) => !!s) as string[]); - this.errors.push(ERRORS.REQUIRED_INPUT_FIELD_MISSING_IN_SOME_SUBGRAPH.err({ - message: `Input object field "${destField.coordinate}" is required in some subgraphs but does not appear in all subgraphs: it is required in ${nonOptionalSubgraphs} but does not appear in ${missingSources}`, - nodes: sourceASTs(...sources.map((s) => s?.field(name))), - })); + this.errors.push(ERRORS.REQUIRED_INPUT_FIELD_MISSING_IN_SOME_SUBGRAPH.err( + `Input object field "${destField.coordinate}" is required in some subgraphs but does not appear in all subgraphs: it is required in ${nonOptionalSubgraphs} but does not appear in ${missingSources}`, + { nodes: sourceASTs(...sources.map((s) => s?.field(name))) }, + )); } else { this.reportMismatchHint( HINTS.INCONSISTENT_INPUT_OBJECT_FIELD, @@ -1779,10 +1779,10 @@ class Merger { // We could be left with an input type with no fields, and that's invalid in graphQL if (!dest.hasFields()) { - this.errors.push(ERRORS.EMPTY_MERGED_INPUT_TYPE.err({ - message: `None of the fields of input object type "${dest}" are consistently defined in all the subgraphs defining that type. As only fields common to all subgraphs are merged, this would result in an empty type.`, - nodes: sourceASTs(...sources), - })); + this.errors.push(ERRORS.EMPTY_MERGED_INPUT_TYPE.err( + `None of the fields of input object type "${dest}" are consistently defined in all the subgraphs defining that type. As only fields common to all subgraphs are merged, this would result in an empty type.`, + { nodes: sourceASTs(...sources) }, + )); } } @@ -2143,14 +2143,16 @@ class Merger { const typeInSubgraph = s.type(type.name); return typeInSubgraph !== undefined && (typeInSubgraph as ObjectType | InterfaceType).implementsInterface(itf.name); }); - this.errors.push(ERRORS.INTERFACE_FIELD_NO_IMPLEM.err({ - message: `Interface field "${itfField.coordinate}" is declared in ${printSubgraphNames(subgraphsWithTheField)} but type "${type}", ` + this.errors.push(ERRORS.INTERFACE_FIELD_NO_IMPLEM.err( + `Interface field "${itfField.coordinate}" is declared in ${printSubgraphNames(subgraphsWithTheField)} but type "${type}", ` + `which implements "${itf}" only in ${printSubgraphNames(subgraphsWithTypeImplementingItf)} does not have field "${itfField.name}".`, - nodes: sourceASTs( - ...subgraphsWithTheField.map(s => this.subgraphByName(s).typeOfKind(itf.name, 'InterfaceType')?.field(itfField.name)), - ...subgraphsWithTypeImplementingItf.map(s => this.subgraphByName(s).type(type.name)) - ) - })); + { + nodes: sourceASTs( + ...subgraphsWithTheField.map(s => this.subgraphByName(s).typeOfKind(itf.name, 'InterfaceType')?.field(itfField.name)), + ...subgraphsWithTypeImplementingItf.map(s => this.subgraphByName(s).type(type.name)) + ) + } + )); continue; } diff --git a/docs/source/errors.md b/docs/source/errors.md index 6fcb2f38d..9a6e56c04 100644 --- a/docs/source/errors.md +++ b/docs/source/errors.md @@ -23,6 +23,7 @@ The following errors might be raised during composition: | `DEFAULT_VALUE_USES_INACCESSIBLE` | An element is marked as @inaccessible but is used in the default value of an element visible in the API schema. | 2.0.0 | | | `DIRECTIVE_DEFINITION_INVALID` | A built-in or federation directive has an invalid definition in the schema. | 2.0.0 | Replaces: `TAG_DEFINITION_INVALID` | | `DISALLOWED_INACCESSIBLE` | An element is marked as @inaccessible that is not allowed to be @inaccessible. | 2.0.0 | | +| `DOWNSTREAM_SERVICE_ERROR` | Indicates an error in a subgraph service query during query execution in a federated service. | 0.x | | | `EMPTY_MERGED_ENUM_TYPE` | An enum type has no value common to all the subgraphs that define the type. Merging that type would result in an invalid empty enum type. | 2.0.0 | | | `EMPTY_MERGED_INPUT_TYPE` | An input object type has no field common to all the subgraphs that define the type. Merging that type would result in an invalid empty input object type. | 2.0.0 | | | `ENUM_VALUE_MISMATCH` | An enum type that is used as both an input and output type has a value that is not defined in all the subgraphs that define the enum type. | 2.0.0 | | @@ -30,6 +31,7 @@ The following errors might be raised during composition: | `EXTERNAL_ARGUMENT_DEFAULT_MISMATCH` | An `@external` field declares an argument with a default that is incompatible with the corresponding argument in the declaration(s) of that field in other subgraphs. | 2.0.0 | | | `EXTERNAL_ARGUMENT_MISSING` | An `@external` field is missing some arguments present in the declaration(s) of that field in other subgraphs. | 2.0.0 | | | `EXTERNAL_ARGUMENT_TYPE_MISMATCH` | An `@external` field declares an argument with a type that is incompatible with the corresponding argument in the declaration(s) of that field in other subgraphs. | 2.0.0 | | +| `EXTERNAL_COLLISION_WITH_ANOTHER_DIRECTIVE` | The @external directive collides with other directives in some situations. | 2.1.0 | | | `EXTERNAL_MISSING_ON_BASE` | A field is marked as `@external` in a subgraph but with no non-external declaration in any other subgraph. | 0.x | | | `EXTERNAL_ON_INTERFACE` | The field of an interface type is marked with `@external`: as external is about marking field not resolved by the subgraph and as interface field are not resolved (only implementations of those fields are), an "external" interface field is nonsensical | 2.0.0 | | | `EXTERNAL_TYPE_MISMATCH` | An `@external` field has a type that is incompatible with the declaration(s) of that field in other subgraphs. | 0.x | | @@ -41,9 +43,11 @@ The following errors might be raised during composition: | `INPUT_FIELD_DEFAULT_MISMATCH` | An input field has a default value that is incompatible with other declarations of that field in other subgraphs. | 2.0.0 | | | `INTERFACE_FIELD_IMPLEM_TYPE_MISMATCH` | For an interface field, some of its concrete implementations have @external or @requires and there is difference in those implementations return type (which is currently not supported; see https://github.com/apollographql/federation/issues/1257) | 2.0.0 | | | `INTERFACE_FIELD_NO_IMPLEM` | After subgraph merging, an implementation is missing a field of one of the interface it implements (which can happen for valid subgraphs). | 2.0.0 | | +| `INVALID_FEDERATION_SUPERGRAPH` | Indicates that a schema provided for an Apollo Federation supergraph is not a valid supergraph schema. | 2.1.0 | | | `INVALID_FIELD_SHARING` | A field that is non-shareable in at least one subgraph is resolved by multiple subgraphs. | 2.0.0 | | | `INVALID_GRAPHQL` | A schema is invalid GraphQL: it violates one of the rule of the specification. | 2.0.0 | | | `INVALID_LINK_DIRECTIVE_USAGE` | An application of the @link directive is invalid/does not respect the specification. | 2.0.0 | | +| `INVALID_LINK_IDENTIFIER` | A url/version for a @link feature is invalid/does not respect the specification. | 2.1.0 | | | `INVALID_SUBGRAPH_NAME` | A subgraph name is invalid (subgraph names cannot be a single underscore ("_")). | 2.0.0 | | | `KEY_FIELDS_HAS_ARGS` | The `fields` argument of a `@key` directive includes a field defined with arguments (which is not currently supported). | 2.0.0 | | | `KEY_FIELDS_SELECT_INVALID_TYPE` | The `fields` argument of `@key` directive includes a field whose type is a list, interface, or union type. Fields of these types cannot be part of a `@key` | 0.x | | @@ -82,6 +86,8 @@ The following errors might be raised during composition: | `TYPE_KIND_MISMATCH` | A type has the same name in different subgraphs, but a different kind. For instance, one definition is an object type but another is an interface. | 2.0.0 | Replaces: `VALUE_TYPE_KIND_MISMATCH`, `EXTENSION_OF_WRONG_KIND`, `ENUM_MISMATCH_TYPE` | | `TYPE_WITH_ONLY_UNUSED_EXTERNAL` | A federation 1 schema has a composite type comprised only of unused external fields. Note that this error can _only_ be raised for federation 1 schema as federation 2 schema do not allow unused external fields (and errors with code EXTERNAL_UNUSED will be raised in that case). But when federation 1 schema are automatically migrated to federation 2 ones, unused external fields are automatically removed, and in rare case this can leave a type empty. If that happens, an error with this code will be raised | 2.0.0 | | | `UNKNOWN_FEDERATION_LINK_VERSION` | The version of federation in a @link directive on the schema is unknown. | 2.0.0 | | +| `UNKNOWN_LINK_VERSION` | The version of @link set on the schema is unknown. | 2.1.0 | | +| `UNSUPPORTED_FEATURE` | Indicates an error due to feature currently unsupported by federation. | 2.1.0 | | diff --git a/gateway-js/CHANGELOG.md b/gateway-js/CHANGELOG.md index 95ab7d51b..aa51d2dfc 100644 --- a/gateway-js/CHANGELOG.md +++ b/gateway-js/CHANGELOG.md @@ -2,7 +2,8 @@ This CHANGELOG pertains only to Apollo Federation packages in the 2.x range. The Federation v0.x equivalent for this package can be found [here](https://github.com/apollographql/federation/blob/version-0.x/gateway-js/CHANGELOG.md) on the `version-0.x` branch of this repo. -- Fix issue generating plan for a "diamond-shaped" dependency [PR #1900](https://github.com/apollographql/federation/pull/1900) +- Cleanup error related code, adding missing error code to a few errors [PR #1914](https://github.com/apollographql/federation/pull/1914). +- Fix issue generating plan for a "diamond-shaped" dependency [PR #1900](https://github.com/apollographql/federation/pull/1900). ## 2.1.0-alpha.0 diff --git a/gateway-js/src/__tests__/integration/configuration.test.ts b/gateway-js/src/__tests__/integration/configuration.test.ts index b63defbf7..5af71bec1 100644 --- a/gateway-js/src/__tests__/integration/configuration.test.ts +++ b/gateway-js/src/__tests__/integration/configuration.test.ts @@ -202,8 +202,8 @@ describe('gateway startup errors', () => { const expected = "A valid schema couldn't be composed. The following composition errors were found:\n" - + ' [accounts] On type "User", for @key(fields: "id"): Cannot query field "id" on type "User" (the field should be either be added to this subgraph or, if it should not be resolved by this subgraph, you need to add it to this subgraph with @external).\n' - + ' [accounts] On type "Account", for @key(fields: "id"): Cannot query field "id" on type "Account" (the field should be either be added to this subgraph or, if it should not be resolved by this subgraph, you need to add it to this subgraph with @external).' + + ' [accounts] On type "User", for @key(fields: "id"): Cannot query field "id" on type "User" (the field should either be added to this subgraph or, if it should not be resolved by this subgraph, you need to add it to this subgraph with @external).\n' + + ' [accounts] On type "Account", for @key(fields: "id"): Cannot query field "id" on type "Account" (the field should either be added to this subgraph or, if it should not be resolved by this subgraph, you need to add it to this subgraph with @external).' expect(err.message).toBe(expected); }); }); diff --git a/gateway-js/src/executeQueryPlan.ts b/gateway-js/src/executeQueryPlan.ts index e489f1241..169b2ee9a 100644 --- a/gateway-js/src/executeQueryPlan.ts +++ b/gateway-js/src/executeQueryPlan.ts @@ -14,6 +14,7 @@ import { GraphQLSchema, isObjectType, isInterfaceType, + GraphQLErrorOptions, } from 'graphql'; import { Trace, google } from 'apollo-reporting-protobuf'; import { GraphQLDataSource, GraphQLDataSourceRequestKind } from './datasources/types'; @@ -31,7 +32,7 @@ import { deepMerge } from './utilities/deepMerge'; import { isNotNullOrUndefined } from './utilities/array'; import { SpanStatusCode } from "@opentelemetry/api"; import { OpenTelemetrySpanNames, tracer } from "./utilities/opentelemetry"; -import { defaultRootName } from '@apollo/federation-internals'; +import { defaultRootName, errorCodeDef, ERRORS } from '@apollo/federation-internals'; export type ServiceMap = { [serviceName: string]: GraphQLDataSource; @@ -118,11 +119,7 @@ export async function executeQueryPlan( errors: [ new GraphQLError( error.message, - undefined, - undefined, - undefined, - undefined, - error as Error, + { originalError: error }, ) ] }; @@ -593,22 +590,25 @@ function downstreamServiceError( if (!message) { message = `Error while fetching subquery from service "${serviceName}"`; } - extensions = { - code: 'DOWNSTREAM_SERVICE_ERROR', - // XXX The presence of a serviceName in extensions is used to - // determine if this error should be captured for metrics reporting. - serviceName, - ...extensions, + + const errorOptions: GraphQLErrorOptions = { + originalError: originalError as Error, + extensions: { + ...extensions, + // XXX The presence of a serviceName in extensions is used to + // determine if this error should be captured for metrics reporting. + serviceName, + } }; - return new GraphQLError( - message, - undefined, - undefined, - undefined, - undefined, - originalError as Error, - extensions, - ); + + let codeDef = errorCodeDef(originalError); + // It's possible the orignal has a code, but not one we know about (one generated by the underlying `GraphQLDataSource`, + // which we don't control). In that case, we want to use that code (and have thus no `ErrorCodeDefinition` usable). + if (!codeDef && extensions?.code) { + return new GraphQLError(message, errorOptions); + } + // Otherwise, we either use the code we found and know, or default to a general downstream error code. + return (codeDef ?? ERRORS.DOWNSTREAM_SERVICE_ERROR).err(message, errorOptions); } export const defaultFieldResolverWithAliasSupport: GraphQLFieldResolver< diff --git a/gateway-js/src/operationContext.ts b/gateway-js/src/operationContext.ts index 737f7c809..47b9d4ab9 100644 --- a/gateway-js/src/operationContext.ts +++ b/gateway-js/src/operationContext.ts @@ -1,7 +1,7 @@ +import { ERRORS } from '@apollo/federation-internals'; import { DocumentNode, FragmentDefinitionNode, - GraphQLError, GraphQLSchema, Kind, OperationDefinitionNode, @@ -37,7 +37,7 @@ export function buildOperationContext({ case Kind.OPERATION_DEFINITION: operationCount++; if (!operationName && operationCount > 1) { - throw new GraphQLError( + throw ERRORS.INVALID_GRAPHQL.err( 'Must provide operation name if query contains ' + 'multiple operations.', ); @@ -55,11 +55,9 @@ export function buildOperationContext({ } }); if (!operation) { - if (operationName) { - throw new GraphQLError(`Unknown operation named "${operationName}".`); - } else { - throw new GraphQLError('Must provide an operation.'); - } + throw ERRORS.INVALID_GRAPHQL.err( + operationName ? `Unknown operation named "${operationName}".` : 'Must provide an operation.' + ); } return { diff --git a/internals-js/src/buildSchema.ts b/internals-js/src/buildSchema.ts index e24e5244f..390ed0158 100644 --- a/internals-js/src/buildSchema.ts +++ b/internals-js/src/buildSchema.ts @@ -55,6 +55,7 @@ import { errorCauses, NamedSchemaElement, } from "./definitions"; +import { ERRORS, withModifiedErrorNodes } from "./error"; function buildValue(value?: ValueNode): any { return value ? valueFromASTUntyped(value) : undefined; @@ -195,7 +196,7 @@ function buildNamedTypeAndDirectivesShallow(documentNode: DocumentNode, schema: switch (definitionNode.kind) { case 'OperationDefinition': case 'FragmentDefinition': - errors.push(new GraphQLError("Invalid executable definition found while building schema", definitionNode)); + errors.push(ERRORS.INVALID_GRAPHQL.err("Invalid executable definition found while building schema", { nodes: definitionNode })); continue; case 'SchemaDefinition': schemaDefinitions.push(definitionNode); @@ -220,7 +221,7 @@ function buildNamedTypeAndDirectivesShallow(documentNode: DocumentNode, schema: type = schema.addType(newNamedType(withoutTrailingDefinition(definitionNode.kind), definitionNode.name.value)); } else if (type.preserveEmptyDefinition) { // Note: we reuse the same error message than graphQL-js would output - throw new GraphQLError(`There can be only one type named "${definitionNode.name.value}"`); + throw ERRORS.INVALID_GRAPHQL.err(`There can be only one type named "${definitionNode.name.value}"`); } // It's possible for the type definition to be empty, because it is valid graphQL to have: // type Foo @@ -251,7 +252,7 @@ function buildNamedTypeAndDirectivesShallow(documentNode: DocumentNode, schema: if (!existing) { schema.addType(newNamedType(withoutTrailingDefinition(definitionNode.kind), definitionNode.name.value)); } else if (existing.isBuiltIn) { - throw new GraphQLError(`Cannot extend built-in type "${definitionNode.name.value}"`); + throw ERRORS.INVALID_GRAPHQL.err(`Cannot extend built-in type "${definitionNode.name.value}"`); } break; case 'DirectiveDefinition': @@ -281,7 +282,7 @@ function withoutTrailingDefinition(str: string): NamedTypeKind { function getReferencedType(node: NamedTypeNode, schema: Schema): NamedType { const type = schema.type(node.name.value); if (!type) { - throw new GraphQLError(`Unknown type ${node.name.value}`, node); + throw ERRORS.INVALID_GRAPHQL.err(`Unknown type ${node.name.value}`, { nodes: node }); } return type; } @@ -294,15 +295,7 @@ function withNodeAttachedToError(operation: () => void, node: ASTNode, errors: G if (causes) { for (const cause of causes) { const allNodes: ASTNode | ASTNode[] = cause.nodes ? [node, ...cause.nodes] : node; - errors.push(new GraphQLError( - cause.message, - allNodes, - cause.source, - cause.positions, - cause.path, - cause, - cause.extensions - )); + errors.push(withModifiedErrorNodes(cause, allNodes)); } } else { throw e; @@ -392,7 +385,7 @@ function buildNamedTypeInner( () => { const itfName = itfNode.name.value; if (fieldBasedType.implementsInterface(itfName)) { - throw new GraphQLError(`Type "${type}" can only implement "${itfName}" once.`); + throw ERRORS.INVALID_GRAPHQL.err(`Type "${type}" can only implement "${itfName}" once.`); } fieldBasedType.addImplementedInterface(itfName).setOfExtension(extension); }, @@ -409,7 +402,7 @@ function buildNamedTypeInner( () => { const name = namedType.name.value; if (unionType.hasTypeMember(name)) { - throw new GraphQLError(`Union type "${unionType}" can only include type "${name}" once.`); + throw ERRORS.INVALID_GRAPHQL.err(`Union type "${unionType}" can only include type "${name}" once.`); } unionType.addType(name).setOfExtension(extension); }, @@ -477,7 +470,7 @@ function validateOutputType(type: Type, what: string, node: ASTNode, errors: Gra if (isOutputType(type)) { return type; } else { - errors.push(new GraphQLError(`The type of "${what}" must be Output Type but got "${type}", a ${type.kind}.`, node)); + errors.push(ERRORS.INVALID_GRAPHQL.err(`The type of "${what}" must be Output Type but got "${type}", a ${type.kind}.`, { nodes: node })); return undefined; } } @@ -486,7 +479,7 @@ function validateInputType(type: Type, what: string, node: ASTNode, errors: Grap if (isInputType(type)) { return type; } else { - errors.push(new GraphQLError(`The type of "${what}" must be Input Type but got "${type}", a ${type.kind}.`, node)); + errors.push(ERRORS.INVALID_GRAPHQL.err(`The type of "${what}" must be Input Type but got "${type}", a ${type.kind}.`, { nodes: node })); return undefined; } } @@ -502,7 +495,7 @@ function buildTypeReferenceFromAST(typeNode: TypeNode, schema: Schema): Type { case Kind.NON_NULL_TYPE: const wrapped = buildTypeReferenceFromAST(typeNode.type, schema); if (wrapped.kind == Kind.NON_NULL_TYPE) { - throw new GraphQLError(`Cannot apply the non-null operator (!) twice to the same type`, typeNode); + throw ERRORS.INVALID_GRAPHQL.err(`Cannot apply the non-null operator (!) twice to the same type`, { nodes: typeNode }); } return new NonNullType(wrapped); default: diff --git a/internals-js/src/coreSpec.ts b/internals-js/src/coreSpec.ts index bb1fc076b..917a37948 100644 --- a/internals-js/src/coreSpec.ts +++ b/internals-js/src/coreSpec.ts @@ -1,6 +1,6 @@ import { ASTNode, DirectiveLocation, GraphQLError, StringValueNode } from "graphql"; import { URL } from "url"; -import { CoreFeature, Directive, DirectiveDefinition, EnumType, ErrGraphQLAPISchemaValidationFailed, ErrGraphQLValidationFailed, InputType, ListType, NamedType, NonNullType, ScalarType, Schema, SchemaDefinition, SchemaElement } from "./definitions"; +import { CoreFeature, Directive, DirectiveDefinition, EnumType, ErrGraphQLAPISchemaValidationFailed, ErrGraphQLValidationFailed, InputType, ListType, NamedType, NonNullType, ScalarType, Schema, SchemaDefinition, SchemaElement, sourceASTs } from "./definitions"; import { sameType } from "./types"; import { err } from '@apollo/core-schema'; import { assert, firstOf } from './utils'; @@ -185,10 +185,10 @@ export function extractCoreFeatureImports(url: FeatureUrl, directive: Directive< continue; } if (typeof elt !== 'object') { - errors.push(ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err({ - message: `Invalid sub-value ${valueToString(elt)} for @link(import:) argument: values should be either strings or input object values of the form { name: "", as: "" }.`, - nodes: directive.sourceAST - })); + errors.push(ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err( + `Invalid sub-value ${valueToString(elt)} for @link(import:) argument: values should be either strings or input object values of the form { name: "", as: "" }.`, + { nodes: directive.sourceAST }, + )); continue; } let name: string | undefined; @@ -196,28 +196,28 @@ export function extractCoreFeatureImports(url: FeatureUrl, directive: Directive< switch (key) { case 'name': if (typeof value !== 'string') { - errors.push(ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err({ - message: `Invalid value for the "name" field for sub-value ${valueToString(elt)} of @link(import:) argument: must be a string.`, - nodes: directive.sourceAST - })); + errors.push(ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err( + `Invalid value for the "name" field for sub-value ${valueToString(elt)} of @link(import:) argument: must be a string.`, + { nodes: directive.sourceAST }, + )); continue importArgLoop; } name = value; break; case 'as': if (typeof value !== 'string') { - errors.push(ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err({ - message: `Invalid value for the "as" field for sub-value ${valueToString(elt)} of @link(import:) argument: must be a string.`, - nodes: directive.sourceAST - })); + errors.push(ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err( + `Invalid value for the "as" field for sub-value ${valueToString(elt)} of @link(import:) argument: must be a string.`, + { nodes: directive.sourceAST }, + )); continue importArgLoop; } break; default: - errors.push(ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err({ - message: `Unknown field "${key}" for sub-value ${valueToString(elt)} of @link(import:) argument.`, - nodes: directive.sourceAST - })); + errors.push(ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err( + `Unknown field "${key}" for sub-value ${valueToString(elt)} of @link(import:) argument.`, + { nodes: directive.sourceAST }, + )); continue importArgLoop; } } @@ -226,24 +226,24 @@ export function extractCoreFeatureImports(url: FeatureUrl, directive: Directive< imports.push(i); if (i.as) { if (i.name.charAt(0) === '@' && i.as.charAt(0) !== '@') { - errors.push(ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err({ - message: `Invalid @link import renaming: directive "${i.name}" imported name should start with a '@' character, but got "${i.as}".`, - nodes: directive.sourceAST - })); + errors.push(ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err( + `Invalid @link import renaming: directive "${i.name}" imported name should start with a '@' character, but got "${i.as}".`, + { nodes: directive.sourceAST }, + )); } else if (i.name.charAt(0) !== '@' && i.as.charAt(0) === '@') { - errors.push(ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err({ - message: `Invalid @link import renaming: type "${i.name}" imported name should not start with a '@' character, but got "${i.as}" (or, if @${i.name} is a directive, then it should be referred to with a '@').`, - nodes: directive.sourceAST - })); + errors.push(ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err( + `Invalid @link import renaming: type "${i.name}" imported name should not start with a '@' character, but got "${i.as}" (or, if @${i.name} is a directive, then it should be referred to with a '@').`, + { nodes: directive.sourceAST }, + )); } } validateImportedName(name, knownElements, errors, directive); } else { - errors.push(ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err({ - message: `Invalid sub-value ${valueToString(elt)} for @link(import:) argument: missing mandatory "name" field.`, - nodes: directive.sourceAST - })); + errors.push(ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err( + `Invalid sub-value ${valueToString(elt)} for @link(import:) argument: missing mandatory "name" field.`, + { nodes: directive.sourceAST }, + )); } } @@ -264,10 +264,10 @@ function validateImportedName(name: string, knownElements: string[] | undefined, details = didYouMean(suggestions); } } - errors.push(ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err({ - message: `Cannot import unknown element "${name}".${details}`, - nodes: directive.sourceAST - })); + errors.push(ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err( + `Cannot import unknown element "${name}".${details}`, + { nodes: directive.sourceAST }, + )); } } @@ -419,9 +419,9 @@ export class CoreSpecDefinition extends FeatureDefinition { // Already exists with the same version, let it be. return []; } else { - return [ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err({ - message: `Cannot add feature ${this} to the schema, it already uses ${existingCore.coreItself.url}` - })]; + return [ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err( + `Cannot add feature ${this} to the schema, it already uses ${existingCore.coreItself.url}` + )]; } } @@ -555,7 +555,7 @@ export class FeatureVersion { public static parse(input: string): FeatureVersion { const match = input.match(this.VERSION_RE) if (!match) { - throw new GraphQLError(`Expected a version string (of the form v1.2), got ${input}`); + throw ERRORS.INVALID_LINK_IDENTIFIER.err(`Expected a version string (of the form v1.2), got ${input}`); } return new this(+match[1], +match[2]) } @@ -663,17 +663,17 @@ export class FeatureUrl { public static parse(input: string, node?: ASTNode): FeatureUrl { const url = new URL(input) if (!url.pathname || url.pathname === '/') { - throw new GraphQLError(`Missing path in feature url '${url}'`, node) + throw ERRORS.INVALID_LINK_IDENTIFIER.err(`Missing path in feature url '${url}'`, { nodes: node }) } const path = url.pathname.split('/') const verStr = path.pop() if (!verStr) { - throw new GraphQLError(`Missing version component in feature url '${url}'`, node) + throw ERRORS.INVALID_LINK_IDENTIFIER.err(`Missing version component in feature url '${url}'`, { nodes: node }) } const version = FeatureVersion.parse(verStr) const name = path[path.length - 1] if (!name) { - throw new GraphQLError(`Missing feature name component in feature url '${url}'`, node) + throw ERRORS.INVALID_LINK_IDENTIFIER.err(`Missing feature name component in feature url '${url}'`, { nodes: node }) } const element = url.hash ? url.hash.slice(1): undefined url.hash = '' @@ -804,11 +804,15 @@ export function removeAllCoreFeatures(schema: Schema) { for (const { feature, type, references } of typeReferences) { const referencesInSchema = references.filter(r => r.isAttached()); if (referencesInSchema.length > 0) { - errors.push(new GraphQLError( + // Note: using REFERENCED_INACCESSIBLE is slightly abusive because the reference element is not marked + // @inacessible exactly. Instead, it is inacessible due to core elements being removed, but that's very + // very close semantically. Overall, adding a publicly documented error code just to minor difference + // doesn't feel worth it, especially since that case is super unlikely in the first place (and, as + // the prior comment says, may one day be removed too). + errors.push(ERRORS.REFERENCED_INACCESSIBLE.err( `Cannot remove elements of feature ${feature} as feature type ${type}` + ` is referenced by elements: ${referencesInSchema.join(', ')}`, - references.map(r => r.sourceAST) - .filter(n => n !== undefined) as ASTNode[] + { nodes: sourceASTs(...references) }, )); } } diff --git a/internals-js/src/definitions.ts b/internals-js/src/definitions.ts index 4e0099d6b..34879b6f1 100644 --- a/internals-js/src/definitions.ts +++ b/internals-js/src/definitions.ts @@ -43,7 +43,7 @@ import { specifiedSDLRules } from "graphql/validation/specifiedRules"; import { validateSchema } from "./validate"; import { createDirectiveSpecification, createScalarTypeSpecification, DirectiveSpecification, TypeSpecification } from "./directiveAndTypeSpecification"; import { didYouMean, suggestionList } from "./suggestions"; -import { withModifiedErrorMessage } from "./error"; +import { ERRORS, withModifiedErrorMessage } from "./error"; const validationErrorCode = 'GraphQLValidationFailed'; const DEFAULT_VALIDATION_ERROR_MESSAGE = 'The schema is not a valid GraphQL schema.'; @@ -325,7 +325,7 @@ export function typeFromAST(schema: Schema, node: TypeNode): Type { default: const type = schema.type(node.name.value); if (!type) { - throw new GraphQLError(`Unknown type "${node.name.value}"`, node); + throw ERRORS.INVALID_GRAPHQL.err(`Unknown type "${node.name.value}"`, { nodes: node }); } return type; } @@ -477,9 +477,7 @@ abstract class Element | Schema | Direct // to a schema could bring a whole hierarchy of types and directives for instance). If they are attached, it only work if // it's to the same schema, but you have to check. // Overall, it's simpler to force attaching elements before you add other elements to them. - if (!this.isAttached()) { - throw error(`Cannot modify detached element ${this}`); - } + assert(this.isAttached(), () => `Cannot modify detached element ${this}`); } } @@ -534,7 +532,7 @@ export abstract class SchemaElement if (!def) { throw this.schema().blueprint.onGraphQLJSValidationError( this.schema(), - new GraphQLError(`Unknown directive "@${nameOrDef}".`) + ERRORS.INVALID_GRAPHQL.err(`Unknown directive "@${nameOrDef}".`) ); } if (Array.isArray(def)) { @@ -588,9 +586,7 @@ export abstract class SchemaElement protected abstract removeTypeReference(type: NamedType): void; protected checkRemoval() { - if (this.isElementBuiltIn() && !Schema.prototype['canModifyBuiltIn'].call(this.schema())) { - throw error(`Cannot modify built-in ${this}`); - } + assert(!this.isElementBuiltIn() || Schema.prototype['canModifyBuiltIn'].call(this.schema()), () => `Cannot modify built-in ${this}`); // We allow removals even on detached element because that doesn't particularly create issues (and we happen to do such // removals on detached internally; though of course we could refactor the code if we wanted). } @@ -601,17 +597,13 @@ export abstract class SchemaElement // Ensure this element (the modified one), is not a built-in, or part of one. let thisElement: SchemaElement | Schema | undefined = this; while (thisElement && thisElement instanceof SchemaElement) { - if (thisElement.isElementBuiltIn()) { - throw error(`Cannot modify built-in (or part of built-in) ${this}`); - } + assert(!thisElement.isElementBuiltIn(), () => `Cannot modify built-in (or part of built-in) ${this}`); thisElement = thisElement.parent; } } if (addedElement && addedElement.isAttached()) { const thatSchema = addedElement.schema(); - if (thatSchema && thatSchema != this.schema()) { - throw error(`Cannot add element ${addedElement} to ${this} as it is attached to another schema`); - } + assert(!thatSchema || thatSchema === this.schema(), () => `Cannot add element ${addedElement} to ${this} as it is attached to another schema`); } } } @@ -677,9 +669,7 @@ abstract class BaseNamedType `Cannot add extension to type ${this}: it is already added to another type`); this._extensions.add(extension); Extension.prototype['setExtendedElement'].call(extension, this); this.onModification(); @@ -830,10 +820,6 @@ export abstract class NamedSchemaElementWithType extends Element { private _extension?: Extension; @@ -848,9 +834,7 @@ abstract class BaseExtensionMember extends setOfExtension(extension: Extension | undefined) { this.checkUpdate(); // See similar comment on FieldDefinition.setOfExtension for why we have to cast. - if (extension && !this._parent?.extensions().has(extension as any)) { - throw error(`Cannot set object as part of the provided extension: it is not an extension of parent ${this.parent}`); - } + assert(!extension || this._parent?.extensions().has(extension as any), () => `Cannot set object as part of the provided extension: it is not an extension of parent ${this.parent}`); this._extension = extension; } @@ -980,7 +964,7 @@ export class CoreFeatures { this.add(coreItself); const coreDef = findCoreSpecVersion(coreItself.url); if (!coreDef) { - throw error(`Schema uses unknown version ${coreItself.url.version} of the ${coreItself.url.name} spec`); + throw ERRORS.UNKNOWN_LINK_VERSION.err(`Schema uses unknown version ${coreItself.url.version} of the ${coreItself.url.name} spec`); } this.coreDefinition = coreDef; } @@ -1010,7 +994,8 @@ export class CoreFeatures { const url = this.coreDefinition.extractFeatureUrl(args); const existing = this.byIdentity.get(url.identity); if (existing) { - throw error(`Duplicate inclusion of feature ${url.identity}`); + // TODO: we may want to lossen that limitation at some point. Including the same feature for 2 different major versions should be ok. + throw ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err(`Duplicate inclusion of feature ${url.identity}`); } const imports = extractCoreFeatureImports(url, typedDirective); const feature = new CoreFeature(url, args.as ?? url.name, directive, imports, args.for); @@ -1335,24 +1320,16 @@ export class Schema { const existing = this.type(type.name); if (existing) { // Like for directive, we let user shadow built-in types, but the definition must be valid. - if (existing.isBuiltIn) { - } else { - throw error(`Type ${type} already exists in this schema`); - } + assert(existing.isBuiltIn, () => `Type ${type} already exists in this schema`); } if (type.isAttached()) { // For convenience, let's not error out on adding an already added type. - if (type.parent == this) { - return type; - } - throw error(`Cannot add type ${type} to this schema; it is already attached to another schema`); + assert(type.parent == this, () => `Cannot add type ${type} to this schema; it is already attached to another schema`); + return type; } if (type.isBuiltIn) { - if (!this.isConstructed) { - this._builtInTypes.set(type.name, type); - } else { - throw error(`Cannot add built-in ${type} to this schema (built-ins can only be added at schema construction time)`); - } + assert(!this.isConstructed, `Cannot add built-in ${type} to this schema (built-ins can only be added at schema construction time)`); + this._builtInTypes.set(type.name, type); } else { this._types.set(type.name, type); } @@ -1424,22 +1401,15 @@ export class Schema { const existing = this.directive(definition.name); // Note that we allow the schema to define a built-in manually (and the manual definition will shadow the // built-in one). It's just that validation will ensure the definition ends up the one expected. - if (existing && !existing.isBuiltIn) { - throw error(`Directive ${definition} already exists in this schema`); - } + assert(!existing || existing.isBuiltIn, () => `Directive ${definition} already exists in this schema`); if (definition.isAttached()) { // For convenience, let's not error out on adding an already added directive. - if (definition.parent == this) { - return definition; - } - throw error(`Cannot add directive ${definition} to this schema; it is already attached to another schema`); + assert(definition.parent == this, () => `Cannot add directive ${definition} to this schema; it is already attached to another schema`); + return definition; } if (definition.isBuiltIn) { - if (!this.isConstructed) { - this._builtInDirectives.set(definition.name, definition); - } else { - throw error(`Cannot add built-in ${definition} to this schema (built-ins can only be added at schema construction time)`); - } + assert(!this.isConstructed, () => `Cannot add built-in ${definition} to this schema (built-ins can only be added at schema construction time)`); + this._builtInDirectives.set(definition.name, definition); } else { this._directives.set(definition.name, definition); } @@ -1525,7 +1495,8 @@ export class Schema { */ elementByCoordinate(coordinate: string): NamedSchemaElement | undefined { if (!coordinate.match(coordinateRegexp)) { - throw error(`Invalid argument "${coordinate}: it is not a syntactically valid graphQL coordinate."`); + // To be fair, graphQL coordinate is not yet officially part of the spec but well... + throw ERRORS.INVALID_GRAPHQL.err(`Invalid argument "${coordinate}: it is not a syntactically valid graphQL coordinate."`); } const argStartIdx = coordinate.indexOf('('); @@ -1538,7 +1509,7 @@ export class Schema { const isDirective = typeOrDirectiveName.startsWith('@'); if (isDirective) { if (fieldOrEnumName) { - throw error(`Invalid argument "${coordinate}: it is not a syntactically valid graphQL coordinate."`); + throw ERRORS.INVALID_GRAPHQL.err(`Invalid argument "${coordinate}: it is not a syntactically valid graphQL coordinate."`); } const directive = this.directive(typeOrDirectiveName.slice(1)); return argName ? directive?.argument(argName) : directive; @@ -1554,16 +1525,16 @@ export class Schema { return argName ? field?.argument(argName) : field; case 'InputObjectType': if (argName) { - throw error(`Invalid argument "${coordinate}: it is not a syntactically valid graphQL coordinate."`); + throw ERRORS.INVALID_GRAPHQL.err(`Invalid argument "${coordinate}: it is not a syntactically valid graphQL coordinate."`); } return type.field(fieldOrEnumName); case 'EnumType': if (argName) { - throw error(`Invalid argument "${coordinate}: it is not a syntactically valid graphQL coordinate."`); + throw ERRORS.INVALID_GRAPHQL.err(`Invalid argument "${coordinate}: it is not a syntactically valid graphQL coordinate."`); } return type.value(fieldOrEnumName); default: - throw error(`Invalid argument "${coordinate}: it is not a syntactically valid graphQL coordinate."`); + throw ERRORS.INVALID_GRAPHQL.err(`Invalid argument "${coordinate}: it is not a syntactically valid graphQL coordinate."`); } } } @@ -1603,7 +1574,7 @@ export class SchemaDefinition extends SchemaElement { const coreFeatures = schema.coreFeatures; if (isCoreSpecDirectiveApplication(applied)) { if (coreFeatures) { - throw error(`Invalid duplicate application of @core/@link`); + throw ERRORS.INVALID_LINK_DIRECTIVE_USAGE.err(`Invalid duplicate application of @core/@link`); } const schemaDirective = applied as Directive; const args = schemaDirective.arguments(); @@ -1636,9 +1607,9 @@ export class SchemaDefinition extends SchemaElement { this.checkUpdate(); const obj = this.schema().type(nameOrType); if (!obj) { - throw new GraphQLError(`Cannot set schema ${rootKind} root to unknown type ${nameOrType}`); + throw ERRORS.INVALID_GRAPHQL.err(`Cannot set schema ${rootKind} root to unknown type ${nameOrType}`); } else if (obj.kind != 'ObjectType') { - throw new GraphQLError(`${defaultRootName(rootKind)} root type must be an Object type${rootKind === 'query' ? '' : ' if provided'}, it cannot be set to ${nameOrType} (an ${obj.kind}).`); + throw ERRORS.INVALID_GRAPHQL.err(`${defaultRootName(rootKind)} root type must be an Object type${rootKind === 'query' ? '' : ' if provided'}, it cannot be set to ${nameOrType} (an ${obj.kind}).`); } toSet = new RootType(rootKind, obj); } else { @@ -1670,9 +1641,7 @@ export class SchemaDefinition extends SchemaElement { if (this._extensions.has(extension)) { return extension; } - if (extension.extendedElement) { - throw error(`Cannot add extension to this schema: extension is already added to another schema`); - } + assert(!extension.extendedElement, 'Cannot add extension to this schema: extension is already added to another schema'); this._extensions.add(extension); Extension.prototype['setExtendedElement'].call(extension, this); this.onModification(); @@ -1804,9 +1773,9 @@ abstract class FieldBasedType { this.checkUpdate(); const maybeObj = this.schema().type(nameOrTypeOrMember); if (!maybeObj) { - throw new GraphQLError(`Cannot add unknown type ${nameOrTypeOrMember} as member of union type ${this.name}`); + throw ERRORS.INVALID_GRAPHQL.err(`Cannot add unknown type ${nameOrTypeOrMember} as member of union type ${this.name}`); } else if (maybeObj.kind != 'ObjectType') { - throw new GraphQLError(`Cannot add non-object type ${nameOrTypeOrMember} (of type ${maybeObj.kind}) as member of union type ${this.name}`); + throw ERRORS.INVALID_GRAPHQL.err(`Cannot add non-object type ${nameOrTypeOrMember} (of type ${maybeObj.kind}) as member of union type ${this.name}`); } obj = maybeObj; } else { @@ -2230,10 +2199,10 @@ export class InputObjectType extends BaseNamedType extends NamedSchemaE // For some reason (bad codegen, maybe?), some users have field where a arg is defined more than one. And this doesn't seem rejected by // graphQL (?). So we accept it, but ensure the types/default values are the same. if (type && existing.type && !sameType(type, existing.type)) { - throw error(`Argument ${toAdd.name} already exists on field ${this.name} with a different type (${existing.type})`); + throw ERRORS.INVALID_GRAPHQL.err(`Argument ${toAdd.name} already exists on field ${this.name} with a different type (${existing.type})`); } if (defaultValue && (!existing.defaultValue || !valueEquals(defaultValue, existing.defaultValue))) { - throw error(`Argument ${toAdd.name} already exists on field ${this.name} with a different default value (${valueToString(existing.defaultValue)})`); + throw ERRORS.INVALID_GRAPHQL.err(`Argument ${toAdd.name} already exists on field ${this.name} with a different default value (${valueToString(existing.defaultValue)})`); } return existing; } if (type && !isInputType(type)) { - throw error(`Invalid output type ${type} for argument ${toAdd.name} of ${this}: arguments should be input types.`); + throw ERRORS.INVALID_GRAPHQL.err(`Invalid output type ${type} for argument ${toAdd.name} of ${this}: arguments should be input types.`); } this._args.set(toAdd.name, toAdd); Element.prototype['setParent'].call(toAdd, this); @@ -2416,9 +2385,10 @@ export class FieldDefinition extends NamedSchemaE this.checkUpdate(); // It seems typescript "expand" `TParent` below into `ObjectType | Interface`, so it essentially lose the context that // the `TParent` in `Extension` will always match. Hence the `as any`. - if (extension && !this._parent?.extensions().has(extension as any)) { - throw error(`Cannot mark field ${this.name} as part of the provided extension: it is not an extension of field parent type ${this.parent}`); - } + assert( + !extension || this._parent?.extensions().has(extension as any), + () => `Cannot mark field ${this.name} as part of the provided extension: it is not an extension of field parent type ${this.parent}` + ); this._extension = extension; this.onModification(); } @@ -2527,9 +2497,10 @@ export class InputFieldDefinition extends NamedSchemaElementWithType` will always match. Hence the `as any`. - if (extension && !this._parent?.extensions().has(extension as any)) { - throw error(`Cannot mark field ${this.name} as part of the provided extension: it is not an extension of field parent type ${this.parent}`); - } + assert( + !extension || this._parent?.extensions().has(extension as any), + () => `Cannot mark field ${this.name} as part of the provided extension: it is not an extension of field parent type ${this.parent}`, + ); this._extension = extension; this.onModification(); } @@ -2676,9 +2647,10 @@ export class EnumValue extends NamedSchemaElement { setOfExtension(extension: Extension | undefined) { this.checkUpdate(); - if (extension && !this._parent?.extensions().has(extension)) { - throw error(`Cannot mark field ${this.name} as part of the provided extension: it is not an extension of field parent type ${this.parent}`); - } + assert( + !extension || this._parent?.extensions().has(extension as any), + () => `Cannot mark field ${this.name} as part of the provided extension: it is not an extension of enum value parent type ${this.parent}`, + ); this._extension = extension; this.onModification(); } @@ -2767,7 +2739,7 @@ export class DirectiveDefinition `Cannot include default values for arguments: cannot find directive definition for ${this.name}`); const updated = Object.create(null); for (const argDef of definition.arguments()) { updated[argDef.name] = withDefaultValues(this._args[argDef.name], argDef); @@ -2978,13 +2948,11 @@ export class Directive< this.checkUpdate(); if (extension) { const parent = this.parent; - if (parent instanceof SchemaDefinition || parent instanceof BaseNamedType) { - if (!parent.extensions().has(extension)) { - throw error(`Cannot mark directive ${this.name} as part of the provided extension: it is not an extension of parent ${parent}`); - } - } else { - throw error(`Can only mark directive parts of extensions when directly apply to type or schema definition.`); - } + assert( + parent instanceof SchemaDefinition || parent instanceof BaseNamedType, + 'Can only mark directive parts of extensions when directly apply to type or schema definition.' + ); + assert(parent.extensions().has(extension), () => `Cannot mark directive ${this.name} as part of the provided extension: it is not an extension of parent ${parent}`); } this._extension = extension; this.onModification(); @@ -3252,7 +3220,7 @@ export function variableDefinitionsFromAST(schema: Schema, definitionNodes: read for (const definitionNode of definitionNodes) { if (!definitions.add(variableDefinitionFromAST(schema, definitionNode))) { const name = definitionNode.variable.name.value; - throw new GraphQLError(`Duplicate definition for variable ${name}`, definitionNodes.filter(n => n.variable.name.value === name)); + throw ERRORS.INVALID_GRAPHQL.err(`Duplicate definition for variable ${name}`, { nodes: definitionNodes.filter(n => n.variable.name.value === name) }); } } return definitions; @@ -3262,7 +3230,7 @@ export function variableDefinitionFromAST(schema: Schema, definitionNode: Variab const variable = new Variable(definitionNode.variable.name.value); const type = typeFromAST(schema, definitionNode.type); if (!isInputType(type)) { - throw new GraphQLError(`Invalid type "${type}" for variable $${variable}: not an input type`, definitionNode.type); + throw ERRORS.INVALID_GRAPHQL.err(`Invalid type "${type}" for variable $${variable}: not an input type`, { nodes: definitionNode.type }); } const def = new VariableDefinition( schema, diff --git a/internals-js/src/directiveAndTypeSpecification.ts b/internals-js/src/directiveAndTypeSpecification.ts index 786ae4590..d4b96abfd 100644 --- a/internals-js/src/directiveAndTypeSpecification.ts +++ b/internals-js/src/directiveAndTypeSpecification.ts @@ -117,10 +117,10 @@ export function createObjectTypeSpecification({ for (const { name, type, args } of expectedFields) { const existingField = existing.field(name); if (!existingField) { - errors = errors.concat(ERRORS.TYPE_DEFINITION_INVALID.err({ - message: `Invalid definition of type ${name}: missing field ${name}`, - nodes: existing.sourceAST - })); + errors = errors.concat(ERRORS.TYPE_DEFINITION_INVALID.err( + `Invalid definition of type ${name}: missing field ${name}`, + { nodes: existing.sourceAST }, + )); continue; } // We allow adding non-nullability because we've seen redefinition of the federation _Service type with type String! for the `sdl` field @@ -130,10 +130,10 @@ export function createObjectTypeSpecification({ existingType = existingType.ofType; } if (!sameType(type, existingType)) { - errors = errors.concat(ERRORS.TYPE_DEFINITION_INVALID.err({ - message: `Invalid definition for field ${name} of type ${name}: should have type ${type} but found type ${existingField.type}`, - nodes: existingField.sourceAST - })); + errors = errors.concat(ERRORS.TYPE_DEFINITION_INVALID.err( + `Invalid definition for field ${name} of type ${name}: should have type ${type} but found type ${existingField.type}`, + { nodes: existingField.sourceAST }, + )); } errors = errors.concat(ensureSameArguments( { name, args }, @@ -171,10 +171,10 @@ export function createUnionTypeSpecification({ const expectedMembers = membersFct(schema).sort((n1, n2) => n1.localeCompare(n2)); if (expectedMembers.length === 0) { if (existing) { - return [ERRORS.TYPE_DEFINITION_INVALID.err({ - message: `Invalid definition of type ${name}: expected the union type to not exist/have no members but it is defined.`, - nodes: existing.sourceAST - })]; + return [ERRORS.TYPE_DEFINITION_INVALID.err( + `Invalid definition of type ${name}: expected the union type to not exist/have no members but it is defined.`, + { nodes: existing.sourceAST }, + )]; } return []; } @@ -188,10 +188,10 @@ export function createUnionTypeSpecification({ // This is kind of fragile in a core schema world where members may have been renamed, but we currently // only use this one for the _Entity type where that shouldn't be an issue. if (!arrayEquals(expectedMembers, actualMembers)) { - errors = errors.concat(ERRORS.TYPE_DEFINITION_INVALID.err({ - message: `Invalid definition of type ${name}: expected members [${expectedMembers}] but found [${actualMembers}].`, - nodes: existing.sourceAST - })); + errors = errors.concat(ERRORS.TYPE_DEFINITION_INVALID.err( + `Invalid definition of type ${name}: expected members [${expectedMembers}] but found [${actualMembers}].`, + { nodes: existing.sourceAST }, + )); } return errors; } else { @@ -226,10 +226,10 @@ export function createEnumTypeSpecification({ assert(isEnumType(existing), 'Should be an enum type'); const actualValueNames = existing.values.map(v => v.name).sort((n1, n2) => n1.localeCompare(n2)); if (!arrayEquals(expectedValueNames, actualValueNames)) { - errors = errors.concat(ERRORS.TYPE_DEFINITION_INVALID.err({ - message: `Invalid definition for type "${name}": expected values [${expectedValueNames.join(', ')}] but found [${actualValueNames.join(', ')}].`, - nodes: existing.sourceAST - })); + errors = errors.concat(ERRORS.TYPE_DEFINITION_INVALID.err( + `Invalid definition for type "${name}": expected values [${expectedValueNames.join(', ')}] but found [${actualValueNames.join(', ')}].`, + { nodes: existing.sourceAST }, + )); } return errors; } else { @@ -246,10 +246,12 @@ export function createEnumTypeSpecification({ function ensureSameTypeKind(expected: NamedType['kind'], actual: NamedType): GraphQLError[] { return expected === actual.kind ? [] - : [ERRORS.TYPE_DEFINITION_INVALID.err({ - message: `Invalid definition for type ${actual.name}: ${actual.name} should be a ${expected} but is defined as a ${actual.kind}`, - nodes: actual.sourceAST - })]; + : [ + ERRORS.TYPE_DEFINITION_INVALID.err( + `Invalid definition for type ${actual.name}: ${actual.name} should be a ${expected} but is defined as a ${actual.kind}`, + { nodes: actual.sourceAST }, + ) + ]; } function ensureSameDirectiveStructure( @@ -265,17 +267,17 @@ function ensureSameDirectiveStructure( let errors = ensureSameArguments(expected, actual, `directive ${directiveName}`); // It's ok to say you'll never repeat a repeatable directive. It's not ok to repeat one that isn't. if (!expected.repeatable && actual.repeatable) { - errors = errors.concat(ERRORS.DIRECTIVE_DEFINITION_INVALID.err({ - message: `Invalid definition for directive ${directiveName}: ${directiveName} should${expected.repeatable ? "" : " not"} be repeatable`, - nodes: actual.sourceAST - })); + errors = errors.concat(ERRORS.DIRECTIVE_DEFINITION_INVALID.err( + `Invalid definition for directive ${directiveName}: ${directiveName} should${expected.repeatable ? "" : " not"} be repeatable`, + { nodes: actual.sourceAST }, + )); } // Similarly, it's ok to say that you will never use a directive in some locations, but not that you will use it in places not allowed by what is expected. if (!actual.locations.every(loc => expected.locations.includes(loc))) { - errors = errors.concat(ERRORS.DIRECTIVE_DEFINITION_INVALID.err({ - message: `Invalid definition for directive ${directiveName}: ${directiveName} should have locations ${expected.locations.join(', ')}, but found (non-subset) ${actual.locations.join(', ')}`, - nodes: actual.sourceAST - })); + errors = errors.concat(ERRORS.DIRECTIVE_DEFINITION_INVALID.err( + `Invalid definition for directive ${directiveName}: ${directiveName} should have locations ${expected.locations.join(', ')}, but found (non-subset) ${actual.locations.join(', ')}`, + { nodes: actual.sourceAST }, + )); } return errors; } @@ -297,10 +299,10 @@ function ensureSameArguments( // Not declaring an optional argument is ok: that means you won't be able to pass a non-default value in your schema, but we allow you that. // But missing a required argument it not ok. if (isNonNullType(type) && defaultValue === undefined) { - errors.push(ERRORS.DIRECTIVE_DEFINITION_INVALID.err({ - message: `Invalid definition for ${what}: missing required argument "${name}"`, - nodes: containerSourceAST - })); + errors.push(ERRORS.DIRECTIVE_DEFINITION_INVALID.err( + `Invalid definition for ${what}: missing required argument "${name}"`, + { nodes: containerSourceAST }, + )); } continue; } @@ -313,24 +315,24 @@ function ensureSameArguments( actualType = actualType.ofType; } if (!sameType(type, actualType) && !isValidInputTypeRedefinition(type, actualType)) { - errors.push(ERRORS.DIRECTIVE_DEFINITION_INVALID.err({ - message: `Invalid definition for ${what}: argument "${name}" should have type "${type}" but found type "${actualArgument.type!}"`, - nodes: actualArgument.sourceAST - })); + errors.push(ERRORS.DIRECTIVE_DEFINITION_INVALID.err( + `Invalid definition for ${what}: argument "${name}" should have type "${type}" but found type "${actualArgument.type!}"`, + { nodes: actualArgument.sourceAST }, + )); } else if (!isNonNullType(actualArgument.type!) && !valueEquals(defaultValue, actualArgument.defaultValue)) { - errors.push(ERRORS.DIRECTIVE_DEFINITION_INVALID.err({ - message: `Invalid definition for ${what}: argument "${name}" should have default value ${valueToString(defaultValue)} but found default value ${valueToString(actualArgument.defaultValue)}`, - nodes: actualArgument.sourceAST - })); + errors.push(ERRORS.DIRECTIVE_DEFINITION_INVALID.err( + `Invalid definition for ${what}: argument "${name}" should have default value ${valueToString(defaultValue)} but found default value ${valueToString(actualArgument.defaultValue)}`, + { nodes: actualArgument.sourceAST }, + )); } } for (const actualArgument of actual.arguments()) { // If it's an expect argument, we already validated it. But we still need to reject unkown argument. if (!expectedArguments.some((arg) => arg.name === actualArgument.name)) { - errors.push(ERRORS.DIRECTIVE_DEFINITION_INVALID.err({ - message: `Invalid definition for ${what}: unknown/unsupported argument "${actualArgument.name}"`, - nodes: actualArgument.sourceAST - })); + errors.push(ERRORS.DIRECTIVE_DEFINITION_INVALID.err( + `Invalid definition for ${what}: unknown/unsupported argument "${actualArgument.name}"`, + { nodes: actualArgument.sourceAST }, + )); } } return errors; diff --git a/internals-js/src/error.ts b/internals-js/src/error.ts index b410a2587..c428faf71 100644 --- a/internals-js/src/error.ts +++ b/internals-js/src/error.ts @@ -1,7 +1,12 @@ -import { ASTNode, GraphQLError, Source } from "graphql"; -import { SchemaRootKind } from "./definitions"; +import { ASTNode, GraphQLError, GraphQLErrorOptions, GraphQLFormattedError } from "graphql"; import { assert } from "./utils"; +// Redefining `SchemaRootKind` here instead of using the version from `./definition.ts` because we don't want +// this file to import other ones, so it can be used without worrying about recusive dependency. That +// "duplication" is very minor in practice though and unlikely to be a maintenance headache (graphQL is unlikely +// to add new root kind all that often). +type SchemaRootKind = 'query' | 'mutation' | 'subscription'; + /* * We didn't track errors addition precisely pre-2.0 and tracking it now has an * unclear ROI, so we just mark all the error code that predates 2.0 as 0.x. @@ -13,22 +18,11 @@ export type ErrorCodeMetadata = { replaces?: string[], } -export type GraphQLErrorArgs = { - message: string, - nodes?: readonly ASTNode[] | ASTNode, - source?: Source, - positions?: readonly number[], - path?: readonly (string | number)[], - originalError?: Error | null, - extensions?: { [key: string]: unknown }, -}; - - export type ErrorCodeDefinition = { code: string, description: string, metadata: ErrorCodeMetadata, - err: (args: GraphQLErrorArgs) => GraphQLError, + err: (message: string, options?: GraphQLErrorOptions) => GraphQLError, } const makeCodeDefinition = ( @@ -39,28 +33,29 @@ const makeCodeDefinition = ( code, description, metadata, - err: ({ - message, - nodes, - source, - positions, - path, - originalError, - extensions, - }: GraphQLErrorArgs) => new GraphQLError( + err: (message: string, options?: GraphQLErrorOptions) => new GraphQLError( message, - nodes, - source, - positions, - path, - originalError, { - ...extensions, - code, - }, + ...options, + extensions: { + ...options?.extensions, + code, + } + } ), }); +export function extractGraphQLErrorOptions(e: GraphQLError): GraphQLErrorOptions { + return { + nodes: e.nodes, + source: e.source, + positions: e.positions, + path: e.path, + originalError: e.originalError, + extensions: e.extensions, + }; +} + /* * Most codes currently originate from the initial fed 2 release so we use this for convenience. * This can be changed later, inline versions everywhere, if that becomes irrelevant. @@ -93,14 +88,14 @@ const makeFederationDirectiveErrorCodeCategory = ( ) => makeErrorCodeCategory((directive) => `${directive.toLocaleUpperCase()}_${codeSuffix}`, makeDescription, metadata); -export function errorCode(e: GraphQLError): string | undefined { - if (!('code' in e.extensions)) { +export function errorCode(e: GraphQLError | GraphQLFormattedError): string | undefined { + if (!e.extensions || !('code' in e.extensions)) { return undefined; } return e.extensions.code as string; } -export function errorCodeDef(e: GraphQLError | string): ErrorCodeDefinition | undefined { +export function errorCodeDef(e: GraphQLError | GraphQLFormattedError | string): ErrorCodeDefinition | undefined { const code = typeof e === 'string' ? e : errorCode(e); return code ? codeDefByCode[code] : undefined; } @@ -154,6 +149,12 @@ const UNKNOWN_FEDERATION_LINK_VERSION = makeCodeDefinition( 'The version of federation in a @link directive on the schema is unknown.', ); +const UNKNOWN_LINK_VERSION = makeCodeDefinition( + 'UNKNOWN_LINK_VERSION', + 'The version of @link set on the schema is unknown.', + { addedIn: '2.1.0' }, +); + const FIELDS_HAS_ARGS = makeFederationDirectiveErrorCodeCategory( 'FIELDS_HAS_ARGS', (directive) => `The \`fields\` argument of a \`@${directive}\` directive includes a field defined with arguments (which is not currently supported).` @@ -260,6 +261,12 @@ const EXTERNAL_TYPE_MISMATCH = makeCodeDefinition( { addedIn: FED1_CODE }, ); +const EXTERNAL_COLLISION_WITH_ANOTHER_DIRECTIVE = makeCodeDefinition( + 'EXTERNAL_COLLISION_WITH_ANOTHER_DIRECTIVE', + 'The @external directive collides with other directives in some situations.', + { addedIn: '2.1.0' }, +); + const EXTERNAL_ARGUMENT_MISSING = makeCodeDefinition( 'EXTERNAL_ARGUMENT_MISSING', 'An `@external` field is missing some arguments present in the declaration(s) of that field in other subgraphs.', @@ -301,7 +308,6 @@ const INPUT_FIELD_DEFAULT_MISMATCH = makeCodeDefinition( 'INPUT_FIELD_DEFAULT_MISMATCH', 'An input field has a default value that is incompatible with other declarations of that field in other subgraphs.', ); - const ARGUMENT_DEFAULT_MISMATCH = makeCodeDefinition( 'FIELD_ARGUMENT_DEFAULT_MISMATCH', 'An argument (of a field/directive) has a default value that is incompatible with that of other declarations of that same argument in other subgraphs.', @@ -339,6 +345,12 @@ const INVALID_LINK_DIRECTIVE_USAGE = makeCodeDefinition( 'An application of the @link directive is invalid/does not respect the specification.' ); +const INVALID_LINK_IDENTIFIER = makeCodeDefinition( + 'INVALID_LINK_IDENTIFIER', + 'A url/version for a @link feature is invalid/does not respect the specification.', + { addedIn: '2.1.0' }, +); + const LINK_IMPORT_NAME_MISMATCH = makeCodeDefinition( 'LINK_IMPORT_NAME_MISMATCH', 'The import name for a merged directive (as declared by the relevant `@link(import:)` argument) is inconsistent between subgraphs.' @@ -424,6 +436,24 @@ const OVERRIDE_COLLISION_WITH_ANOTHER_DIRECTIVE = makeCodeDefinition( 'The @override directive cannot be used on external fields, nor to override fields with either @external, @provides, or @requires.', ); +const UNSUPPORTED_FEATURE = makeCodeDefinition( + 'UNSUPPORTED_FEATURE', + 'Indicates an error due to feature currently unsupported by federation.', + { addedIn: '2.1.0' }, +); + +const INVALID_FEDERATION_SUPERGRAPH = makeCodeDefinition( + 'INVALID_FEDERATION_SUPERGRAPH', + 'Indicates that a schema provided for an Apollo Federation supergraph is not a valid supergraph schema.', + { addedIn: '2.1.0' }, +); + +const DOWNSTREAM_SERVICE_ERROR = makeCodeDefinition( + 'DOWNSTREAM_SERVICE_ERROR', + 'Indicates an error in a subgraph service query during query execution in a federated service.', + { addedIn: FED1_CODE }, +); + export const ERROR_CATEGORIES = { DIRECTIVE_FIELDS_MISSING_EXTERNAL, DIRECTIVE_UNSUPPORTED_ON_INTERFACE, @@ -438,6 +468,7 @@ export const ERRORS = { DIRECTIVE_DEFINITION_INVALID, TYPE_DEFINITION_INVALID, UNKNOWN_FEDERATION_LINK_VERSION, + UNKNOWN_LINK_VERSION, KEY_FIELDS_HAS_ARGS, PROVIDES_FIELDS_HAS_ARGS, REQUIRES_FIELDS_HAS_ARGS, @@ -447,6 +478,7 @@ export const ERRORS = { PROVIDES_UNSUPPORTED_ON_INTERFACE, REQUIRES_UNSUPPORTED_ON_INTERFACE, EXTERNAL_UNUSED, + EXTERNAL_COLLISION_WITH_ANOTHER_DIRECTIVE, TYPE_WITH_ONLY_UNUSED_EXTERNAL, PROVIDES_ON_NON_OBJECT_FIELD, KEY_INVALID_FIELDS_TYPE, @@ -479,6 +511,7 @@ export const ERRORS = { INTERFACE_FIELD_IMPLEM_TYPE_MISMATCH, INVALID_FIELD_SHARING, INVALID_LINK_DIRECTIVE_USAGE, + INVALID_LINK_IDENTIFIER, LINK_IMPORT_NAME_MISMATCH, REFERENCED_INACCESSIBLE, DEFAULT_VALUE_USES_INACCESSIBLE, @@ -496,6 +529,9 @@ export const ERRORS = { OVERRIDE_COLLISION_WITH_ANOTHER_DIRECTIVE, OVERRIDE_FROM_SELF_ERROR, OVERRIDE_SOURCE_HAS_OVERRIDE, + UNSUPPORTED_FEATURE, + INVALID_FEDERATION_SUPERGRAPH, + DOWNSTREAM_SERVICE_ERROR, }; const codeDefByCode = Object.values(ERRORS).reduce((obj: {[code: string]: ErrorCodeDefinition}, codeDef: ErrorCodeDefinition) => { obj[codeDef.code] = codeDef; return obj; }, {}); diff --git a/internals-js/src/federation.ts b/internals-js/src/federation.ts index 1e05b8fa8..bdffccfa0 100644 --- a/internals-js/src/federation.ts +++ b/internals-js/src/federation.ts @@ -40,6 +40,7 @@ import { print as printAST, Source, SchemaExtensionNode, + GraphQLErrorOptions, } from "graphql"; import { KnownTypeNamesInFederationRule } from "./validation/KnownTypeNamesInFederationRule"; import { buildSchema, buildSchemaFromAST } from "./buildSchema"; @@ -52,6 +53,7 @@ import { ERROR_CATEGORIES, ERRORS, withModifiedErrorMessage, + extractGraphQLErrorOptions, } from "./error"; import { computeShareables } from "./precompute"; import { @@ -125,26 +127,26 @@ function validateFieldSetSelections( const field = selection.element().definition; const isExternal = federationMetadata.isFieldExternal(field); if (field.hasArguments()) { - throw ERROR_CATEGORIES.FIELDS_HAS_ARGS.get(directiveName).err({ - message: `field ${field.coordinate} cannot be included because it has arguments (fields with argument are not allowed in @${directiveName})`, - nodes: field.sourceAST - }); + throw ERROR_CATEGORIES.FIELDS_HAS_ARGS.get(directiveName).err( + `field ${field.coordinate} cannot be included because it has arguments (fields with argument are not allowed in @${directiveName})`, + { nodes: field.sourceAST }, + ); } // The field must be external if we don't allow non-external leaf fields, it's a leaf, and we haven't traversed an external field in parent chain leading here. const mustBeExternal = !selection.selectionSet && !allowOnNonExternalLeafFields && !hasExternalInParents; if (!isExternal && mustBeExternal) { const errorCode = ERROR_CATEGORIES.DIRECTIVE_FIELDS_MISSING_EXTERNAL.get(directiveName); if (federationMetadata.isFieldFakeExternal(field)) { - throw errorCode.err({ - message: `field "${field.coordinate}" should not be part of a @${directiveName} since it is already "effectively" provided by this subgraph ` + throw errorCode.err( + `field "${field.coordinate}" should not be part of a @${directiveName} since it is already "effectively" provided by this subgraph ` + `(while it is marked @${externalDirectiveSpec.name}, it is a @${keyDirectiveSpec.name} field of an extension type, which are not internally considered external for historical/backward compatibility reasons)`, - nodes: field.sourceAST - }); + { nodes: field.sourceAST } + ); } else { - throw errorCode.err({ - message: `field "${field.coordinate}" should not be part of a @${directiveName} since it is already provided by this subgraph (it is not marked @${externalDirectiveSpec.name})`, - nodes: field.sourceAST - }); + throw errorCode.err( + `field "${field.coordinate}" should not be part of a @${directiveName} since it is already provided by this subgraph (it is not marked @${externalDirectiveSpec.name})`, + { nodes: field.sourceAST } + ); } } if (selection.selectionSet) { @@ -197,16 +199,7 @@ function validateFieldSet( if (!(e instanceof GraphQLError)) { throw e; } - const nodes = sourceASTs(directive); - if (e.nodes) { - nodes.push(...e.nodes); - } - const codeDef = errorCodeDef(e) ?? ERROR_CATEGORIES.DIRECTIVE_INVALID_FIELDS.get(directive.name); - return codeDef.err({ - message: `${fieldSetErrorDescriptor(directive)}: ${e.message.trim()}`, - nodes, - originalError: e, - }); + return handleFieldSetValidationError(directive, e); } } catch (e) { if (e instanceof GraphQLError) { @@ -217,6 +210,34 @@ function validateFieldSet( } } +function handleFieldSetValidationError( + directive: Directive, + originalError: GraphQLError, + messageUpdater?: (msg: string) => string, +): GraphQLError { + const nodes = sourceASTs(directive); + if (originalError.nodes) { + nodes.push(...originalError.nodes); + } + let codeDef = errorCodeDef(originalError); + // "INVALID_GRAPHQL" errors happening during validation means that the selection set is invalid, and + // that's where we want to use a more precise code. + if (!codeDef || codeDef === ERRORS.INVALID_GRAPHQL) { + codeDef = ERROR_CATEGORIES.DIRECTIVE_INVALID_FIELDS.get(directive.name); + } + let msg = originalError.message.trim(); + if (messageUpdater) { + msg = messageUpdater(msg); + } + return codeDef.err( + `${fieldSetErrorDescriptor(directive)}: ${msg}`, + { + nodes, + originalError, + } + ); +} + function fieldSetErrorDescriptor(directive: Directive): string { return `On ${fieldSetTargetDescription(directive)}, for ${directiveStrUsingASTIfPossible(directive)}`; } @@ -248,12 +269,12 @@ function validateAllFieldSet>( const parentType = isOnParentType ? type : (elt.parent as NamedType); if (isInterfaceType(parentType)) { const code = ERROR_CATEGORIES.DIRECTIVE_UNSUPPORTED_ON_INTERFACE.get(definition.name); - errorCollector.push(code.err({ - message: isOnParentType + errorCollector.push(code.err( + isOnParentType ? `Cannot use ${definition.coordinate} on interface "${parentType.coordinate}": ${definition.coordinate} is not yet supported on interfaces` : `Cannot use ${definition.coordinate} on ${fieldSetTargetDescription(application)} of parent type "${parentType}": ${definition.coordinate} is not yet supported within interfaces`, - nodes: sourceASTs(application).concat(isOnParentType ? [] : sourceASTs(type)), - })); + { nodes: sourceASTs(application).concat(isOnParentType ? [] : sourceASTs(type)) }, + )); } const error = validateFieldSet( type, @@ -345,11 +366,11 @@ function validateAllExternalFieldsUsed(metadata: FederationMetadata, errorCollec continue; } - errorCollector.push(ERRORS.EXTERNAL_UNUSED.err({ - message: `Field "${field.coordinate}" is marked @external but is not used in any federation directive (@key, @provides, @requires) or to satisfy an interface;` + errorCollector.push(ERRORS.EXTERNAL_UNUSED.err( + `Field "${field.coordinate}" is marked @external but is not used in any federation directive (@key, @provides, @requires) or to satisfy an interface;` + ' the field declaration has no use and should be removed (or the field should not be @external).', - nodes: field.sourceAST, - })); + { nodes: field.sourceAST }, + )); } } } @@ -358,10 +379,10 @@ function validateNoExternalOnInterfaceFields(metadata: FederationMetadata, error for (const itf of metadata.schema.interfaceTypes()) { for (const field of itf.fields()) { if (metadata.isFieldExternal(field)) { - errorCollector.push(ERRORS.EXTERNAL_ON_INTERFACE.err({ - message: `Interface type field "${field.coordinate}" is marked @external but @external is not allowed on interface fields (it is nonsensical).`, - nodes: field.sourceAST, - })); + errorCollector.push(ERRORS.EXTERNAL_ON_INTERFACE.err( + `Interface type field "${field.coordinate}" is marked @external but @external is not allowed on interface fields (it is nonsensical).`, + { nodes: field.sourceAST }, + )); } } } @@ -401,10 +422,10 @@ function validateInterfaceRuntimeImplementationFieldsTypes( } if (withExternalOrRequires.length > 0 && typeToImplems.size > 1) { const typeToImplemsArray = [...typeToImplems.entries()]; - errorCollector.push(ERRORS.INTERFACE_FIELD_IMPLEM_TYPE_MISMATCH.err({ - message: `Some of the runtime implementations of interface field "${field.coordinate}" are marked @external or have a @require (${withExternalOrRequires.map(printFieldCoordinate)}) so all the implementations should use the same type (a current limitation of federation; see https://github.com/apollographql/federation/issues/1257), but ${formatFieldsToReturnType(typeToImplemsArray[0])} while ${joinStrings(typeToImplemsArray.slice(1).map(formatFieldsToReturnType), ' and ')}.`, - nodes - })); + errorCollector.push(ERRORS.INTERFACE_FIELD_IMPLEM_TYPE_MISMATCH.err( + `Some of the runtime implementations of interface field "${field.coordinate}" are marked @external or have a @require (${withExternalOrRequires.map(printFieldCoordinate)}) so all the implementations should use the same type (a current limitation of federation; see https://github.com/apollographql/federation/issues/1257), but ${formatFieldsToReturnType(typeToImplemsArray[0])} while ${joinStrings(typeToImplemsArray.slice(1).map(formatFieldsToReturnType), ' and ')}.`, + { nodes }, + )); } } } @@ -691,12 +712,12 @@ export class FederationBlueprint extends SchemaBlueprint { // composition error. const existing = schema.type(defaultName); if (existing) { - errors.push(ERROR_CATEGORIES.ROOT_TYPE_USED.get(k).err({ - message: `The schema has a type named "${defaultName}" but it is not set as the ${k} root type ("${type.name}" is instead): ` + errors.push(ERROR_CATEGORIES.ROOT_TYPE_USED.get(k).err( + `The schema has a type named "${defaultName}" but it is not set as the ${k} root type ("${type.name}" is instead): ` + 'this is not supported by federation. ' + 'If a root type does not use its default name, there should be no other type with that default name.', - nodes: sourceASTs(type, existing), - })); + { nodes: sourceASTs(type, existing) }, + )); } type.rename(defaultName); } @@ -726,9 +747,9 @@ export class FederationBlueprint extends SchemaBlueprint { if (isUnionType(type) || isInterfaceType(type)) { let kind: string = type.kind; kind = kind.slice(0, kind.length - 'Type'.length); - throw ERRORS.KEY_FIELDS_SELECT_INVALID_TYPE.err({ - message: `field "${field.coordinate}" is a ${kind} type which is not allowed in @key` - }); + throw ERRORS.KEY_FIELDS_SELECT_INVALID_TYPE.err( + `field "${field.coordinate}" is a ${kind} type which is not allowed in @key`, + ); } } ); @@ -755,14 +776,17 @@ export class FederationBlueprint extends SchemaBlueprint { metadata.providesDirective(), field => { if (metadata.isFieldExternal(field)) { - throw new GraphQLError(`Cannot have both @provides and @external on field "${field.coordinate}"`, field.sourceAST); + throw ERRORS.EXTERNAL_COLLISION_WITH_ANOTHER_DIRECTIVE.err( + `Cannot have both @provides and @external on field "${field.coordinate}"`, + { nodes: field.sourceAST }, + ); } const type = baseType(field.type!); if (!isCompositeType(type)) { - throw ERRORS.PROVIDES_ON_NON_OBJECT_FIELD.err({ - message: `Invalid @provides directive on field "${field.coordinate}": field has type "${field.type}" which is not a Composite Type`, - nodes: field.sourceAST, - }); + throw ERRORS.PROVIDES_ON_NON_OBJECT_FIELD.err( + `Invalid @provides directive on field "${field.coordinate}": field has type "${field.type}" which is not a Composite Type`, + { nodes: field.sourceAST }, + ); } return type; }, @@ -1084,10 +1108,10 @@ function completeFed2SubgraphSchema(schema: Schema) { const spec = FEDERATION_VERSIONS.find(fedFeature.url.version); if (!spec) { - return [ERRORS.UNKNOWN_FEDERATION_LINK_VERSION.err({ - message: `Invalid version ${fedFeature.url.version} for the federation feature in @link direction on schema`, - nodes: fedFeature.directive.sourceAST - })]; + return [ERRORS.UNKNOWN_FEDERATION_LINK_VERSION.err( + `Invalid version ${fedFeature.url.version} for the federation feature in @link direction on schema`, + { nodes: fedFeature.directive.sourceAST }, + )]; } return spec.addElementsToSchema(schema); @@ -1116,34 +1140,23 @@ export function parseFieldSetArgument({ throw e; } - const nodes = sourceASTs(directive); - if (e.nodes) { - nodes.push(...e.nodes); - } - let msg = e.message.trim(); - // The rule for validating @requires in fed 1 was not properly recursive, so people upgrading - // may have a @require that selects some fields but without declaring those fields on the - // subgraph. As we fixed the validation, this will now fail, but we try here to provide some - // hint for those users for how to fix the problem. - // Note that this is a tad fragile to rely on the error message like that, but worth case, a - // future change make us not show the hint and that's not the end of the world. - if (msg.startsWith('Cannot query field')) { - if (msg.endsWith('.')) { - msg = msg.slice(0, msg.length - 1); - } - if (directive.name === keyDirectiveSpec.name) { - msg = msg + ' (the field should be either be added to this subgraph or, if it should not be resolved by this subgraph, you need to add it to this subgraph with @external).'; - } else { - msg = msg + ' (if the field is defined in another subgraph, you need to add it to this subgraph with @external).'; - } - } - - const codeDef = errorCodeDef(e) ?? ERROR_CATEGORIES.DIRECTIVE_INVALID_FIELDS.get(directive.name); - throw codeDef.err({ - message: `${fieldSetErrorDescriptor(directive)}: ${msg}`, - nodes, - originalError: e, - }); + throw handleFieldSetValidationError( + directive, + e, + (msg: string) => { + if (msg.startsWith('Cannot query field')) { + if (msg.endsWith('.')) { + msg = msg.slice(0, msg.length - 1); + } + if (directive.name === keyDirectiveSpec.name) { + msg = msg + ' (the field should either be added to this subgraph or, if it should not be resolved by this subgraph, you need to add it to this subgraph with @external).'; + } else { + msg = msg + ' (if the field is defined in another subgraph, you need to add it to this subgraph with @external).'; + } + } + return msg; + }, + ); } } @@ -1195,10 +1208,10 @@ function validateFieldSetValue(directive: Directive this.inaccessibleLocations.includes(loc)); if (hasUnknownArguments || hasRepeatable || !hasValidLocations) { - return ERRORS.DIRECTIVE_DEFINITION_INVALID.err({ - message: `Found invalid @inaccessible directive definition. Please ensure the directive definition in your schema's definitions matches the following:\n\t${this.printedInaccessibleDefinition}`, - }); + return ERRORS.DIRECTIVE_DEFINITION_INVALID.err( + `Found invalid @inaccessible directive definition. Please ensure the directive definition in your schema's definitions matches the following:\n\t${this.printedInaccessibleDefinition}`, + ); } return undefined; } @@ -303,32 +303,34 @@ function validateInaccessibleElements( // @inaccessible, regardless of shadowing. const inaccessibleElements = fetchInaccessibleElementsDeep(type); if (inaccessibleElements.length > 0) { - errors.push(ERRORS.DISALLOWED_INACCESSIBLE.err({ - message: - `Built-in type "${type.coordinate}" cannot use @inaccessible.`, - nodes: type.sourceAST, - extensions: { - inaccessible_elements: inaccessibleElements + errors.push(ERRORS.DISALLOWED_INACCESSIBLE.err( + `Built-in type "${type.coordinate}" cannot use @inaccessible.`, + { + nodes: type.sourceAST, + extensions: { + inaccessible_elements: inaccessibleElements .map((element) => element.coordinate), - inaccessible_referencers: [type.coordinate], - } - })); + inaccessible_referencers: [type.coordinate], + } + }, + )); } } else if (isFeatureDefinition(type)) { // Core feature types (and their descendants) aren't allowed to be // @inaccessible. const inaccessibleElements = fetchInaccessibleElementsDeep(type); if (inaccessibleElements.length > 0) { - errors.push(ERRORS.DISALLOWED_INACCESSIBLE.err({ - message: - `Core feature type "${type.coordinate}" cannot use @inaccessible.`, - nodes: type.sourceAST, - extensions: { - inaccessible_elements: inaccessibleElements + errors.push(ERRORS.DISALLOWED_INACCESSIBLE.err( + `Core feature type "${type.coordinate}" cannot use @inaccessible.`, + { + nodes: type.sourceAST, + extensions: { + inaccessible_elements: inaccessibleElements .map((element) => element.coordinate), - inaccessible_referencers: [type.coordinate], - } - })); + inaccessible_referencers: [type.coordinate], + } + }, + )); } } else if (isInaccessible(type)) { // Types can be referenced by other schema elements in a few ways: @@ -357,28 +359,30 @@ function validateInaccessibleElements( referencer instanceof InputFieldDefinition ) { if (isInAPISchema(referencer)) { - errors.push(ERRORS.REFERENCED_INACCESSIBLE.err({ - message: - `Type "${type.coordinate}" is @inaccessible but is referenced` + - ` by "${referencer.coordinate}", which is in the API schema.`, - nodes: type.sourceAST, - extensions: { - inaccessible_elements: [type.coordinate], - inaccessible_referencers: [referencer.coordinate], - } - })); + errors.push(ERRORS.REFERENCED_INACCESSIBLE.err( + `Type "${type.coordinate}" is @inaccessible but is referenced` + + ` by "${referencer.coordinate}", which is in the API schema.`, + { + nodes: type.sourceAST, + extensions: { + inaccessible_elements: [type.coordinate], + inaccessible_referencers: [referencer.coordinate], + } + }, + )); } } else if (referencer instanceof SchemaDefinition) { if (type === referencer.rootType('query')) { - errors.push(ERRORS.QUERY_ROOT_TYPE_INACCESSIBLE.err({ - message: - `Type "${type.coordinate}" is @inaccessible but is the root` + - ` query type, which must be in the API schema.`, - nodes: type.sourceAST, - extensions: { - inaccessible_elements: [type.coordinate], - } - })); + errors.push(ERRORS.QUERY_ROOT_TYPE_INACCESSIBLE.err( + `Type "${type.coordinate}" is @inaccessible but is the root` + + ` query type, which must be in the API schema.`, + { + nodes: type.sourceAST, + extensions: { + inaccessible_elements: [type.coordinate], + } + }, + )); } } } @@ -396,18 +400,19 @@ function validateInaccessibleElements( if (!isInaccessible(field)) isEmpty = false; } if (isEmpty) { - errors.push(ERRORS.ONLY_INACCESSIBLE_CHILDREN.err({ - message: - `Type "${type.coordinate}" is in the API schema but all of its` + - ` ${(type instanceof InputObjectType) ? 'input ' : ''}fields` + - ` are @inaccessible.`, - nodes: type.sourceAST, - extensions: { - inaccessible_elements: type.fields() + errors.push(ERRORS.ONLY_INACCESSIBLE_CHILDREN.err( + `Type "${type.coordinate}" is in the API schema but all of its` + + ` ${(type instanceof InputObjectType) ? 'input ' : ''}fields` + + ` are @inaccessible.`, + { + nodes: type.sourceAST, + extensions: { + inaccessible_elements: type.fields() .map((field) => field.coordinate), - inaccessible_referencers: [type.coordinate], - } - })); + inaccessible_referencers: [type.coordinate], + } + }, + )); } } else if (type instanceof UnionType) { let isEmpty = true; @@ -415,17 +420,18 @@ function validateInaccessibleElements( if (!isInaccessible(member)) isEmpty = false; } if (isEmpty) { - errors.push(ERRORS.ONLY_INACCESSIBLE_CHILDREN.err({ - message: - `Type "${type.coordinate}" is in the API schema but all of its` + - ` members are @inaccessible.`, - nodes: type.sourceAST, - extensions: { - inaccessible_elements: type.types() + errors.push(ERRORS.ONLY_INACCESSIBLE_CHILDREN.err( + `Type "${type.coordinate}" is in the API schema but all of its` + + ` members are @inaccessible.`, + { + nodes: type.sourceAST, + extensions: { + inaccessible_elements: type.types() .map((type) => type.coordinate), - inaccessible_referencers: [type.coordinate], + inaccessible_referencers: [type.coordinate], + } } - })); + )); } } else if (type instanceof EnumType) { let isEmpty = true; @@ -433,17 +439,18 @@ function validateInaccessibleElements( if (!isInaccessible(enumValue)) isEmpty = false; } if (isEmpty) { - errors.push(ERRORS.ONLY_INACCESSIBLE_CHILDREN.err({ - message: - `Type "${type.coordinate}" is in the API schema but all of its` + - ` values are @inaccessible.`, - nodes: type.sourceAST, - extensions: { - inaccessible_elements: type.values + errors.push(ERRORS.ONLY_INACCESSIBLE_CHILDREN.err( + `Type "${type.coordinate}" is in the API schema but all of its` + + ` values are @inaccessible.`, + { + nodes: type.sourceAST, + extensions: { + inaccessible_elements: type.values .map((enumValue) => enumValue.coordinate), - inaccessible_referencers: [type.coordinate], + inaccessible_referencers: [type.coordinate], + } } - })); + )); } } @@ -473,18 +480,19 @@ function validateInaccessibleElements( for (const implementedInterface of implementedInterfaces) { const implementedField = implementedInterface.field(field.name); if (implementedField && isInAPISchema(implementedField)) { - errors.push(ERRORS.IMPLEMENTED_BY_INACCESSIBLE.err({ - message: - `Field "${field.coordinate}" is @inaccessible but` + - ` implements the interface field` + - ` "${implementedField.coordinate}", which is in the API` + - ` schema.`, - nodes: field.sourceAST, - extensions: { - inaccessible_elements: [field.coordinate], - inaccessible_referencers: [implementedField.coordinate], + errors.push(ERRORS.IMPLEMENTED_BY_INACCESSIBLE.err( + `Field "${field.coordinate}" is @inaccessible but` + + ` implements the interface field` + + ` "${implementedField.coordinate}", which is in the API` + + ` schema.`, + { + nodes: field.sourceAST, + extensions: { + inaccessible_elements: [field.coordinate], + inaccessible_referencers: [implementedField.coordinate], + } } - })); + )); } } } else { @@ -494,16 +502,17 @@ function validateInaccessibleElements( // When an argument is hidden (but its ancestors aren't), we // check that it isn't a required argument of its field. if (argument.isRequired()) { - errors.push(ERRORS.REQUIRED_INACCESSIBLE.err({ - message: - `Argument "${argument.coordinate}" is @inaccessible but` + - ` is a required argument of its field.`, - nodes: argument.sourceAST, - extensions: { - inaccessible_elements: [argument.coordinate], - inaccessible_referencers: [argument.coordinate], - } - })); + errors.push(ERRORS.REQUIRED_INACCESSIBLE.err( + `Argument "${argument.coordinate}" is @inaccessible but` + + ` is a required argument of its field.`, + { + nodes: argument.sourceAST, + extensions: { + inaccessible_elements: [argument.coordinate], + inaccessible_referencers: [argument.coordinate], + } + }, + )); } // When an argument is hidden (but its ancestors aren't), we // check that it isn't a required argument of any implementing @@ -540,20 +549,21 @@ function validateInaccessibleElements( isInAPISchema(implementingArgument) && implementingArgument.isRequired() ) { - errors.push(ERRORS.REQUIRED_INACCESSIBLE.err({ - message: - `Argument "${argument.coordinate}" is @inaccessible` + - ` but is implemented by the required argument` + - ` "${implementingArgument.coordinate}", which is` + - ` in the API schema.`, - nodes: argument.sourceAST, - extensions: { - inaccessible_elements: [argument.coordinate], - inaccessible_referencers: [ - implementingArgument.coordinate, - ], - } - })); + errors.push(ERRORS.REQUIRED_INACCESSIBLE.err( + `Argument "${argument.coordinate}" is @inaccessible` + + ` but is implemented by the required argument` + + ` "${implementingArgument.coordinate}", which is` + + ` in the API schema.`, + { + nodes: argument.sourceAST, + extensions: { + inaccessible_elements: [argument.coordinate], + inaccessible_referencers: [ + implementingArgument.coordinate, + ], + } + }, + )); } } @@ -569,20 +579,21 @@ function validateInaccessibleElements( implementedArgument && isInAPISchema(implementedArgument) ) { - errors.push(ERRORS.IMPLEMENTED_BY_INACCESSIBLE.err({ - message: - `Argument "${argument.coordinate}" is @inaccessible` + - ` but implements the interface argument` + - ` "${implementedArgument.coordinate}", which is in` + - ` the API schema.`, - nodes: argument.sourceAST, - extensions: { - inaccessible_elements: [argument.coordinate], - inaccessible_referencers: [ - implementedArgument.coordinate, - ], - } - })); + errors.push(ERRORS.IMPLEMENTED_BY_INACCESSIBLE.err( + `Argument "${argument.coordinate}" is @inaccessible` + + ` but implements the interface argument` + + ` "${implementedArgument.coordinate}", which is in` + + ` the API schema.`, + { + nodes: argument.sourceAST, + extensions: { + inaccessible_elements: [argument.coordinate], + inaccessible_referencers: [ + implementedArgument.coordinate, + ], + } + }, + )); } } } @@ -595,16 +606,17 @@ function validateInaccessibleElements( // When an input field is hidden (but its parent isn't), we check // that it isn't a required argument of its field. if (inputField.isRequired()) { - errors.push(ERRORS.REQUIRED_INACCESSIBLE.err({ - message: - `Input field "${inputField.coordinate}" is @inaccessible` + - ` but is a required input field of its type.`, - nodes: inputField.sourceAST, - extensions: { - inaccessible_elements: [inputField.coordinate], - inaccessible_referencers: [inputField.coordinate], - } - })); + errors.push(ERRORS.REQUIRED_INACCESSIBLE.err( + `Input field "${inputField.coordinate}" is @inaccessible` + + ` but is a required input field of its type.`, + { + nodes: inputField.sourceAST, + extensions: { + inaccessible_elements: [inputField.coordinate], + inaccessible_referencers: [inputField.coordinate], + } + }, + )); } // Input fields can be referenced by schema default values. When an @@ -619,17 +631,18 @@ function validateInaccessibleElements( const referencers = defaultValueReferencers.get(inputField) ?? []; for (const referencer of referencers) { if (isInAPISchema(referencer)) { - errors.push(ERRORS.DEFAULT_VALUE_USES_INACCESSIBLE.err({ - message: - `Input field "${inputField.coordinate}" is @inaccessible` + - ` but is used in the default value of` + - ` "${referencer.coordinate}", which is in the API schema.`, - nodes: type.sourceAST, - extensions: { - inaccessible_elements: [type.coordinate], - inaccessible_referencers: [referencer.coordinate], - } - })); + errors.push(ERRORS.DEFAULT_VALUE_USES_INACCESSIBLE.err( + `Input field "${inputField.coordinate}" is @inaccessible` + + ` but is used in the default value of` + + ` "${referencer.coordinate}", which is in the API schema.`, + { + nodes: type.sourceAST, + extensions: { + inaccessible_elements: [type.coordinate], + inaccessible_referencers: [referencer.coordinate], + } + }, + )); } } } @@ -649,17 +662,18 @@ function validateInaccessibleElements( const referencers = defaultValueReferencers.get(enumValue) ?? []; for (const referencer of referencers) { if (isInAPISchema(referencer)) { - errors.push(ERRORS.DEFAULT_VALUE_USES_INACCESSIBLE.err({ - message: - `Enum value "${enumValue.coordinate}" is @inaccessible` + - ` but is used in the default value of` + - ` "${referencer.coordinate}", which is in the API schema.`, - nodes: type.sourceAST, - extensions: { - inaccessible_elements: [type.coordinate], - inaccessible_referencers: [referencer.coordinate], - } - })); + errors.push(ERRORS.DEFAULT_VALUE_USES_INACCESSIBLE.err( + `Enum value "${enumValue.coordinate}" is @inaccessible` + + ` but is used in the default value of` + + ` "${referencer.coordinate}", which is in the API schema.`, + { + nodes: type.sourceAST, + extensions: { + inaccessible_elements: [type.coordinate], + inaccessible_referencers: [referencer.coordinate], + } + }, + )); } } } @@ -679,17 +693,17 @@ function validateInaccessibleElements( const inaccessibleElements = fetchInaccessibleElementsDeep(directive); if (inaccessibleElements.length > 0) { - errors.push(ERRORS.DISALLOWED_INACCESSIBLE.err({ - message: - `Built-in directive "${directive.coordinate}" cannot use` + - ` @inaccessible.`, - nodes: directive.sourceAST, - extensions: { - inaccessible_elements: inaccessibleElements + errors.push(ERRORS.DISALLOWED_INACCESSIBLE.err( + `Built-in directive "${directive.coordinate}" cannot use @inaccessible.`, + { + nodes: directive.sourceAST, + extensions: { + inaccessible_elements: inaccessibleElements .map((element) => element.coordinate), - inaccessible_referencers: [directive.coordinate], - } - })); + inaccessible_referencers: [directive.coordinate], + } + }, + )); } } else if (isFeatureDefinition(directive)) { // Core feature directives (and their descendants) aren't allowed to be @@ -697,17 +711,17 @@ function validateInaccessibleElements( const inaccessibleElements = fetchInaccessibleElementsDeep(directive); if (inaccessibleElements.length > 0) { - errors.push(ERRORS.DISALLOWED_INACCESSIBLE.err({ - message: - `Core feature directive "${directive.coordinate}" cannot use` + - ` @inaccessible.`, - nodes: directive.sourceAST, - extensions: { - inaccessible_elements: inaccessibleElements + errors.push(ERRORS.DISALLOWED_INACCESSIBLE.err( + `Core feature directive "${directive.coordinate}" cannot use @inaccessible.`, + { + nodes: directive.sourceAST, + extensions: { + inaccessible_elements: inaccessibleElements .map((element) => element.coordinate), - inaccessible_referencers: [directive.coordinate], - } - })); + inaccessible_referencers: [directive.coordinate], + } + }, + )); } } else if (typeSystemLocations.length > 0) { // Directives that can appear on type-system locations (and their @@ -715,18 +729,19 @@ function validateInaccessibleElements( const inaccessibleElements = fetchInaccessibleElementsDeep(directive); if (inaccessibleElements.length > 0) { - errors.push(ERRORS.DISALLOWED_INACCESSIBLE.err({ - message: - `Directive "${directive.coordinate}" cannot use @inaccessible` + - ` because it may be applied to these type-system locations:` + - ` ${typeSystemLocations.join(', ')}.`, - nodes: directive.sourceAST, - extensions: { - inaccessible_elements: inaccessibleElements + errors.push(ERRORS.DISALLOWED_INACCESSIBLE.err( + `Directive "${directive.coordinate}" cannot use @inaccessible` + + ` because it may be applied to these type-system locations:` + + ` ${typeSystemLocations.join(', ')}.`, + { + nodes: directive.sourceAST, + extensions: { + inaccessible_elements: inaccessibleElements .map((element) => element.coordinate), - inaccessible_referencers: [directive.coordinate], - } - })); + inaccessible_referencers: [directive.coordinate], + } + }, + )); } } else { // At this point, we know the directive must be in the API schema. Descend @@ -736,16 +751,17 @@ function validateInaccessibleElements( // isn't a required argument of its directive. if (argument.isRequired()) { if (isInaccessible(argument)) { - errors.push(ERRORS.REQUIRED_INACCESSIBLE.err({ - message: - `Argument "${argument.coordinate}" is @inaccessible but is a` + - ` required argument of its directive.`, - nodes: argument.sourceAST, - extensions: { - inaccessible_elements: [argument.coordinate], - inaccessible_referencers: [argument.coordinate], - } - })); + errors.push(ERRORS.REQUIRED_INACCESSIBLE.err( + `Argument "${argument.coordinate}" is @inaccessible but is a` + + ` required argument of its directive.`, + { + nodes: argument.sourceAST, + extensions: { + inaccessible_elements: [argument.coordinate], + inaccessible_referencers: [argument.coordinate], + } + }, + )); } } } diff --git a/internals-js/src/operations.ts b/internals-js/src/operations.ts index dd4c893b6..9e92e22da 100644 --- a/internals-js/src/operations.ts +++ b/internals-js/src/operations.ts @@ -7,7 +7,6 @@ import { FieldNode, FragmentDefinitionNode, FragmentSpreadNode, - GraphQLError, InlineFragmentNode, Kind, OperationDefinitionNode, @@ -45,13 +44,14 @@ import { isConditionalDirective, isDirectiveApplicationsSubset, } from "./definitions"; +import { ERRORS } from "./error"; import { sameType } from "./types"; import { assert, mapEntries, MapWithCachedArrays, MultiMap } from "./utils"; import { argumentsEquals, argumentsFromAST, isValidValue, valueToAST, valueToString } from "./values"; function validate(condition: any, message: () => string, sourceAST?: ASTNode): asserts condition { if (!condition) { - throw new GraphQLError(message(), sourceAST); + throw ERRORS.INVALID_GRAPHQL.err(message(), { nodes: sourceAST }); } } @@ -486,7 +486,7 @@ export class NamedFragments { add(fragment: NamedFragmentDefinition) { if (this.fragments.has(fragment.name)) { - throw new GraphQLError(`Duplicate fragment name '${fragment}'`); + throw ERRORS.INVALID_GRAPHQL.err(`Duplicate fragment name '${fragment}'`); } this.fragments.set(fragment.name, fragment); } @@ -1490,10 +1490,10 @@ export function operationFromDocument( const typeName = definition.typeCondition.name.value; const typeCondition = schema.type(typeName); if (!typeCondition) { - throw new GraphQLError(`Unknown type "${typeName}" for fragment "${name}"`, definition); + throw ERRORS.INVALID_GRAPHQL.err(`Unknown type "${typeName}" for fragment "${name}"`, { nodes: definition }); } if (!isCompositeType(typeCondition)) { - throw new GraphQLError(`Invalid fragment "${name}" on non-composite type "${typeName}"`, definition); + throw ERRORS.INVALID_GRAPHQL.err(`Invalid fragment "${name}" on non-composite type "${typeName}"`, { nodes: definition }); } const fragment = new NamedFragmentDefinition(schema, name, typeCondition, new SelectionSet(typeCondition, fragments)); addDirectiveNodesToElement(definition.directives, fragment); diff --git a/internals-js/src/schemaUpgrader.ts b/internals-js/src/schemaUpgrader.ts index 506228b90..879c98bbf 100644 --- a/internals-js/src/schemaUpgrader.ts +++ b/internals-js/src/schemaUpgrader.ts @@ -365,10 +365,10 @@ class SchemaUpgrader { } // We look at all the other subgraphs and didn't found a (non-extension) definition of that type - this.addError(ERRORS.EXTENSION_WITH_NO_BASE.err({ - message: `Type "${type}" is an extension type, but there is no type definition for "${type}" in any subgraph.`, - nodes: extensionAST, - })); + this.addError(ERRORS.EXTENSION_WITH_NO_BASE.err( + `Type "${type}" is an extension type, but there is no type definition for "${type}" in any subgraph.`, + { nodes: extensionAST }, + )); } private preUpgradeValidations(): void { @@ -614,10 +614,10 @@ class SchemaUpgrader { } if (!type.hasFields()) { if (type.isReferenced()) { - this.addError(ERRORS.TYPE_WITH_ONLY_UNUSED_EXTERNAL.err({ - message: `Type ${type} contains only external fields and all those fields are all unused (they do not appear in any @key, @provides or @requires).`, - nodes: type.sourceAST - })); + this.addError(ERRORS.TYPE_WITH_ONLY_UNUSED_EXTERNAL.err( + `Type ${type} contains only external fields and all those fields are all unused (they do not appear in any @key, @provides or @requires).`, + { nodes: type.sourceAST }, + )); } else { // The type only had unused externals, but it is also unreferenced in the subgraph. Unclear why // it was there in the first place, but we can remove it and move on. diff --git a/internals-js/src/supergraphs.ts b/internals-js/src/supergraphs.ts index 3aa2784b6..5951f10f4 100644 --- a/internals-js/src/supergraphs.ts +++ b/internals-js/src/supergraphs.ts @@ -1,10 +1,11 @@ -import { ASTNode, DocumentNode, GraphQLError } from "graphql"; +import { ASTNode, DocumentNode } from "graphql"; import { err } from '@apollo/core-schema'; import { ErrCoreCheckFailed, FeatureUrl, FeatureVersion } from "./coreSpec"; import { CoreFeature, CoreFeatures, Schema } from "./definitions"; import { joinIdentity, JoinSpecDefinition, JOIN_VERSIONS } from "./joinSpec"; import { buildSchema, buildSchemaFromAST } from "./buildSchema"; import { extractSubgraphsNamesAndUrlsFromSupergraph } from "./extractSubgraphsFromSupergraph"; +import { ERRORS } from "./error"; const SUPPORTED_FEATURES = new Set([ 'https://specs.apollo.dev/core/v0.1', @@ -78,15 +79,15 @@ function checkFeatureSupport(coreFeatures: CoreFeatures) { export function validateSupergraph(supergraph: Schema): [CoreFeatures, JoinSpecDefinition] { const coreFeatures = supergraph.coreFeatures; if (!coreFeatures) { - throw new GraphQLError("Invalid supergraph: must be a core schema"); + throw ERRORS.INVALID_FEDERATION_SUPERGRAPH.err("Invalid supergraph: must be a core schema"); } const joinFeature = coreFeatures.getByIdentity(joinIdentity); if (!joinFeature) { - throw new GraphQLError("Invalid supergraph: must use the join spec"); + throw ERRORS.INVALID_FEDERATION_SUPERGRAPH.err("Invalid supergraph: must use the join spec"); } const joinSpec = JOIN_VERSIONS.find(joinFeature.url.version); if (!joinSpec) { - throw new GraphQLError( + throw ERRORS.INVALID_FEDERATION_SUPERGRAPH.err( `Invalid supergraph: uses unsupported join spec version ${joinFeature.url.version} (supported versions: ${JOIN_VERSIONS.versions().join(', ')})`); } return [coreFeatures, joinSpec]; diff --git a/internals-js/src/tagSpec.ts b/internals-js/src/tagSpec.ts index 60550a11d..61ec52fbc 100644 --- a/internals-js/src/tagSpec.ts +++ b/internals-js/src/tagSpec.ts @@ -62,9 +62,8 @@ export class TagSpecDefinition extends FeatureDefinition { const hasValidNameArg = nameArg && sameType(nameArg.type!, new NonNullType(definition.schema().stringType())); const hasValidLocations = definition.locations.every(loc => this.tagLocations.includes(loc)); if (hasUnknownArguments || !hasValidNameArg || !hasValidLocations) { - return ERRORS.DIRECTIVE_DEFINITION_INVALID.err({ - message: `Found invalid @tag directive definition. Please ensure the directive definition in your schema's definitions matches the following:\n\t${this.printedTagDefinition}`, - } + return ERRORS.DIRECTIVE_DEFINITION_INVALID.err( + `Found invalid @tag directive definition. Please ensure the directive definition in your schema's definitions matches the following:\n\t${this.printedTagDefinition}`, ); } return undefined; diff --git a/internals-js/src/validate.ts b/internals-js/src/validate.ts index f00ef4d92..9cd03a361 100644 --- a/internals-js/src/validate.ts +++ b/internals-js/src/validate.ts @@ -16,10 +16,11 @@ import { UnionType, VariableDefinitions } from "./definitions"; -import { assertName, ASTNode, GraphQLError } from "graphql"; +import { assertName, ASTNode, GraphQLError, GraphQLErrorOptions } from "graphql"; import { isValidValue, valueToString } from "./values"; import { isIntrospectionName } from "./introspection"; import { isSubtype, sameType } from "./types"; +import { ERRORS } from "./error"; // Note really meant to be called manually as it is part of `Schema.validate`, but separated for core-organization reasons. // This mostly apply the validations that graphQL-js does in `validateSchema` which we don't reuse because it applies to @@ -35,7 +36,7 @@ class InputObjectCircularRefsValidator { // Position in the field path private readonly fieldPathIndexByTypeName = new Map(); - constructor(private readonly onError: (error: GraphQLError) => void) { + constructor(private readonly onError: (message: string, options: GraphQLErrorOptions) => void) { } detectCycles(type: InputObjectType) { @@ -57,10 +58,10 @@ class InputObjectCircularRefsValidator { } else { const cyclePath = this.fieldPath.slice(cycleIndex); const pathStr = cyclePath.map((fieldObj) => fieldObj.name).join('.'); - this.onError(new GraphQLError( + this.onError( `Cannot reference Input Object "${fieldType.name}" within itself through a series of non-null fields: "${pathStr}".`, - sourceASTs(...cyclePath) - )); + { nodes: sourceASTs(...cyclePath) }, + ); } this.fieldPath.pop(); } @@ -112,7 +113,7 @@ class Validator { // we found any type missing (in which case, there will be some errors and users should fix those // first). if (!this.hasMissingTypes) { - const refsValidator = new InputObjectCircularRefsValidator(e => this.errors.push(e)); + const refsValidator = new InputObjectCircularRefsValidator((msg, opts) => this.addError(msg, opts)); for (const type of this.schema.types()) { switch (type.kind) { case 'ObjectType': @@ -129,11 +130,15 @@ class Validator { return this.errors; } + private addError(message: string, options: GraphQLErrorOptions) { + this.errors.push(ERRORS.INVALID_GRAPHQL.err(message, options)); + } + private validateHasType(elt: { type?: Type, coordinate: string, sourceAST?: ASTNode }): boolean { // Note that this error can't happen if you parse the schema since it wouldn't be valid syntax, but it can happen for // programmatically constructed schema. if (!elt.type) { - this.errors.push(new GraphQLError(`Element ${elt.coordinate} does not have a type set`, elt.sourceAST)); + this.addError(`Element ${elt.coordinate} does not have a type set`, { nodes: elt.sourceAST }); this.hasMissingTypes = false; } return !!elt.type; @@ -146,13 +151,13 @@ class Validator { try { assertName(elt.name); } catch (e) { - this.errors.push(elt.sourceAST ? new GraphQLError(e.message, elt.sourceAST) : e); + this.addError(e.message, elt.sourceAST ? { nodes: elt.sourceAST } : {}); } } private validateObjectOrInterfaceType(type: ObjectType | InterfaceType) { if (!type.hasFields()) { - this.errors.push(new GraphQLError(`Type ${type.name} must define one or more fields.`, type.sourceAST)); + this.addError(`Type ${type.name} must define one or more fields.`, { nodes: type.sourceAST }); } for (const field of type.fields()) { this.validateName(field); @@ -165,47 +170,47 @@ class Validator { private validateImplementedInterfaces(type: ObjectType | InterfaceType) { if (type.implementsInterface(type.name)) { - this.errors.push(new GraphQLError( + this.addError( `Type ${type} cannot implement itself because it would create a circular reference.`, - sourceASTs(type, type.interfaceImplementation(type.name)!) - )); + { nodes: sourceASTs(type, type.interfaceImplementation(type.name)!) }, + ); } for (const itf of type.interfaces()) { for (const itfField of itf.fields()) { const field = type.field(itfField.name); if (!field) { - this.errors.push(new GraphQLError( + this.addError( `Interface field ${itfField.coordinate} expected but ${type} does not provide it.`, - sourceASTs(itfField, type) - )); + { nodes: sourceASTs(itfField, type) }, + ); continue; } // Note that we may not have validated the interface yet, so making sure we have a meaningful error // if the type is not set, even if that means a bit of cpu wasted since we'll re-check later (and // as many type as the interface is implemented); it's a cheap check anyway. if (this.validateHasType(itfField) && !isSubtype(itfField.type!, field.type!)) { - this.errors.push(new GraphQLError( + this.addError( `Interface field ${itfField.coordinate} expects type ${itfField.type} but ${field.coordinate} of type ${field.type} is not a proper subtype.`, - sourceASTs(itfField, field) - )); + { nodes: sourceASTs(itfField, field) }, + ); } for (const itfArg of itfField.arguments()) { const arg = field.argument(itfArg.name); if (!arg) { - this.errors.push(new GraphQLError( + this.addError( `Interface field argument ${itfArg.coordinate} expected but ${field.coordinate} does not provide it.`, - sourceASTs(itfArg, field) - )); + { nodes: sourceASTs(itfArg, field) }, + ); continue; } // Note that we could use contra-variance but as graphQL-js currently doesn't allow it, we mimic that. if (this.validateHasType(itfArg) && !sameType(itfArg.type!, arg.type!)) { - this.errors.push(new GraphQLError( + this.addError( `Interface field argument ${itfArg.coordinate} expects type ${itfArg.type} but ${arg.coordinate} is type ${arg.type}.`, - sourceASTs(itfArg, arg) - )); + { nodes: sourceASTs(itfArg, arg) }, + ); } } @@ -215,10 +220,10 @@ class Validator { continue; } if (arg.isRequired()) { - this.errors.push(new GraphQLError( + this.addError( `Field ${field.coordinate} includes required argument ${arg.name} that is missing from the Interface field ${itfField.coordinate}.`, - sourceASTs(arg, itfField) - )); + { nodes: sourceASTs(arg, itfField) }, + ); } } } @@ -227,12 +232,15 @@ class Validator { for (const itfOfItf of itf.interfaces()) { if (!type.implementsInterface(itfOfItf)) { if (itfOfItf === type) { - this.errors.push(new GraphQLError(`Type ${type} cannot implement ${itf} because it would create a circular reference.`, sourceASTs(type, itf))); + this.addError( + `Type ${type} cannot implement ${itf} because it would create a circular reference.`, + { nodes: sourceASTs(type, itf) }, + ); } else { - this.errors.push(new GraphQLError( + this.addError( `Type ${type} must implement ${itfOfItf} because it is implemented by ${itf}.`, - sourceASTs(type, itf, itfOfItf) - )); + { nodes: sourceASTs(type, itf, itfOfItf) }, + ); } } } @@ -241,7 +249,7 @@ class Validator { private validateInputObjectType(type: InputObjectType) { if (!type.hasFields()) { - this.errors.push(new GraphQLError(`Input Object type ${type.name} must define one or more fields.`, type.sourceAST)); + this.addError(`Input Object type ${type.name} must define one or more fields.`, { nodes: type.sourceAST }); } for (const field of type.fields()) { this.validateName(field); @@ -249,16 +257,16 @@ class Validator { continue; } if (field.isRequired() && field.isDeprecated()) { - this.errors.push(new GraphQLError( + this.addError( `Required input field ${field.coordinate} cannot be deprecated.`, - sourceASTs(field.appliedDirectivesOf('deprecated')[0], field) - )); + { nodes: sourceASTs(field.appliedDirectivesOf('deprecated')[0], field) }, + ); } if (field.defaultValue !== undefined && !isValidValue(field.defaultValue, field, new VariableDefinitions())) { - this.errors.push(new GraphQLError( + this.addError( `Invalid default value (got: ${valueToString(field.defaultValue)}) provided for input field ${field.coordinate} of type ${field.type}.`, - sourceASTs(field) - )); + { nodes: sourceASTs(field) }, + ); } } } @@ -269,36 +277,36 @@ class Validator { return; } if (arg.isRequired() && arg.isDeprecated()) { - this.errors.push(new GraphQLError( + this.addError( `Required argument ${arg.coordinate} cannot be deprecated.`, - sourceASTs(arg.appliedDirectivesOf('deprecated')[0], arg) - )); + { nodes: sourceASTs(arg.appliedDirectivesOf('deprecated')[0], arg) }, + ); } if (arg.defaultValue !== undefined && !isValidValue(arg.defaultValue, arg, new VariableDefinitions())) { - this.errors.push(new GraphQLError( + this.addError( `Invalid default value (got: ${valueToString(arg.defaultValue)}) provided for argument ${arg.coordinate} of type ${arg.type}.`, - sourceASTs(arg) - )); + { nodes: sourceASTs(arg) }, + ); } } private validateUnionType(type: UnionType) { if (type.membersCount() === 0) { - this.errors.push(new GraphQLError(`Union type ${type.coordinate} must define one or more member types.`, type.sourceAST)); + this.addError(`Union type ${type.coordinate} must define one or more member types.`, { nodes: type.sourceAST }); } } private validateEnumType(type: EnumType) { if (type.values.length === 0) { - this.errors.push(new GraphQLError(`Enum type ${type.coordinate} must define one or more values.`, type.sourceAST)); + this.addError(`Enum type ${type.coordinate} must define one or more values.`, { nodes: type.sourceAST }); } for (const value of type.values) { this.validateName(value); if (value.name === 'true' || value.name === 'false' || value.name === 'null') { - this.errors.push(new GraphQLError( + this.addError( `Enum type ${type.coordinate} cannot include value: ${value}.`, - value.sourceAST - )); + { nodes: value.sourceAST }, + ); } } } @@ -322,10 +330,10 @@ class Validator { const parentDesc = parent instanceof NamedSchemaElement ? parent.coordinate : 'schema'; - this.errors.push(new GraphQLError( + this.addError( `Invalid value for "${argument.coordinate}" of type "${argument.type}" in application of "${definition.coordinate}" to "${parentDesc}".`, - sourceASTs(application, argument) - )); + { nodes: sourceASTs(application, argument) }, + ); } } } diff --git a/internals-js/src/validation/KnownTypeNamesInFederationRule.ts b/internals-js/src/validation/KnownTypeNamesInFederationRule.ts index 84225e522..2800500cb 100644 --- a/internals-js/src/validation/KnownTypeNamesInFederationRule.ts +++ b/internals-js/src/validation/KnownTypeNamesInFederationRule.ts @@ -40,7 +40,7 @@ export function KnownTypeNamesInFederationRule( context.reportError( new GraphQLError( `Unknown type "${typeName}".` + didYouMean(suggestedTypes), - node, + { nodes: node }, ), ); } diff --git a/internals-js/src/values.ts b/internals-js/src/values.ts index 1b9401ff4..1c4a88ddc 100644 --- a/internals-js/src/values.ts +++ b/internals-js/src/values.ts @@ -34,6 +34,7 @@ import { didYouMean, suggestionList } from './suggestions'; import { inspect } from 'util'; import { sameType } from './types'; import { assert, assertUnreachable } from './utils'; +import { ERRORS } from './error'; // Per-GraphQL spec, max and value for an Int type. const MAX_INT = 2147483647; @@ -189,7 +190,7 @@ function applyDefaultValues(value: any, type: InputType): any { if (value === null) { if (isNonNullType(type)) { - throw new GraphQLError(`Invalid null value for non-null type ${type} while computing default values`); + throw ERRORS.INVALID_GRAPHQL.err(`Invalid null value for non-null type ${type} while computing default values`); } return null; } @@ -208,7 +209,7 @@ function applyDefaultValues(value: any, type: InputType): any { if (isInputObjectType(type)) { if (typeof value !== 'object') { - throw new GraphQLError(`Expected value for type ${type} to be an object, but is ${typeof value}.`); + throw ERRORS.INVALID_GRAPHQL.err(`Expected value for type ${type} to be an object, but is ${typeof value}.`); } const updated = Object.create(null); @@ -221,7 +222,7 @@ function applyDefaultValues(value: any, type: InputType): any { if (field.defaultValue !== undefined) { updated[field.name] = applyDefaultValues(field.defaultValue, field.type); } else if (isNonNullType(field.type)) { - throw new GraphQLError(`Field "${field.name}" of required type ${type} was not provided.`); + throw ERRORS.INVALID_GRAPHQL.err(`Field "${field.name}" of required type ${type} was not provided.`); } } else { updated[field.name] = applyDefaultValues(fieldValue, field.type); @@ -232,7 +233,7 @@ function applyDefaultValues(value: any, type: InputType): any { for (const fieldName of Object.keys(value)) { if (!type.field(fieldName)) { const suggestions = suggestionList(fieldName, type.fields().map(f => f.name)); - throw new GraphQLError(`Field "${fieldName}" is not defined by type "${type}".` + didYouMean(suggestions)); + throw ERRORS.INVALID_GRAPHQL.err(`Field "${fieldName}" is not defined by type "${type}".` + didYouMean(suggestions)); } } return updated; @@ -561,7 +562,7 @@ function isValidValueApplication(value: any, locationType: InputType, locationDe export function valueFromAST(node: ValueNode, expectedType: InputType): any { if (node.kind === Kind.NULL) { if (isNonNullType(expectedType)) { - throw new GraphQLError(`Invalid null value for non-null type "${expectedType}"`); + throw ERRORS.INVALID_GRAPHQL.err(`Invalid null value for non-null type "${expectedType}"`); } return null; } @@ -584,11 +585,11 @@ export function valueFromAST(node: ValueNode, expectedType: InputType): any { if (isIntType(expectedType)) { if (node.kind !== Kind.INT) { - throw new GraphQLError(`Int cannot represent non-integer value ${print(node)}.`); + throw ERRORS.INVALID_GRAPHQL.err(`Int cannot represent non-integer value ${print(node)}.`); } const i = parseInt(node.value, 10); if (i > MAX_INT || i < MIN_INT) { - throw new GraphQLError(`Int cannot represent non 32-bit signed integer value ${i}.`); + throw ERRORS.INVALID_GRAPHQL.err(`Int cannot represent non 32-bit signed integer value ${i}.`); } return i; } @@ -600,31 +601,31 @@ export function valueFromAST(node: ValueNode, expectedType: InputType): any { } else if (node.kind === Kind.FLOAT) { parsed = parseFloat(node.value); } else { - throw new GraphQLError(`Float can only represent integer or float value, but got a ${node.kind}.`); + throw ERRORS.INVALID_GRAPHQL.err(`Float can only represent integer or float value, but got a ${node.kind}.`); } if (!isFinite(parsed)) { - throw new GraphQLError( `Float cannot represent non numeric value ${parsed}.`); + throw ERRORS.INVALID_GRAPHQL.err( `Float cannot represent non numeric value ${parsed}.`); } return parsed; } if (isBooleanType(expectedType)) { if (node.kind !== Kind.BOOLEAN) { - throw new GraphQLError(`Boolean cannot represent a non boolean value ${print(node)}.`); + throw ERRORS.INVALID_GRAPHQL.err(`Boolean cannot represent a non boolean value ${print(node)}.`); } return node.value; } if (isStringType(expectedType)) { if (node.kind !== Kind.STRING) { - throw new GraphQLError(`String cannot represent non string value ${print(node)}.`); + throw ERRORS.INVALID_GRAPHQL.err(`String cannot represent non string value ${print(node)}.`); } return node.value; } if (isIDType(expectedType)) { if (node.kind !== Kind.STRING && node.kind !== Kind.INT) { - throw new GraphQLError(`ID cannot represent value ${print(node)}.`); + throw ERRORS.INVALID_GRAPHQL.err(`ID cannot represent value ${print(node)}.`); } return node.value; } @@ -635,14 +636,14 @@ export function valueFromAST(node: ValueNode, expectedType: InputType): any { if (isInputObjectType(expectedType)) { if (node.kind !== Kind.OBJECT) { - throw new GraphQLError(`Input Object Type ${expectedType} cannot represent non-object value ${print(node)}.`); + throw ERRORS.INVALID_GRAPHQL.err(`Input Object Type ${expectedType} cannot represent non-object value ${print(node)}.`); } const obj = Object.create(null); for (const f of node.fields) { const name = f.name.value; const field = expectedType.field(name); if (!field) { - throw new GraphQLError(`Unknown field "${name}" found in value for Input Object Type "${expectedType}".`); + throw ERRORS.INVALID_GRAPHQL.err(`Unknown field "${name}" found in value for Input Object Type "${expectedType}".`); } // TODO: as we recurse in sub-objects, we may get an error on a field value deep in the object // and the error will not be precise to where it happens. We could try to build the path to @@ -654,10 +655,10 @@ export function valueFromAST(node: ValueNode, expectedType: InputType): any { if (isEnumType(expectedType)) { if (node.kind !== Kind.STRING && node.kind !== Kind.ENUM) { - throw new GraphQLError(`Enum Type ${expectedType} cannot represent value ${print(node)}.`); + throw ERRORS.INVALID_GRAPHQL.err(`Enum Type ${expectedType} cannot represent value ${print(node)}.`); } if (!expectedType.value(node.value)) { - throw new GraphQLError(`Enum Type ${expectedType} has no value ${node.value}.`); + throw ERRORS.INVALID_GRAPHQL.err(`Enum Type ${expectedType} has no value ${node.value}.`); } return node.value; } @@ -699,7 +700,7 @@ export function argumentsFromAST( const name = argNode.name.value; const expectedType = argsDefiner.argument(name)?.type; if (!expectedType) { - throw new GraphQLError( + throw ERRORS.INVALID_GRAPHQL.err( `Unknown argument "${name}" found in value: ${context} has no argument named "${name}"` ); } @@ -707,7 +708,7 @@ export function argumentsFromAST( values[name] = valueFromAST(argNode.value, expectedType); } catch (e) { if (e instanceof GraphQLError) { - throw new GraphQLError(`Invalid value for argument ${name}: ${e.message}`); + throw ERRORS.INVALID_GRAPHQL.err(`Invalid value for argument ${name}: ${e.message}`); } throw e; } diff --git a/query-planner-js/src/buildPlan.ts b/query-planner-js/src/buildPlan.ts index 013ca9422..ecd63434c 100644 --- a/query-planner-js/src/buildPlan.ts +++ b/query-planner-js/src/buildPlan.ts @@ -42,6 +42,7 @@ import { directiveApplicationsSubstraction, conditionalDirectivesInOperationPath, MultiMap, + ERRORS, } from "@apollo/federation-internals"; import { advanceSimultaneousPathsWithOperation, @@ -74,7 +75,7 @@ import { terminateWithNonRequestedTypenameField, getLocallySatisfiableKey, } from "@apollo/query-graphs"; -import { stripIgnoredCharacters, print, GraphQLError, parse, OperationTypeNode } from "graphql"; +import { stripIgnoredCharacters, print, parse, OperationTypeNode } from "graphql"; import { QueryPlan, ResponsePath, SequenceNode, PlanNode, ParallelNode, FetchNode, trimSelectionNodes } from "./QueryPlan"; const debug = newDebugLogger('plan'); @@ -1470,9 +1471,9 @@ interface FetchGroupProcessor export function computeQueryPlan(supergraphSchema: Schema, federatedQueryGraph: QueryGraph, operation: Operation): QueryPlan { if (operation.rootKind === 'subscription') { - throw new GraphQLError( - 'Query planning does not support subscriptions for now.', - [parse(operation.toString())], + throw ERRORS.UNSUPPORTED_FEATURE.err( + 'Query planning does not currently support subscriptions.', + { nodes: [parse(operation.toString())] }, ); } // We expand all fragments. This might merge a number of common branches and save us