Merge pull request #30 from wayfair-incubator/gwardwell_improve_empty_froid_key_errors

Improves error messages when an empty key is generated

Author: gwardwell

CHANGELOG.md

@@ -6,6 +6,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to
 [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [v3.2.2] - 2024-08-20
+
+### Fixed
+
+- Improves the error message surfaced in cases where FROID generates an empty
+  key.
+
 ## [v3.2.1] - 2024-04-29
 
 ### Fixed

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@wayfair/node-froid",
-  "version": "3.2.1",
+  "version": "3.2.2",
   "description": "Federated GQL Relay Object Identification implementation",
   "main": "dist/index.js",
   "types": "dist/index.d.ts",

src/schema/Key.ts

@@ -71,7 +71,7 @@ export class Key {
       parseableField = Key.getKeyDirectiveFields(parseableField);
     }
     (
-      Key.parseKeyFields(parseableField)
+      Key.parseKeyFields(this.typename, parseableField)
         .definitions[0] as OperationDefinitionNode
     )?.selectionSet?.selections?.forEach((selection) =>
       this.addSelection(selection)
@@ -174,6 +174,7 @@ export class Key {
    */
   public toString(): string {
     return Key.getSortedSelectionSetFields(
+      this.typename,
       this._fields.map((field) => field.toString()).join(' ')
     );
   }
@@ -210,10 +211,22 @@ export class Key {
    * Parses a key fields string into AST.
    *
    * @param {string} keyFields - The key fields string
+   * @param {string} typename - The typename of the node the directive belongs to
    * @returns {DocumentNode} The key fields represented in AST
    */
-  private static parseKeyFields(keyFields: string): DocumentNode {
-    return parse(`{${keyFields}}`, {noLocation: true});
+  private static parseKeyFields(
+    typename: string,
+    keyFields: string
+  ): DocumentNode {
+    try {
+      return parse(`{${keyFields}}`, {noLocation: true});
+    } catch (error) {
+      throw new Error(
+        `Failed to parse key fields "${keyFields}" for type "${typename}" due to error: ${
+          (error as Error).message
+        }`
+      );
+    }
   }
 
   /**
@@ -234,12 +247,18 @@ export class Key {
    * Sorts the selection set fields.
    *
    * @param {string} fields - The selection set fields.
+   * @param {string} typename - The typename of the node the directive belongs to
    * @returns {string} The sorted selection set fields.
    */
-  public static getSortedSelectionSetFields(fields: string): string {
+  public static getSortedSelectionSetFields(
+    typename: string,
+    fields: string
+  ): string {
     const selections = Key.sortSelectionSetByNameAscending(
-      (Key.parseKeyFields(fields).definitions[0] as OperationDefinitionNode)
-        .selectionSet
+      (
+        Key.parseKeyFields(typename, fields)
+          .definitions[0] as OperationDefinitionNode
+      ).selectionSet
     );
     return Key.formatSelectionSetFields(print(selections));
   }

src/schema/__tests__/FroidSchema.test.ts

@@ -3009,6 +3009,31 @@ describe('FroidSchema class', () => {
     );
   });
 
+  it('provides a helpful message when failing to stringify the FROID schema', () => {
+    const productSchema = gql`
+      type Product @key(fields: "__typename") {
+        name: String
+      }
+    `;
+    const subgraphs = new Map();
+    subgraphs.set('product-subgraph', productSchema);
+
+    let errorMessage;
+    try {
+      generateSchema({
+        subgraphs,
+        froidSubgraphName: 'relay-subgraph',
+        federationVersion: FED2_DEFAULT_VERSION,
+      });
+    } catch (error) {
+      errorMessage = error.message;
+    }
+
+    expect(errorMessage).toMatch(
+      'Failed to parse key fields "" for type "Product"'
+    );
+  });
+
   it('generates schema document AST', () => {
     const productSchema = gql`
       type Product @key(fields: "upc") {

src/schema/sortDocumentAst.ts

@@ -193,7 +193,11 @@ function sortChildren(node: NamedNode): NamedNode {
   }
 
   const directives = node?.directives
-    ? {directives: sortDirectives(sortKeyDirectiveFields(node.directives))}
+    ? {
+        directives: sortDirectives(
+          sortKeyDirectiveFields(node.name.value, node.directives)
+        ),
+      }
     : {};
 
   if (
@@ -254,10 +258,12 @@ function sortChildren(node: NamedNode): NamedNode {
 /**
  * Sorts the `fields` argument of any @key directives in a list of directives.
  *
+ * @param {string} typename - The typename of the node the directive belongs to
  * @param {ConstDirectiveNode[]} directives - A list of directives that may include the @key directive
  * @returns {ConstDirectiveNode[]} The list of directives after any key fields have been sorted
  */
 function sortKeyDirectiveFields(
+  typename: string,
   directives: readonly ConstDirectiveNode[]
 ): readonly ConstDirectiveNode[] {
   return directives.map((directive) => {
@@ -285,7 +291,7 @@ function sortKeyDirectiveFields(
           ...fieldsArgument,
           value: {
             ...fieldsArgument.value,
-            value: Key.getSortedSelectionSetFields(fields),
+            value: Key.getSortedSelectionSetFields(typename, fields),
           },
         },
       ],