Aggregate errors with @inaccessible (#1563)

Previously, `removeInaccessibleElements` would throw an error the first time it encountered an inaccessible mismatch for types & fields. With this change, the code will continue to parse the schema and report an `ErrGraphQLValidationFailed` that contains all the mismatches.

Initially, I attempted to use `AggregateError` but node12 & node14 don't support es2019, which is when `AggregateError` was introduced. `ErrGraphQLValidationFailed` is already available in our codebase and supports multiple errors.

internals-js/src/__tests__/removeInaccessibleElements.test.ts

@@ -1,4 +1,5 @@
-import { ObjectType } from '../definitions';
+import { GraphQLErrorExt } from "@apollo/core-schema/dist/error";
+import { errorCauses, ObjectType } from '../definitions';
 import { buildSchema } from '../buildSchema';
 import { removeInaccessibleElements } from '../inaccessibleSpec';
 
@@ -178,11 +179,63 @@ describe('removeInaccessibleElements', () => {
  union Bar = Foo
  `);
 
- expect(() => {
- removeInaccessibleElements(schema);
- }).toThrow(
- `Field "Query.fooField" returns @inaccessible type "Foo" without being marked @inaccessible itself`,
- );
+ try {
+ // Assert that an AggregateError is thrown, then allow the error to be caught for further validation
+ expect(removeInaccessibleElements(schema)).toThrow(GraphQLErrorExt);
+ } catch (err) {
+ const causes = errorCauses(err);
+ expect(causes).toBeTruthy();
+ expect(err.causes).toHaveLength(1);
+ expect(err.causes[0].toString()).toMatch(
+ `Field "Query.fooField" returns @inaccessible type "Foo" without being marked @inaccessible itself.`
+ );
+ }
+ });
+
+ it(`throws when there are multiple problems`, () => {
+ const schema = buildSchema(`
+ directive @core(feature: String!, as: String, for: core__Purpose) repeatable on SCHEMA
+
+ directive @inaccessible on FIELD_DEFINITION | OBJECT | INTERFACE | UNION
+
+ schema
+ @core(feature: "https://specs.apollo.dev/core/v0.2")
+ @core(feature: "https://specs.apollo.dev/inaccessible/v0.1")
+ {
+ query: Query
+ }
+
+ enum core__Purpose {
+ EXECUTION
+ SECURITY
+ }
+
+ type Query {
+ fooField: Foo
+ fooderationField: Foo
+ }
+
+ type Foo @inaccessible {
+ someField: String
+ }
+
+ union Bar = Foo
+ `);
+
+ try {
+ // Assert that an AggregateError is thrown, then allow the error to be caught for further validation
+ expect(removeInaccessibleElements(schema)).toThrow(GraphQLErrorExt);
+ } catch (err) {
+ const causes = errorCauses(err);
+ expect(causes).toBeTruthy();
+ expect(err.causes).toHaveLength(2);
+ expect(err.causes[0].toString()).toMatch(
+ `Field "Query.fooField" returns @inaccessible type "Foo" without being marked @inaccessible itself.`
+ );
+ expect(err.causes[1].toString()).toMatch(
+ `Field "Query.fooderationField" returns @inaccessible type "Foo" without being marked @inaccessible itself.`
+ );
+ }
  });
 
  it(`removes @inaccessible query root type`, () => {

internals-js/src/definitions.ts

@@ -43,10 +43,11 @@ import { didYouMean, suggestionList } from "./suggestions";
 import { withModifiedErrorMessage } from "./error";
 
 const validationErrorCode = 'GraphQLValidationFailed';
+const DEFAULT_VALIDATION_ERROR_MESSAGE = 'The schema is not a valid GraphQL schema.';
 
-export const ErrGraphQLValidationFailed = (causes: GraphQLError[]) =>
+export const ErrGraphQLValidationFailed = (causes: GraphQLError[], message: string = DEFAULT_VALIDATION_ERROR_MESSAGE) =>
  err(validationErrorCode, {
- message: 'The schema is not a valid GraphQL schema',
+ message,
  causes
  });
 

internals-js/src/inaccessibleSpec.ts

@@ -1,6 +1,7 @@
 import { FeatureDefinition, FeatureDefinitions, FeatureUrl, FeatureVersion } from "./coreSpec";
 import {
  DirectiveDefinition,
+ ErrGraphQLValidationFailed,
  FieldDefinition,
  isCompositeType,
  isInterfaceType,
@@ -66,6 +67,7 @@ export function removeInaccessibleElements(schema: Schema) {
  );
  }
 
+ const errors = [];
  for (const type of schema.types()) {
  // @inaccessible can only be on composite types.
  if (!isCompositeType(type)) {
@@ -81,14 +83,14 @@ export function removeInaccessibleElements(schema: Schema) {
  if (!reference.hasAppliedDirective(inaccessibleDirective)) {
  // We ship the inaccessible type and it's invalid reference in the extensions so composition can extract those and add proper links
  // in the subgraphs for those elements.
- throw ERRORS.REFERENCED_INACCESSIBLE.err({
+ errors.push(ERRORS.REFERENCED_INACCESSIBLE.err({
  message: `Field "${reference.coordinate}" returns @inaccessible type "${type}" without being marked @inaccessible itself.`,
  nodes: reference.sourceAST,
  extensions: {
  "inaccessible_element": type.coordinate,
  "inaccessible_reference": reference.coordinate,
  }
- });
+ }));
  }
  }
  // Other references can be:
@@ -101,4 +103,8 @@ export function removeInaccessibleElements(schema: Schema) {
  toRemove.forEach(f => f.remove());
  }
  }
+
+ if (errors.length > 0) {
+ throw ErrGraphQLValidationFailed(errors, `Schema has ${errors.length === 1 ? 'an invalid use' : 'invalid uses'} of the @inaccessible directive.`);
+ }
 }