Fix configuration error display (#164)

.changeset/few-cougars-cry.md

@@ -0,0 +1,5 @@
+---
+"vscode-apollo": minor
+---
+
+Fix configuration error display

src/extension.ts

@@ -130,23 +130,26 @@ export async function activate(
     const response = JSON.parse(params) as Array<any> | ErrorShape;
 
     const hasActiveTextEditor = Boolean(window.activeTextEditor);
-    if (isError(response)) {
-      statusBar.showWarningState({
-        hasActiveTextEditor,
-        tooltip: "Configuration Error",
-      });
-      outputChannel.append(response.stack);
-
-      const infoButtonText = "More Info";
-      window
-        .showInformationMessage(response.message, infoButtonText)
-        .then((clicked) => {
-          if (clicked === infoButtonText) {
-            outputChannel.show();
-          }
+    if (Array.isArray(response)) {
+      const errors = response.filter((item) => isError(item));
+      if (errors.length) {
+        errors.forEach((response) => {
+          statusBar.showWarningState({
+            hasActiveTextEditor,
+            tooltip: "Configuration Error",
+          });
+          outputChannel.appendLine("---\n" + response.stack + "\n---");
+
+          const infoButtonText = "More Info";
+          window
+            .showInformationMessage(response.message, infoButtonText)
+            .then((clicked) => {
+              if (clicked === infoButtonText) {
+                outputChannel.show();
+              }
+            });
         });
-    } else if (Array.isArray(response)) {
-      if (response.length === 0) {
+      } else if (response.length === 0) {
         statusBar.showWarningState({
           hasActiveTextEditor,
           tooltip: "No apollo.config.js file found",

src/language-server/config/__tests__/loadConfig.ts

@@ -182,19 +182,11 @@ Object {
     it("throws when explorer.search fails", async () => {
       writeFilesToDir(dir, { "apollo.config.js": `* 98375^%*&^ its lit` });
 
-      const spy = jest.spyOn(console, "error");
-      // use this to keep the log quiet
-      spy.mockImplementation();
-
-      await loadConfig({
+      const error = await loadConfig({
         configPath: dirPath,
-      });
-
-      expect(spy).toHaveBeenCalledWith(
-        expect.stringMatching(/config file failed to load/i),
-      );
+      }).catch((e: any) => e);
 
-      spy.mockRestore();
+      expect(error.message).toMatch(/config file failed to load/i);
     });
 
     it("issues a deprecation warning when loading config from package.json", async () => {
@@ -234,21 +226,17 @@ Object {
     });
 
     it("throws if project type cant be resolved", async () => {
-      const spy = jest.spyOn(console, "error");
-      spy.mockImplementation();
-
       writeFilesToDir(dir, {
         "apollo.config.js": `module.exports = {}`,
       });
 
-      await loadConfig({
+      const error = await loadConfig({
         configPath: dirPath,
-      });
+      }).catch((e: any) => e);
 
-      expect(spy).toHaveBeenCalledWith(
-        expect.stringMatching(/Config needs to contain a 'client' field./i),
+      expect(error.message).toMatch(
+        /Config needs to contain a 'client' field./i,
       );
-      spy.mockRestore();
     });
   });
 

src/language-server/config/config.ts

@@ -195,24 +195,20 @@ export function parseApolloConfig(
         });
       }
     }
-    Debug.error(
-      fromZodError(parsed.error, {
-        prefix: "Error parsing config file:",
-        prefixSeparator: "\n",
-        issueSeparator: ";\n",
-        unionSeparator: "\n  or\n",
-      }).toString(),
-    );
-    return null;
+    throw fromZodError(parsed.error, {
+      prefix: `Error parsing config file ${configURI?.fsPath}:`,
+      prefixSeparator: "\n",
+      issueSeparator: ";\n",
+      unionSeparator: "\n  or\n",
+    });
   }
   if (parsed.data.client) {
     return new ClientConfig(parsed.data, configURI);
   } else if (parsed.data.rover) {
     return new RoverConfig(parsed.data, configURI);
   } else {
     // should never happen
-    Debug.warning("Invalid config file format!");
-    return null;
+    throw new Error("Invalid config file format!");
   }
 }
 

src/language-server/config/loadConfig.ts

@@ -51,17 +51,17 @@ export async function loadConfig({
       configPath,
     )) as ConfigResult<RawApolloConfigFormat>;
   } catch (error) {
-    Debug.error(`A config file failed to load with options: ${JSON.stringify(
+    throw new Error(`A config file failed to load with options: ${JSON.stringify(
       arguments[0],
     )}.
     The error was: ${error}`);
-    return null;
   }
 
   if (!loadedConfig || loadedConfig.isEmpty) {
     Debug.error(
       `No Apollo config found for project or config file failed to load. For more information, please refer to: https://go.apollo.dev/t/config`,
     );
+    // deliberately returning `null` here, but not throwing an error - the user may not have a config file and that's okay, it might just be a project without a graph
     return null;
   }
 

src/language-server/server.ts

@@ -19,9 +19,17 @@ import {
   LanguageServerCommands as Commands,
   LanguageServerRequests as Requests,
 } from "../messages";
+import { isValidationError } from "zod-validation-error";
 
 const connection = createConnection(ProposedFeatures.all);
+
 Debug.SetConnection(connection);
+const { sendNotification: originalSendNotification } = connection;
+connection.sendNotification = async (...args: [any, ...any[]]) => {
+  await whenConnectionInitialized;
+  connection.sendNotification = originalSendNotification;
+  connection.sendNotification(...args);
+};
 
 let hasWorkspaceFolderCapability = false;
 
@@ -61,14 +69,15 @@ workspace.onSchemaTags((params) => {
 });
 
 workspace.onConfigFilesFound(async (params) => {
-  await whenConnectionInitialized;
-
   connection.sendNotification(
     Notifications.ConfigFilesFound,
-    params instanceof Error
-      ? // Can't stringify Errors, just results in "{}"
-        JSON.stringify({ message: params.message, stack: params.stack })
-      : JSON.stringify(params),
+    JSON.stringify(params, (_key, value) =>
+      !value
+        ? value
+        : value instanceof Error || isValidationError(value)
+        ? { message: value.message, stack: value.stack }
+        : value,
+    ),
   );
 });
 

src/language-server/workspace.ts

@@ -24,7 +24,7 @@ export class GraphQLWorkspace {
   private _onDiagnostics?: NotificationHandler<PublishDiagnosticsParams>;
   private _onDecorations?: NotificationHandler<EngineDecoration[]>;
   private _onSchemaTags?: NotificationHandler<[ServiceID, SchemaTag[]]>;
-  private _onConfigFilesFound?: NotificationHandler<ApolloConfig[]>;
+  private _onConfigFilesFound?: NotificationHandler<(ApolloConfig | Error)[]>;
   private _projectForFileCache: Map<string, GraphQLProject> = new Map();
 
   private projectsByFolderUri: Map<string, GraphQLProject[]> = new Map();
@@ -46,7 +46,7 @@ export class GraphQLWorkspace {
     this._onSchemaTags = handler;
   }
 
-  onConfigFilesFound(handler: NotificationHandler<ApolloConfig[]>) {
+  onConfigFilesFound(handler: NotificationHandler<(ApolloConfig | Error)[]>) {
     this._onConfigFilesFound = handler;
   }
 
@@ -120,7 +120,7 @@ export class GraphQLWorkspace {
     const apolloConfigFolders = new Set<string>(apolloConfigFiles.map(dirname));
 
     // go from possible folders to known array of configs
-    let foundConfigs: ApolloConfig[] = [];
+    let foundConfigs: (ApolloConfig | Error)[] = [];
 
     const projectConfigs = Array.from(apolloConfigFolders).map((configFolder) =>
       loadConfig({ configPath: configFolder })
@@ -142,7 +142,7 @@ export class GraphQLWorkspace {
             );
           }
         })
-        .catch((error) => Debug.error(error)),
+        .catch((error) => foundConfigs.push(error)),
     );
 
     await Promise.all(projectConfigs);
@@ -181,8 +181,8 @@ export class GraphQLWorkspace {
 
     const project = this.projectForFile(configUri);
 
-    if (!config && this._onConfigFilesFound) {
-      this._onConfigFilesFound(error);
+    if (this._onConfigFilesFound) {
+      this._onConfigFilesFound([config || error]);
     }
     // If project exists, update the config
     if (project && config) {