Preset Unknown types errors as GraphQLError (#68)

Previously, some logic paths were throwing an exception
the-guild-org · Jul 15, 2024 · 51dd57a · 51dd57a
1 parent d097a2d
commit 51dd57a
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 8 deletions.
diff --git a/.changeset/sour-adults-sit.md b/.changeset/sour-adults-sit.md
@@ -0,0 +1,6 @@
+---
+'@theguild/federation-composition': patch
+---
+
+Unknown types are now always reported as GraphQLError (previously in some logic paths, it was an
+exception).
diff --git a/__tests__/subgraph/key-fields.spec.ts b/__tests__/subgraph/key-fields.spec.ts
@@ -187,4 +187,37 @@ testVersions((api, version) => {
       ]),
     );
   });
+
+  test('missing _FieldSet definition', () => {
+    expect(
+      api.composeServices([
+        {
+          name: 'foo',
+          typeDefs: graphql`
+            directive @key(fields: _FieldSet!) repeatable on OBJECT | INTERFACE
+
+            type Query {
+              users: [User]
+            }
+
+            type User @key(fields: "id") {
+              id: ID!
+              name: String!
+            }
+          `,
+        },
+      ]),
+    ).toEqual(
+      expect.objectContaining({
+        errors: expect.arrayContaining([
+          expect.objectContaining({
+            message: expect.stringContaining(`[foo] Unknown type _FieldSet`),
+            extensions: expect.objectContaining({
+              code: 'INVALID_GRAPHQL',
+            }),
+          }),
+        ]),
+      }),
+    );
+  });
 });
diff --git a/src/subgraph/validation/validate-state.ts b/src/subgraph/validation/validate-state.ts
@@ -34,6 +34,8 @@ const specifiedScalars = new Set(specifiedScalarTypes.map(t => t.name));
 
 type ReportErrorFn = (message: string) => void;
 
+const SKIP = Symbol('skip');
+
 export function validateSubgraphState(state: SubgraphState, context: SubgraphValidationContext) {
   const errors: GraphQLError[] = [];
 
@@ -130,7 +132,13 @@ function validateDirectives(
           continue;
         }
 
-        if (!isInputType(state, argInputTypeName)) {
+        const isInput = isInputType(state, argInputTypeName);
+
+        if (isInput === SKIP) {
+          continue;
+        }
+
+        if (!isInput) {
           reportError(
             `The type of @${directive.name}(${arg.name}:) must be Input Type ` +
               `but got: ${arg.type}.`,
@@ -255,7 +263,13 @@ function validateFields(
     }
 
     // Ensure the type is an output type
-    if (!isOutputType(state, fieldTypeName)) {
+    const isOutput = isOutputType(state, fieldTypeName);
+
+    if (isOutput === SKIP) {
+      continue;
+    }
+
+    if (!isOutput) {
       reportError(
         `The type of "${type.name}.${field.name}" must be Output Type but got: "${field.type}".`,
       );
@@ -278,7 +292,13 @@ function validateFields(
       }
 
       // Ensure the type is an input type
-      if (!isInputType(state, argTypeName)) {
+      const isInput = isInputType(state, argTypeName);
+
+      if (isInput === SKIP) {
+        continue;
+      }
+
+      if (!isInput) {
         const isList = arg.type.endsWith(']');
         const isNonNull = arg.type.endsWith('!');
         const extra = isList ? ', a ListType' : isNonNull ? ', a NonNullType' : '';
@@ -459,7 +479,13 @@ function validateInputFields(
     }
 
     // Ensure the type is an input type
-    if (!isInputType(state, fieldTypeName)) {
+    const isInput = isInputType(state, fieldTypeName);
+
+    if (isInput === SKIP) {
+      continue;
+    }
+
+    if (!isInput) {
       const isList = field.type.endsWith(']');
       const isNonNull = field.type.endsWith('!');
       const extra = isList ? ', a ListType' : isNonNull ? ', a NonNullType' : '';
@@ -766,7 +792,7 @@ function isRequiredInputField(arg: InputField): boolean {
   return isNonNull(arg.type) && arg.defaultValue === undefined;
 }
 
-function isOutputType(state: SubgraphState, typeName: string): boolean {
+function isOutputType(state: SubgraphState, typeName: string) {
   const type = state.types.get(typeName);
 
   if (!type) {
@@ -776,15 +802,16 @@ function isOutputType(state: SubgraphState, typeName: string): boolean {
 
     // The existence of the type was already validated.
     // See: KnownTypeNamesRule
-    throw new Error(`Expected to find ${typeName} type`);
+    // No need to throw an exception here, because it affects the rest of the validation logic.
+    return SKIP;
   }
 
   // Only input types are not output types (scalars and enums are in both).
   // This is a quick check, to make it least expensive possible.
   return !isInputObjectType(type);
 }
 
-export function isInputType(state: SubgraphState, typeName: string): boolean {
+export function isInputType(state: SubgraphState, typeName: string) {
   const type = state.types.get(typeName);
 
   if (!type) {
@@ -794,7 +821,8 @@ export function isInputType(state: SubgraphState, typeName: string): boolean {
 
     // The existence of the type was already validated.
     // See: KnownTypeNamesRule
-    throw new Error(`Expected to find ${typeName} type`);
+    // No need to throw an exception here, because it affects the rest of the validation logic.
+    return SKIP;
   }
 
   return isScalarType(type) || isEnumType(type) || isInputObjectType(type);