chore: improve incompatible types error

composition/src/errors/errors.ts

@@ -7,6 +7,7 @@ import {
 } from '../schema-building/types';
 import {
   IncompatibleMergedTypesErrorParams,
+  IncompatibleParentTypeMergeErrorParams,
   InvalidNamedTypeErrorParams,
   InvalidRootTypeFieldEventsDirectiveData,
   OneOfRequiredFieldsErrorParams,
@@ -338,20 +339,21 @@ export function unexpectedEdgeFatalError(typeName: string, edgeNames: Array<stri
   );
 }
 
-export function incompatibleParentKindMergeError(
-  parentTypeName: string,
-  expectedTypeString: string,
-  actualTypeString: string,
-): Error {
-  return new Error(
-    ` When merging types, expected "${parentTypeName}" to be type "${expectedTypeString}" but received "${actualTypeString}".`,
-  );
-}
+const interfaceObject = `"Interface Object" (an "Object" type that also defines the "@interfaceObject" directive)`;
 
-export function fieldTypeMergeFatalError(fieldName: string) {
+export function incompatibleParentTypeMergeError({
+  existingData,
+  incomingNodeType,
+  incomingSubgraphName,
+}: IncompatibleParentTypeMergeErrorParams): Error {
+  const existingSubgraphNames = [...existingData.subgraphNames];
+  const nodeType = incomingNodeType ? `"${incomingNodeType}"` : interfaceObject;
   return new Error(
-    `Fatal: Unsuccessfully merged the cross-subgraph types of field "${fieldName}"` +
-      ` without producing a type error object.`,
+    ` "${existingData.name}" is defined using incompatible types across subgraphs.` +
+      ` It is defined as type "${kindToNodeType(existingData.kind)}" in subgraph` +
+      (existingSubgraphNames.length > 1 ? 's' : '') +
+      ` "${existingSubgraphNames.join(QUOTATION_JOIN)}" but type ${nodeType} in subgraph` +
+      ` "${incomingSubgraphName}".`,
   );
 }
 
@@ -378,7 +380,7 @@ export function unexpectedParentKindForChildError(
   childTypeString: string,
 ): Error {
   return new Error(
-    ` Expected "${parentTypeName}" to be type ${expectedTypeString} but received "${actualTypeString}"` +
+    ` Expected "${parentTypeName}" to be type "${expectedTypeString}" but received "${actualTypeString}"` +
       ` when handling child "${childName}" of type "${childTypeString}".`,
   );
 }

composition/src/errors/types.ts

@@ -1,5 +1,5 @@
 import { FieldData, InputValueData, ParentDefinitionData } from '../schema-building/types';
-import { FieldName, TypeName } from '../types/types';
+import { FieldName, SubgraphName, TypeName } from '../types/types';
 
 export type InvalidRootTypeFieldEventsDirectiveData = {
   definesDirectives: boolean;
@@ -34,3 +34,9 @@ export type OneOfRequiredFieldsErrorParams = {
   requiredFieldNames: Array<FieldName>;
   typeName: TypeName;
 };
+
+export type IncompatibleParentTypeMergeErrorParams = {
+  existingData: ParentDefinitionData;
+  incomingSubgraphName: SubgraphName;
+  incomingNodeType?: string;
+};

composition/src/v1/federation/federation-factory.ts

@@ -33,7 +33,7 @@ import {
   incompatibleFederatedFieldNamedTypeError,
   incompatibleMergedTypesError,
   incompatibleParentKindFatalError,
-  incompatibleParentKindMergeError,
+  incompatibleParentTypeMergeError,
   incompatibleSharedEnumError,
   invalidFieldShareabilityError,
   invalidImplementedTypeError,
@@ -1207,7 +1207,7 @@ export class FederationFactory {
     return existingData;
   }
 
-  upsertParentDefinitionData(incomingData: ParentDefinitionData, subgraphName: string) {
+  upsertParentDefinitionData(incomingData: ParentDefinitionData, subgraphName: SubgraphName) {
     const entityInterfaceData = this.entityInterfaceFederationDataByTypeName.get(incomingData.name);
     const existingData = this.parentDefinitionDataByTypeName.get(incomingData.name);
     const targetData = this.getParentTargetData({ existingData, incomingData });
@@ -1217,6 +1217,15 @@ export class FederationFactory {
       this.inaccessibleCoords.add(targetData.name);
     }
     if (entityInterfaceData && entityInterfaceData.interfaceObjectSubgraphNames.has(subgraphName)) {
+      if (existingData && existingData.kind !== Kind.INTERFACE_TYPE_DEFINITION) {
+        this.errors.push(
+          incompatibleParentTypeMergeError({
+            existingData,
+            incomingSubgraphName: subgraphName,
+          }),
+        );
+        return;
+      }
       targetData.kind = Kind.INTERFACE_TYPE_DEFINITION;
       targetData.node.kind = Kind.INTERFACE_TYPE_DEFINITION;
     }
@@ -1232,11 +1241,11 @@ export class FederationFactory {
         incomingData.kind !== Kind.OBJECT_TYPE_DEFINITION
       ) {
         this.errors.push(
-          incompatibleParentKindMergeError(
-            targetData.name,
-            kindToNodeType(targetData.kind),
-            kindToNodeType(incomingData.kind),
-          ),
+          incompatibleParentTypeMergeError({
+            existingData: targetData,
+            incomingNodeType: kindToNodeType(incomingData.kind),
+            incomingSubgraphName: subgraphName,
+          }),
         );
         return;
       }

composition/tests/v1/entity-interface.test.ts

@@ -2,17 +2,22 @@ import {
   ConfigurationData,
   EntityInterfaceFederationData,
   EntityInterfaceSubgraphData,
+  incompatibleParentTypeMergeError,
   INTERFACE,
+  InterfaceDefinitionData,
   InvalidEntityInterface,
+  OBJECT,
+  ObjectDefinitionData,
   ROUTER_COMPATIBILITY_VERSION_ONE,
   SimpleFieldData,
   Subgraph,
+  SubgraphName,
   undefinedEntityInterfaceImplementationsError,
 } from '../../src';
 import { describe, expect, test } from 'vitest';
 import { versionOneRouterDefinitions, versionTwoRouterDefinitions } from './utils/utils';
 
-import { parse } from 'graphql';
+import { Kind, parse } from 'graphql';
 import {
   federateSubgraphsFailure,
   federateSubgraphsSuccess,
@@ -256,7 +261,7 @@ describe('Entity Interface Tests', () => {
     );
   });
 
-  test('that  @interfaceObject works correctly with implicit key checks #1.2', () => {
+  test('that @interfaceObject works correctly with implicit key checks #1.2', () => {
     const result = federateSubgraphsSuccess([subgraphJ, subgraphI], ROUTER_COMPATIBILITY_VERSION_ONE);
     expect(result.success).toBe(true);
     expect(schemaToSortedNormalizedString(result.federatedGraphSchema)).toBe(
@@ -300,6 +305,44 @@ describe('Entity Interface Tests', () => {
     );
   });
 
+  test('that error is returned if an entity Interface is defined as a regular Object type #1', () => {
+    const { errors } = federateSubgraphsFailure([kaaa, kaab, kaac], ROUTER_COMPATIBILITY_VERSION_ONE);
+    expect(errors).toHaveLength(1);
+    const existingData = {
+      kind: Kind.INTERFACE_TYPE_DEFINITION,
+      name: INTERFACE,
+      subgraphNames: new Set<SubgraphName>([kaaa.name, kaab.name]),
+    } as InterfaceDefinitionData;
+    expect(errors).toStrictEqual([
+      incompatibleParentTypeMergeError({
+        existingData,
+        incomingNodeType: OBJECT,
+        incomingSubgraphName: kaac.name,
+      }),
+    ]);
+  });
+
+  test('that error is returned if an entity Interface is defined as a regular Object type #2', () => {
+    const { errors } = federateSubgraphsFailure([kaac, kaab, kaaa], ROUTER_COMPATIBILITY_VERSION_ONE);
+    expect(errors).toHaveLength(2);
+    const existingData = {
+      kind: Kind.OBJECT_TYPE_DEFINITION,
+      name: INTERFACE,
+      subgraphNames: new Set<SubgraphName>([kaac.name]),
+    } as ObjectDefinitionData;
+    expect(errors).toStrictEqual([
+      incompatibleParentTypeMergeError({
+        existingData,
+        incomingSubgraphName: kaab.name,
+      }),
+      incompatibleParentTypeMergeError({
+        existingData,
+        incomingNodeType: INTERFACE,
+        incomingSubgraphName: kaaa.name,
+      }),
+    ]);
+  });
+
   test.skip('that an error is returned if a type declared with @interfaceObject is not an interface in other subgraphs', () => {});
 
   test.skip('that an error is returned if a type declared with @interfaceObject is not an entity', () => {});
@@ -504,3 +547,37 @@ const subgraphJ: Subgraph = {
     }
   `),
 };
+
+const kaaa: Subgraph = {
+  name: 'kaaa',
+  url: '',
+  definitions: parse(`
+    interface Interface @key(fields: "id", resolvable: false) {
+      id: ID!
+    }
+
+    type Query {
+      a: ID
+    }
+  `),
+};
+
+const kaab: Subgraph = {
+  name: 'kaab',
+  url: '',
+  definitions: parse(`
+    type Interface @key(fields: "id", resolvable: false) @interfaceObject {
+      id: ID!
+    }
+  `),
+};
+
+const kaac: Subgraph = {
+  name: 'kaac',
+  url: '',
+  definitions: parse(`
+    type Interface @key(fields: "id", resolvable: false) {
+      id: ID!
+    }
+  `),
+};

composition/tests/v1/federation-factory.test.ts

@@ -1,14 +1,18 @@
 import {
-  incompatibleParentKindMergeError,
+  incompatibleParentTypeMergeError,
   INPUT_OBJECT,
+  InputObjectDefinitionData,
   invalidSubgraphNamesError,
   noBaseDefinitionForExtensionError,
   noQueryRootTypeError,
   OBJECT,
+  ObjectDefinitionData,
   parse,
   ROUTER_COMPATIBILITY_VERSION_ONE,
   SCALAR,
+  ScalarDefinitionData,
   Subgraph,
+  SubgraphName,
 } from '../../src';
 import { describe, expect, test } from 'vitest';
 import {
@@ -28,6 +32,7 @@ import {
   normalizeString,
   schemaToSortedNormalizedString,
 } from '../utils/utils';
+import { Kind } from 'graphql';
 
 // @ts-ignore
 const __filename = fileURLToPath(import.meta.url);
@@ -880,26 +885,70 @@ describe('FederationFactory tests', () => {
   test('that an error is returned when merging incompatible types #1.1', () => {
     const result = federateSubgraphsFailure([subgraphR, subgraphS], ROUTER_COMPATIBILITY_VERSION_ONE);
     expect(result.errors).toHaveLength(1);
-    expect(result.errors[0]).toStrictEqual(incompatibleParentKindMergeError(OBJECT, SCALAR, OBJECT));
+    const existingData = {
+      kind: Kind.SCALAR_TYPE_DEFINITION,
+      name: OBJECT,
+      subgraphNames: new Set<SubgraphName>([subgraphR.name]),
+    } as ScalarDefinitionData;
+    expect(result.errors).toStrictEqual([
+      incompatibleParentTypeMergeError({
+        existingData,
+        incomingNodeType: OBJECT,
+        incomingSubgraphName: subgraphS.name,
+      }),
+    ]);
   });
 
   test('that an error is returned when merging incompatible types #1.2', () => {
     const result = federateSubgraphsFailure([subgraphS, subgraphR], ROUTER_COMPATIBILITY_VERSION_ONE);
     expect(result.errors).toHaveLength(1);
-    expect(result.errors[0]).toStrictEqual(incompatibleParentKindMergeError(OBJECT, OBJECT, SCALAR));
+    const existingData = {
+      kind: Kind.OBJECT_TYPE_DEFINITION,
+      name: OBJECT,
+      subgraphNames: new Set<SubgraphName>([subgraphS.name]),
+    } as ObjectDefinitionData;
+    expect(result.errors).toStrictEqual([
+      incompatibleParentTypeMergeError({
+        existingData,
+        incomingNodeType: SCALAR,
+        incomingSubgraphName: subgraphR.name,
+      }),
+    ]);
   });
 
   test('that an error is returned when merging an object extension orphan with an incompatible base type #1.1', () => {
     const result = federateSubgraphsFailure([subgraphT, subgraphU], ROUTER_COMPATIBILITY_VERSION_ONE);
     expect(result.errors).toHaveLength(2);
-    expect(result.errors[0]).toStrictEqual(incompatibleParentKindMergeError(OBJECT, OBJECT, INPUT_OBJECT));
-    expect(result.errors[1]).toStrictEqual(noBaseDefinitionForExtensionError(OBJECT, OBJECT));
+    const existingData = {
+      kind: Kind.OBJECT_TYPE_DEFINITION,
+      name: OBJECT,
+      subgraphNames: new Set<SubgraphName>([subgraphT.name]),
+    } as ObjectDefinitionData;
+    expect(result.errors).toStrictEqual([
+      incompatibleParentTypeMergeError({
+        existingData,
+        incomingNodeType: INPUT_OBJECT,
+        incomingSubgraphName: subgraphU.name,
+      }),
+      noBaseDefinitionForExtensionError(OBJECT, OBJECT),
+    ]);
   });
 
   test('that an error is returned when merging an object extension orphan with an incompatible base type #1.2', () => {
     const result = federateSubgraphsFailure([subgraphU, subgraphT], ROUTER_COMPATIBILITY_VERSION_ONE);
     expect(result.errors).toHaveLength(1);
-    expect(result.errors[0]).toStrictEqual(incompatibleParentKindMergeError(OBJECT, INPUT_OBJECT, OBJECT));
+    const existingData = {
+      kind: Kind.INPUT_OBJECT_TYPE_DEFINITION,
+      name: OBJECT,
+      subgraphNames: new Set<SubgraphName>([subgraphU.name]),
+    } as InputObjectDefinitionData;
+    expect(result.errors).toStrictEqual([
+      incompatibleParentTypeMergeError({
+        existingData,
+        incomingNodeType: OBJECT,
+        incomingSubgraphName: subgraphT.name,
+      }),
+    ]);
   });
 
   test('that renaming a root type also renames field return types of the same type #1.1', () => {