Better handling of syntax errors encountered while generating the per…

…sisted query manifest (#345)

.changeset/tidy-vans-deliver.md

@@ -0,0 +1,5 @@
+---
+"@apollo/generate-persisted-query-manifest": patch
+---
+
+Better error reporting when an error is raised while parsing a source file for GraphQL query strings (such as a syntax error). Previously the stack trace was reported to the console with little to no identifying information as to which file caused the error. The filename is now reported similarly to other errors encountered when running the CLI. An additional improvement is that it will now gather all errors, including syntax errors in a single pass so that syntax errors do not halt the program in place.

packages/generate-persisted-query-manifest/src/@types/lib.es5.d.ts

@@ -0,0 +1,28 @@
+/**
+ * Fixes https://github.com/microsoft/TypeScript/issues/16655 for `Array.prototype.filter()`
+ * For example, using the fix the type of `bar` is `string[]` in the below snippet as it should be.
+ *
+ *  const foo: (string | null | undefined)[] = [];
+ *  const bar = foo.filter(Boolean);
+ *
+ * For related definitions, see https://github.com/microsoft/TypeScript/blob/master/src/lib/es5.d.ts
+ *
+ * Original licenses apply, see
+ *  - https://github.com/microsoft/TypeScript/blob/master/LICENSE.txt
+ *  - https://stackoverflow.com/help/licensing
+ */
+
+/** See https://stackoverflow.com/a/51390763/1470607  */
+type Falsy = false | 0 | "" | null | undefined;
+
+interface Array<T> {
+  /**
+   * Returns the elements of an array that meet the condition specified in a callback function.
+   * @param predicate A function that accepts up to three arguments. The filter method calls the predicate function one time for each element in the array.
+   * @param thisArg An object to which the this keyword can refer in the predicate function. If thisArg is omitted, undefined is used as the this value.
+   */
+  filter<S extends T>(
+    predicate: BooleanConstructor,
+    thisArg?: any,
+  ): Exclude<S, Falsy>[];
+}

packages/generate-persisted-query-manifest/src/__tests__/cli.test.ts

@@ -1021,6 +1021,41 @@ export default config;
   await cleanup();
 });
 
+test("handles syntax errors when parsing source files", async () => {
+  const { cleanup, runCommand, writeFile } = await setup();
+
+  await writeFile(
+    "./src/greeting.graphql",
+    `
+query {
+  greeting
+`,
+  );
+  await writeFile(
+    "./src/components/my-component.js",
+    `
+import { gql } from '@apollo/client';
+
+const query;
+`,
+  );
+
+  const { code, stderr } = await runCommand();
+
+  expect(code).toBe(1);
+  expect(stderr).toMatchInlineSnapshot(`
+    [
+      "src/greeting.graphql",
+      "2:11  error  GraphQLError: Syntax Error: Expected Name, found <EOF>.",
+      "src/components/my-component.js",
+      "3:11  error  SyntaxError: Missing initializer in const declaration. (3:11)",
+      "✖ 2 errors",
+    ]
+  `);
+
+  await cleanup();
+});
+
 test("gathers and reports all errors together", async () => {
   const { cleanup, runCommand, writeFile } = await setup();
   const anonymousQuery = gql`
@@ -1060,6 +1095,13 @@ test("gathers and reports all errors together", async () => {
     "./src/current-user-fragment2.graphql",
     print(currentUserFragment),
   );
+  await writeFile(
+    "./src/syntax-error.graphql",
+    `
+query {
+  greeting
+`,
+  );
 
   const { code, stderr } = await runCommand();
 
@@ -1076,7 +1118,9 @@ test("gathers and reports all errors together", async () => {
       "1:1  error  Operation named "HelloQuery" already defined in: src/hello-query.graphql",
       "src/query.graphql",
       "1:1  error  Anonymous GraphQL operations are not supported. Please name your query.",
-      "✖ 5 errors",
+      "src/syntax-error.graphql",
+      "2:11  error  GraphQLError: Syntax Error: Expected Name, found <EOF>.",
+      "✖ 6 errors",
     ]
   `);
 

packages/generate-persisted-query-manifest/src/index.ts

@@ -14,6 +14,7 @@ import {
   print,
   type DocumentNode,
   visit,
+  GraphQLError,
 } from "graphql";
 import { first, sortBy } from "lodash";
 import { createHash } from "node:crypto";
@@ -88,9 +89,9 @@ interface Location {
 }
 
 interface DocumentSource {
-  node: DocumentNode;
+  node: DocumentNode | null;
   file: VFile;
-  location: Location;
+  location: Location | undefined;
 }
 
 const COLORS = {
@@ -126,6 +127,9 @@ const ERROR_MESSAGES = {
       definedOperationName,
     )}".`;
   },
+  parseError(error: Error) {
+    return `${error.name}: ${error.message}`;
+  },
 };
 
 function addError(
@@ -136,29 +140,64 @@ function addError(
   vfileMessage.fatal = true;
 }
 
+function parseLocationFromError(error: Error) {
+  if (error instanceof GraphQLError && error.locations) {
+    return error.locations[0];
+  }
+
+  const loc =
+    "loc" in error &&
+    typeof error.loc === "object" &&
+    error.loc !== null &&
+    error.loc;
+
+  const line = loc && "line" in loc && typeof loc.line === "number" && loc.line;
+  const column =
+    loc && "column" in loc && typeof loc.column === "number" && loc.column;
+
+  if (typeof line === "number" && typeof column === "number") {
+    return { line, column };
+  }
+
+  return;
+}
+
 function getDocumentSources(filepath: string): DocumentSource[] {
   const file = vfile({
     path: filepath,
     contents: readFileSync(filepath, "utf-8"),
   });
 
-  if (file.extname === ".graphql" || file.extname === ".gql") {
-    return [
-      {
-        node: parse(file.toString()),
-        file,
-        location: { line: 1, column: 1 },
-      },
-    ];
-  }
+  try {
+    if (file.extname === ".graphql" || file.extname === ".gql") {
+      return [
+        {
+          node: parse(file.toString()),
+          file,
+          location: { line: 1, column: 1 },
+        },
+      ];
+    }
 
-  return gqlPluckFromCodeStringSync(filepath, file.toString()).map(
-    (source) => ({
-      node: parse(source.body),
+    return gqlPluckFromCodeStringSync(filepath, file.toString()).map(
+      (source) => ({
+        node: parse(source.body),
+        file,
+        location: source.locationOffset,
+      }),
+    );
+  } catch (e: unknown) {
+    const error = e as Error;
+    const source = {
+      node: null,
       file,
-      location: source.locationOffset,
-    }),
-  );
+      location: parseLocationFromError(error),
+    };
+
+    addError(source, ERROR_MESSAGES.parseError(error));
+
+    return [source];
+  }
 }
 
 function maybeReportErrorsAndExit(files: VFile | VFile[]) {
@@ -197,6 +236,10 @@ export async function generatePersistedQueryManifest(
   const operationsByName = new Map<string, DocumentSource[]>();
 
   for (const source of sources) {
+    if (!source.node) {
+      continue;
+    }
+
     visit(source.node, {
       FragmentDefinition(node) {
         const name = node.name.value;
@@ -243,7 +286,9 @@ export async function generatePersistedQueryManifest(
   // Using createFragmentRegistry means our minimum AC version is 3.7. We can
   // probably go back to 3.2 (original createPersistedQueryLink) if we just
   // reimplement/copy the fragment registry code here.
-  const fragments = createFragmentRegistry(...sources.map(({ node }) => node));
+  const fragments = createFragmentRegistry(
+    ...sources.map(({ node }) => node).filter(Boolean),
+  );
   const manifestOperationIds = new Map<string, string>();
 
   const manifestOperations: PersistedQueryManifestOperation[] = [];
@@ -293,7 +338,9 @@ export async function generatePersistedQueryManifest(
 
   for (const [_, sources] of sortBy([...operationsByName.entries()], first)) {
     for (const source of sources) {
-      await client.query({ query: source.node, fetchPolicy: "no-cache" });
+      if (source.node) {
+        await client.query({ query: source.node, fetchPolicy: "no-cache" });
+      }
     }
   }